Re: [PATCH bpf v5 1/2] lib/strncpy_from_user.c: Don't overcopy bytes after NUL terminator

[Linus Torvalds]

On Fri, Nov 13, 2020 at 12:57 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> (a) is the only case.

Ok, good.

The (b) case is certainly valid in theory (and we might even
traditionaly have had something like that for things like ->comm[]
accesses, although I think we got rid of it).

But the (b) case is _so_ hard to think about and so easy to get wrong
- readers have to be very careful to only read each byte of the source
exactly once - that it's much much better to try to avoid it.

> But I think if glibc's strncpy() did something like this it would
> probably caused a lot of pain for user space.

Oh, absolutely. The standard strncpy() function has some very strict
behavior issues, including that zero-padding of the *whole*
destination buffer, which would be absolutely horrid for things like
fetching pathnames from user space (our buffer is generally close to a
page in size).

In fact, the kernel strncpy() (ie the one that doesn't copy from user)
does ado the whole "pad all zeroes at the end" exactly because people
might depend on that. So the _actual_ strncpy() function conforms to
the standard use - but you generally shouldn't use it, exactly because
it's such a horrible interface. Only good for very small buffers.

> The hash element example above is typical bpf usage.

The core kernel does have one very common string hash case, but it's
for path components, and never the whole string - so it already has to
deal with the fact that the string is very much delimited in place by
not just NUL at the end, but also '/' characters etc.

So no "copy it as a string from user space, and then use it as a
block" that I'm aware of.

              Linus