On Wed, Mar 11, 2020 at 05:45:31PM +0100, Takashi Iwai wrote: > On Wed, 11 Mar 2020 17:40:02 +0100, Johan Hovold wrote: > > But isn't the first snprintf() in such a sequence as much a part of the > > problem as the following ones? > > > > If the first pos = snprintf(buf, limit, ...) overflows buf, then the > > next pos += snprintf(buf, limit - pos, ...) will be called with with a > > negative size argument (i.e. a very large unsigned value), which > > effectively breaks the length check regardless of whether you replace it > > with scnprintf() or not. And all later calls will similarly continue > > writing beyond the end of buf. > > Yeah, that's the possible case although most calls are fine with it > since the limit is PAGE_SIZE or so. This might need a bit more > special care. Yeah, sure, it should be fine here too currently, even if the buffer size is defined in the caller. > > But wait a minute. This is user-space code, so there's no scnprintf(). > > Did you not compile test this? ;P > > > > In fact it seems no-one has for a while. This code is just broken and > > doesn't even compile any more. Maybe we should just drop it instead. > > Bah, I'm afraid that I overlooked this point! > > I've scraped over many places via a script-like work, and did the > compile testing of the kernel, but not about tools. If that's the > case, sorry for the mess, feel free to drop it. No worries. I'll try to take a look at the other breakages tomorrow and see if there's any point trying to salvage this at all. Johan _______________________________________________ greybus-dev mailing list greybus-dev@xxxxxxxxxxxxxxxx https://lists.linaro.org/mailman/listinfo/greybus-dev
|