Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1241027

Re: [PATCH] string: Improve the generic strlcpy() implementation

Show all headers | View raw

On Tue, Oct 06 2015, Ingo Molnar <mingo@kernel.org> wrote:

> * Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>
>> 
>> I'm not against making strlcpy more robust, but I think the theoretical race is 
>> far more likely to manifest through a member of the printf family.
>
> So the printf family is generally less frequently used in ABI output than string 
> copies,

Huh? snprintf and friends are often used just to copy or concatenate
strings (essentially, any format string containing no specifiers other
than %s does exactly that) - even if a str*() function could do the same
thing. I see no reason to believe this wouldn't also be done in cases
where the final string ends up being presented to userspace.

> but yeah, since there are 15,000 s[n]printf() calls in the kernel it's 
> more likely to be an issue not just by virtue of timing, but also by sheer mass of 
> usage, statistically.

Yes.

> So I'd improve it all in the following order:
>
>   - fix the strscpy() uninitialized use
>
>   - base strlcpy() on strscpy() via the patch I sent. This makes all users faster 
>     and eliminates the theoretical race.

I'm not so sure about that part. I'd strongly suspect that the vast
majority of strings handled by strlcpy (and in the future strscpy) are
shorter than 32 bytes, so is all the word_at_a_time and pre/post
alignment yoga really worth it? strscpy is 299 bytes - that's a lot of
instruction cache lines, and it will almost always be cache cold. This
isn't an argument against basing strlcpy on strscpy (that would likely
just make the former a little smaller), but I'd like to see numbers
(cycle counts, distribution of input lengths, ...)  before I believe in
the performance argument.

>   - phase out 'simple' strlcpy() uses via an automated patch. This gets rid of 
>     2,000 strlcpy() call sites in a single pass.

That seems to be exactly the kind of mass-conversion Linus referred to
above. Also, you can't really do it if strscpy keeps it __must_check
annotation, as you'd then introduce 2000+ warnings...

Rasmus
--
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/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH] string: Improve the generic strlcpy() implementation Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-10-06 00:30 +0200
  Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-06 10:00 +0200
  Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-06 10:10 +0200
    Re: [PATCH] string: Improve the generic strlcpy() implementation Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-10-07 00:10 +0200
      Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-07 09:20 +0200
        Re: [PATCH] string: Improve the generic strlcpy() implementation Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-10-07 11:10 +0200
          Re: [PATCH] string: Improve the generic strlcpy() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-07 11:30 +0200
            Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-08 10:50 +0200
              Re: [PATCH] string: Improve the generic strlcpy() implementation Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-10-09 10:20 +0200
                [RFC 0/3] eliminate potential race in string() (was: [PATCH] string: Improve the generic strlcpy() implementation) Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-10-09 11:20 +0200
                [RFC 1/3] lib/vsprintf.c: pull out padding code from dentry_name() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-10-09 11:20 +0200
                [RFC 3/3] lib/vsprintf.c: eliminate potential race in string() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-10-09 11:20 +0200
                [RFC 2/3] lib/vsprintf.c: move string() below widen_string() Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-10-09 11:20 +0200
                Re: [RFC 0/3] eliminate potential race in string() (was: [PATCH]  string: Improve the generic strlcpy() implementation) Ingo Molnar <mingo@kernel.org> - 2015-10-10 09:50 +0200

csiph-web