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


Groups > linux.kernel > #1239507

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

From Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups linux.kernel
Subject Re: [PATCH] string: Improve the generic strlcpy() implementation
Date 2015-10-05 14:30 +0200
Message-ID <qgd3Y-7eX-27@gated-at.bofh.it> (permalink)
References <q7g13-2Rm-7@gated-at.bofh.it> <qfTRD-4Np-7@gated-at.bofh.it> <qgc7U-5TG-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, Oct 5, 2015 at 12:27 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> 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?

Well, I'm not sure the race really matters. I personally think
strlcpy() is a horrible interface, and the thing is, the return value
of strlcpy (which is what can race) is kind of useless because it's
not actually the size of the resulting string *anyway* (because of the
overflow issue).

So I'm not sure it's worth even fixing.

Also, if you do this, then you're better off using the (hopefully
optimized) "strlen()" for the tail part of the strlcpy destination for
the overflow case that didn't get copied.

In other words, I think your patch is overly fragile and complex.
Instead, you might choose to implement strlcpy() in terms of
"strscpy()" and "strlen()".

Something like

  int strlcpy(dst, src, len)
  {
     // do the actual copy
     int n = strscpy(dst, src, len);

     // handle the insane and broken strlcpy overflow return value
     if (n < 0)
         return len + strlen(src+len);

     return n;
   }

but I didn't actually verify that the above is correct for all the corner case.

The point being, that you really shouldn't waste your time
implementing the broken BSD strlcpy crap as an actual first-class
implementation. You're better off just using a strscpy() as the
primary engine, and then implementing the broken strlcpy interfaces on
top of it.

Does the above work? I'd take a patch that implements that if it's
tested and somebody has thought about it a lot. But I don't like your
patch that open-codes the insane interface with complex and fragile
code.

          Linus
--
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 | NextPrevious 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