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

Groups > linux.kernel > #1240230

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

Show all headers | View raw

* Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> On Mon, Oct 05 2015, Ingo Molnar <mingo@kernel.org> wrote:
> 
> > * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> >> So I finally pulled it. I like the patch, I like the new interface, but 
> >> despite that I wasn't really sure if I wanted to pull it in - thus the long 
> >> delay of me just seeing this in my list of pending pulls for almost a month, 
> >> but never really getting to the point where I decided I want to commit to it.
> >
> > Interesting. I noticed that strscpy() says this in its comments:
> >
> >  * In addition, the implementation is robust to the string changing out
> >  * from underneath it, unlike the current strlcpy() implementation.
> >
> > The strscpy() interface is very nice, but shouldn't we also fix this strlcpy() 
> > unrobustness/race it refers to, in light of the 2000+ existing strlcpy() call 
> > sites?
> 
> How about every single occurence of %s in a format string? vsnprintf also has 
> that "issue", but has it actually ever been a problem? The window for something 
> bad to happen is probably also much larger in the printf case, and especially 
> when when some %p extension is used and/or the vsnprintf user is kasprintf() 
> (where we 'replay' the formatting, having hopefully obtained a correct-sized 
> buffer).
> 
> In fact, writing this, it occurs to me that we should probably check the return 
> value of the second vsnprintf call in kasprintf and compare to the first, 
> issuing a warning if they don't match.
> 
> 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, 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.

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.

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

  - phase out the remaining two dozen or so 'complex' strlcpy() uses one by one.

  - mark strlcpy() deprecated, add checkpatch warning.

Thanks,

	Ingo
--
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: [GIT PULL] strscpy string copy function Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-04 18:00 +0200
  [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-05 13:30 +0200
    Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-05 14:00 +0200
      Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-05 15:20 +0200
        Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-05 16:10 +0200
        Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-05 16:10 +0200
        Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-05 16:40 +0200
          Re: [PATCH] string: Improve the generic strlcpy() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-05 17:40 +0200
            Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-05 18:10 +0200
    Re: [PATCH] string: Improve the generic strlcpy() implementation Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-05 14:30 +0200
      Re: [PATCH] string: Improve the generic strlcpy() implementation Ingo Molnar <mingo@kernel.org> - 2015-10-05 15:20 +0200
    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