Re: [PATCH] isdnloop: NUL-terminate strings from userspace

[Vegard Nossum]

On 31 March 2014 15:36, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> On Mon, Mar 31, 2014 at 02:56:07PM +0200, Vegard Nossum wrote:
>> Ping, Dave? Just making sure this doesn't fall through the cracks. I
>> don't see the patch applied anywhere yet and without this patch we
>> still have a valid security concern IMO.
>
> Gar.  No...  To recap:
>
>> On 7 March 2014 11:56, Vegard Nossum <vegard.nossum@xxxxxxxxxx> wrote:
>> > Both the in-kernel and BSD strlcpy() require that the source string is
>> > NUL terminated.
>
> The *whole point* of strlcpy() is that the source string doesn't have to
> be NUL terminated.  The BSD man pages are trying to say that strlcpy()
> only works on C-strings as opposed to Vstr or other safer string
> implementions.

I read the BSD man page differently. Also, if you look at the actual
BSD implementation, it also scans the remaining buffer until it hits a
0.

I quote again: "for strlcpy() src must be NUL-terminated". It doesn't
get much clearer than that.

> There is a potential problem in the kernel implementation of strlcpy()
> because it does:
>
> lib/string.c
>    149  size_t strlcpy(char *dest, const char *src, size_t size)
>    150  {
>    151          size_t ret = strlen(src);
>    152
>    153          if (size) {
>    154                  size_t len = (ret >= size) ? size - 1 : ret;
>    155                  memcpy(dest, src, len);
>    156                  dest[len] = '\0';
>    157          }
>    158          return ret;
>    159  }
>
> The strlen() on line 151 could read beyond the end of the source buffer
> and if the memory wasn't mapped, it could Oops.
>
> That concern doesn't apply here because the source string is on stack
> memory and we will hit a NUL character before we hit unmapped memory.

As before, I agree that it's _likely_ we'll hit a 0 before hitting
unmapped memory, but I don't see at all that we have a guarantee of
it.


Vegard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/