On Wed, Mar 11, 2020 at 12:01:26PM +0100, Takashi Iwai wrote: > On Wed, 11 Mar 2020 11:09:03 +0100, > Johan Hovold wrote: > > > > On Wed, Mar 11, 2020 at 11:02:33AM +0100, Takashi Iwai wrote: > > > On Wed, 11 Mar 2020 10:58:14 +0100, > > > Johan Hovold wrote: > > > > > > > > On Wed, Mar 11, 2020 at 10:19:06AM +0100, Takashi Iwai wrote: > > > > > Since snprintf() returns the would-be-output size instead of the > > > > > actual output size, the succeeding calls may go beyond the given > > > > > buffer limit. Fix it by replacing with scnprintf(). > > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > > > > > --- > > > > > drivers/staging/greybus/tools/loopback_test.c | 24 ++++++++++++------------ > > > > > > > > Thanks for the fix. > > > > > > > > Would you mind resending with a "staging: greybus: loopback_test:" > > > > prefix since this is not a subsystem wide issue, bur rather a bug in a > > > > specific user-space tool? > > > > > > OK, will do that. > > > > Thanks. > > > > Perhaps you should replace the snprintf() at the start of the function > > in question as well by the way. > > Yeah, it's I also wonder while working on many other codes, too. > I decided to minimize the changes at this time and concentrate only on > the code that has a pattern like: > pos += snprintf(buf, limit - pos, ...) 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. 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. Johan _______________________________________________ greybus-dev mailing list greybus-dev@xxxxxxxxxxxxxxxx https://lists.linaro.org/mailman/listinfo/greybus-dev
|