tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Usage of strncpy in the kernel



Yikes. I wrote that too late in the night. :-)

strncpy(buf, src, sizeof(buf));
buf[sizeof(buf)-1] = 0;

is what I should have written. Sorry.

But no, it only throws away the last byte of data if it don't fit into the buffer, in which case that is the proper thing to do. You can't fit more than N-1 bytes into an N size buffer, if you want a terminating NUL. No way around that. If you want 8 actual bytes in there, then the buffer needs to be 9 bytes in size (obviously).

The explicit adding of a NUL after the call could be considered a slight inconvenience, but honestly, I very much do not consider that to disqualify the function, and I do think if people can't get this right, then they probably also do all kind of other errors, and maybe they should be put in a safer place, and not poke around in the kernel.

But yes, if the source is short, and destination large, and we potentially might run off into nonexistent memory for the source, then you have another problem in that. I'm not sure that is a real world problem, though. What kind of text data would you expect to be copying in and out? Novels as one chunk? And I think such access should be handled already anyway if it comes from userland. Users can, after all, provide broken pointers already.

I don't have anything against your suggested functions, though. But I'm going to bet money that people would sometimes end up using the wrong one in the end, and you actually didn't succeed in "solving" this problem after all. In the end, it always comes back to the same thing. The language, or functions, does not solve the problems, or make anything safe. The programmer needs to know what he is doing, and he can do things wrong in any environment if he don't know. (Which is also why I dislike the hype around some languages, which makes it seem like the problem is solved just by a language change.)

  Johnny


On 2025-01-04 11:14, David Holland wrote:
On Sat, Jan 04, 2025 at 03:25:20AM +0100, Johnny Billquist wrote:
  > 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;

That both has the arguments in the wrong order and doesn't entirely
make sense (did you mean src instead of dst? Or did you mean to
null-terminate dst?)

However, no matter how you patch it up it still doesn't terminate the
output unless you remember to, which is a common problem when people
try to use strncpy as a substitute for strlcpy. Then, if you meant
strncpy(dst, buf, sizeof(buf)), it doesn't crosscheck the length of
dst and also throws away the last byte of the data, and if you meant
strncpy(buf, src, sizeof(buf)) it doesn't check the length of src and
will run off the end if the size of buf is too large.

Which is exactly why there should be a function for this :-)

maybe something like

strlcpy_tofixed(char *dest, size_t destlen, const char *src);
strlcpy_fromfixed(char *dest, size_t destmax, const char *src, size_t srclen);

and

strlcpy_zerofill(char *dest, const char *src, size_t destmax);


--
Johnny Billquist                  || "I'm on a bus
                                  ||  on a psychedelic trip
email: bqt%softjar.se@localhost             ||  Reading murder books
pdp is alive!                     ||  tryin' to stay hip" - B. Idol



Home | Main Index | Thread Index | Old Index