lkml.org 
[lkml]   [2014]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] isdnloop: NUL-terminate strings from userspace
On 03/07/2014 12:52 PM, Dan Carpenter wrote:
> On Fri, Mar 07, 2014 at 12:42:12PM +0100, Vegard Nossum wrote:
>> On 03/07/2014 12:26 PM, Dan Carpenter wrote:
>>> On Fri, Mar 07, 2014 at 11:56:04AM +0100, Vegard Nossum wrote:
>>>> Both the in-kernel and BSD strlcpy() require that the source string is
>>>> NUL terminated.
>>>
>>> No. You're obviously wrong. What on earth?
>>
>> Well, from lib/string.c:
>>
>> size_t strlcpy(char *dest, const char *src, size_t size)
>> {
>> size_t ret = strlen(src);
>>
>
> Ah... So you mean that we could read far beyond the end of the string
> and it would be a DoS because there would be 4 gigs of memory before we
> hit a NUL character. That won't happen in this case because the user
> only controls a small buffer. Normal memory is full of NUL chars.

Well, that's true, but what happens if you accidentally read from an
unmapped page or an mmio page?

I agree that it's not likely to happen in practice, but that's true for
a lot of kernel bugs that we nevertheless want to fix. Are you saying
that the patch is bad or wrong?

> I don't know the speed impact of changing the strlen() there to
> strnlen().

I don't think we could or should do that, because it changes the
semantics of the function. I think C string operations are probably
confusing enough already without breaking compatibility with existing
documentation and other implementations.

Also, the size parameter is the size of the destination buffer, not the
source string. So even if we could make it safe to use strnlen() in this
case, it still wouldn't be safe in other cases where the source string
is shorter than the destination.

>
>> The BSD man page:
>>
>> "Also note that strlcpy() and strlcat() only operate on true ``C''
>> strings. This means that for strlcpy() src must be NUL-terminated
>> and for strlcat() both src and dst must be NUL-terminated."
>
> It's talking about the kind of strings. If it's a string which includes
> NUL characters the strlcpy() won't work for that. Or if it is not
> *supposed* to end in a NUL character then it won't work for that.
>
> We are using C strings here.

I think the man page is quite clear: "for strlcpy() src must be
NUL-terminated". Without the patch, this is not necessarily the case.


Vegard


\
 
 \ /
  Last update: 2014-03-07 14:21    [W:0.228 / U:0.348 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site