[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VePn-1kge9NEJHjyAkVJ8xhKutjGqjfB_iUMjZx-Kk0OA@mail.gmail.com>
Date: Tue, 24 Nov 2015 00:51:44 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Rasmus Villemoes <linux@...musvillemoes.dk>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Ingo Molnar <mingo@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 03/14] lib/vsprintf.c: eliminate potential race in string()
On Mon, Nov 23, 2015 at 11:29 PM, Rasmus Villemoes
<linux@...musvillemoes.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@...nel.org>
> Signed-off-by: Rasmus Villemoes <linux@...musvillemoes.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@...r.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists