tech-kern archive

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

Re: Usage of strncpy in the kernel



At Sun, 5 Jan 2025 03:06:30 +0000, David Holland <dholland-tech%netbsd.org@localhost> wrote:
Subject: Re: Usage of strncpy in the kernel
>
> On Sat, Jan 04, 2025 at 01:47:02PM +0100, Roland Illig wrote:
>  > > On 2025-01-04 11:14, David Holland wrote:
>  > >> 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);
>  >
>  > I like these proposals, as their names clearly communicate their
>  > purpose, and when looking at the code during an audit, I can quickly see
>  > that the copying takes place correctly, by looking at only a single
>  > function call. The functions should not be called "strlcpy" though, as
>  > that implies that they perform a strlen(src).
>
> ?
>
> I used the "l" because like strlcpy they (should) check all the
> lengths and won't overrun.

I think you may be confusing strlcpy() and strlcat().

strlcpy(), according to the manual page, is no safer than strncpy() in
terms of not copying more than the specified number of bytes -- the only
difference is in the former's automatic NUL-termination of the
destination.  There's no "checking of the length" in strlcpy().  There's
nothing to check [*].  strlcpy() only avoids one case of forgetfulness,
but at the expense of being unable to copy into a fixed-length non-nul-
terminated array.  The 'l' in strlcpy() is arguably misleading.  I don't
know for sure what a better name would have been -- maybe strncpy_str()
or something a little less ugly?

strlcat() is the only OpenBSD strl*() function which actually provides
any real added safety.  It's the only strl*() function which actually
checks the length of the destination string.  I think it would have been
slightly easier and "safer" to use, and it would be a complete
replacement for strncat(), if it had been given the following API
though:

     char *
     strlcat(char *dst, size_t dstsize, const char *src, size_t len);

As-is strlcat() cannot entirely replace strncat(), both because of the
difference in return type, and in the effective assumption that the
'size' parameter describes the destination.

Personally I don't care too much about strlcat() either way though --
indeed I don't think I've ever used strncat() in production code.  But
then again when I work with fixed-length character arrays I don't tend
to mix them up with C strings unless I first copy and convert them into
a C string.  I feel strncat() is to difficult to use safely except maybe
in a tiny number of possible scenarios where the sizes of all the memory
objects involved are very well known (i.e. maybe not as impossible as
gets() but...).

[*] I suppose if one considers that strlcpy() can detect truncation then
    one could call that a form of "checking the length", but I say that
    is stretching the language quite a bit.  ALSO, the downside of how
    strlcpy() checks for overflow can easily result in wandering past
    the end of a non-NUL-terminated source buffer, especially if the
    programmer assumes anything whatsoever about what the 'size'
    parameter might mean with respect to the valid length of the source
    buffer.  strlcpy() requires without exception that the source be a
    fully valid NUL-terminated string that fits entirely within the
    source memory object, and it does so without any way whatsoever to
    prevent accidental overruns if it is not.  strncpy() wins here.

>  > Simply calling them "strcpy" should suffice. The size_t parameters
>  > should all be called "size" instead of "len" or "max".
>
> There's a distinction there (length of a fixed-sized field vs. max
> size of a buffer) that shouldn't be discarded. It's a cue to the
> consumer of the interface; not necessarily a strong one but any little
> bit can sometimes be that hint that makes you go "wait a sec".

On that I agree...

> "len" vs. "size" is endlessly debatable, especially for char, but I
> tend to favor "length" for strings (and "num" or maybe "count" for
> arrays) because it's clearer about what the units are.

... though to retreat to the "in the kernel" part of this, the kernel
normally (e.g. maybe outside of a display driver) shouldn't really care
about the "length" of a string, especially not how many glyphs are
displayed for a string -- all that matters are the number of bytes to be
shuffled around and whether or not the target memory object is big
enough to hold that many bytes.

--
					Greg A. Woods <gwoods%acm.org@localhost>

Kelowna, BC     +1 250 762-7675           RoboHack <woods%robohack.ca@localhost>
Planix, Inc. <woods%planix.com@localhost>     Avoncote Farms <woods%avoncote.ca@localhost>

Attachment: pgptE7jBJ4Zy2.pgp
Description: OpenPGP Digital Signature



Home | Main Index | Thread Index | Old Index