Re: Usage of strncpy in the kernel

Well, as for using strncpy to read out strings from memory as described 
below, it is really trivial to get it right, using strncpy, so I don't 
really agree with that it should be banned. But people need to do things 
right.

But just the following solves it nicely, with very little headaches.

char buf[8];
strncpy(sizeof(buf), dst, buf);
buf[sizeof(buf)-1] = 0;

And you'll all good, assuming you want the string copied out, and be 
properly terminated. Just change the size of buf to whatever size you 
want, if you want to get strings out that are longer (than 7 in this case).

But I would assume everyone here knows this already. But my point is 
that I don't really see strncpy as having much of an issue. You just 
need to do things right, which is a more general comment for any code.

   Johnny

On 2025-01-04 01:14, David Holland wrote:
> 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)
> 

-- 
Johnny Billquist                  || "I'm on a bus
                                   ||  on a psychedelic trip
email: [email protected]             ||  Reading murder books
pdp is alive!                     ||  tryin' to stay hip" - B. Idol