Re: Usage of strncpy in the kernel

[David Holland <dholland-tech%netbsd.org@localhost>]

On Sat, Jan 04, 2025 at 09:57:56AM +1100, matthew green wrote:
 > the warning above this one is also pretty important for some
 > kernel uses, particularly if the buffer is gonna be written
 > back to userland.
 > 
 > it's very disappointing to me that strlcpy() didn't keep the
 > 'cleans whole buffer' feature of strncpy().  it has been the
 > source of at least one bug that i recall, when a strncpy to
 > strlcpy conversion was passed to copyout().
 > 
 > the problem cases Roland found should be considered carefully
 > before simply converting to strlcpy(), and we are not going to
 > remove strncpy() generally.  there are many APIs we do not use
 > C strings in, and strncpy() is the right method there.  eg,
 > all the ones where device name (char[16]) is used (note that
 > these are all ugly and the '16' is usually a magic number, it
 > would be really keen if someone were to fix this :)

I was going to post "yes we should ban strncpy from the kernel", for
the following reason: it is a special-case function with a specific
purpose, but it both doesn't serve that purpose very well _and_ is
widely misused in the wild.

strncpy is for accessing fixed-length text fields where there isn't
necessarily a null terminator. (The traditional example is ut_name in
struct utmp, which was historically 8 bytes long so there wasn't room
for both 8-character usernames and a \0, and expanding the field to 12
or 16 bytes when usernames were limited to 8 was an unconscionable
waste of space.)

strncpy is for _writing_ such a field: strncpy(field, s, sizeof(field))
will fill in the field correctly for all input strings s. This is why
it zero-fills.

However, it does not work correctly by itself for _reading_ such a
field: you want a function that takes the maximum _input_ length and
copies up to that many bytes, then always null-terminates the output.
There is also no need for that function to zero-fill.

There should also be a function for writing a fixed-length text field
that is always null-terminated (which would be the zero-filling
version of strlcpy) because that is also a thing, one traditional
example being ut_line in struct utmp.

Note that there are not many fixed-length not-necessarily-terminated
fields left nowadays, between general dislike of fixed-length text
fields, general dislike of binary formats, and a few extra bytes for
tidiness not being anywhere like as expensive as it was in 1975.

However, we do have enough of them to warrant not throwing out
strncpy, _except_ that I think we'd be better off defining the other
two functions mentioned above and adding a new (and matching) name for
what strncpy is really supposed to do, then ban strncpy itself.

(and strncat should definitely be banned, it serves no purpose these
days even if it weren't also confusingly defined)

-- 
David A. Holland
dholland%netbsd.org@localhost