Messages in this thread | | | Date | Tue, 24 Nov 2015 00:51:44 +0200 | Subject | Re: [PATCH 03/14] lib/vsprintf.c: eliminate potential race in string() | From | Andy Shevchenko <> |
| |
On Mon, Nov 23, 2015 at 11:29 PM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > If the string corresponding to a %s specifier can change under us, we > might end up copying a \0 byte to the output buffer. There might be > callers who expect the output buffer to contain a genuine C string > whose length is exactly the snprintf return value (assuming truncation > hasn't happened or has been checked for). > > We can avoid this by only passing over the source string once, > stopping the first time we meet a nul byte (or when we reach the given > precision), and then letting widen_string() handle left/right space > padding. As a small bonus, this code reuse also makes the generated > code slightly smaller. >
Could it be pair of patches: a) re-use, b) optimize for fuzzy strings?
> Cc: Ingo Molnar <mingo@kernel.org> > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > lib/vsprintf.c | 28 +++++++++------------------- > 1 file changed, 9 insertions(+), 19 deletions(-) > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index a021e6380404..63ca52366049 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -557,32 +557,22 @@ char *widen_string(char *buf, int n, char *end, struct printf_spec spec) > static noinline_for_stack > char *string(char *buf, char *end, const char *s, struct printf_spec spec) > { > - int len, i; > + int len = 0; > + size_t lim = spec.precision;
Just a nitpick: maybe longer first?
> > if ((unsigned long)s < PAGE_SIZE) > s = "(null)"; > > - len = strnlen(s, spec.precision); > - > - if (!(spec.flags & LEFT)) { > - while (len < spec.field_width--) { > - if (buf < end) > - *buf = ' '; > - ++buf; > - } > - } > - for (i = 0; i < len; ++i) { > - if (buf < end) > - *buf = *s; > - ++buf; ++s; > - } > - while (len < spec.field_width--) { > + while (lim--) { > + char c = *s++; > + if (!c) > + break; > if (buf < end) > - *buf = ' '; > + *buf = c; > ++buf; > + ++len; > } > - > - return buf; > + return widen_string(buf, len, end, spec); > } > > static noinline_for_stack > -- > 2.6.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
-- With Best Regards, Andy Shevchenko
| |