This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Implement C11 annex K?


On Sat, Aug 16, 2014 at 12:40:44AM -0400, David A. Wheeler wrote:
> The "recommended" approach using strncpy is absurd. As Linus
> Torvalds quickly responds on Jul 28, 2014
> > Well, to be fair, "strncpy()" sucks too. It does the insane "pad
> > with zero" which is a performance disaster with any sanely sized
> > buffers.
> 
> Good thing we have snprintf to the rescue!! Oh wait, that *also*
> reads from the source even if it's longer than the target size.

Actually snprintf has both if you want:

snprintf(dest, destsize, "%.*s", srcsize, src);

In this case, src is not required to point to a null-terminated
string; it needs only point to something which is either a
null-terminated string or an array of at least srcsize bytes.

Conceptually this approach makes a lot of sense, since controlling how
much you write into the (limited size) destination and controlling how
much you read from a particular source (to prevent DoS from insanely
long input) are two separate issues. In particular, if you were using
two source strings, and wanted to put a limit of 256 bytes on each of
them when concatenating, you wouldn't want just a 512-byte limit on
the output (this would wrongly accept one or the other being overly
long as long as the other was small) but individual limits on each
input. Note that it's somewhat difficult to treat this kind of source
truncation as an error with snprintf, but it can be done with the help
of %n.

Only caveat: srcsize needs to have type int. If you pass a size_t
here, you invoke UB, since the * expects an int. In practice it
probably always works anyway due to calling conventions, but formally
it's UB.

> Now, it *is* true that Linus Torvalds continues with:
> >  And yeah, the strlcpy return value is broken by design.
> > If you're actually copying from user space in the kernel, do
> >   ret = strncpy_from_user(buf, userptr, len);
> >   if (ret < 0) return ret;
> >   if (ret == len) return -ETOOLONG;
> 
> There is no similar function in glibc to my knowledge. (I think the
> return value should just be negative if the data reaches len, to
> simplify truncation handling and force a strlcpy-like guarantee that
> the dest is terminated if it has length.) Would something like that
> be more acceptable, since that would overcome the objection above,
> and obviously the Linux kernel developers *do* use a special copying
> routine for copying up to a given length into fixed-size buffers?

I wouldn't really take anything the kernel developers do as good
practice. The kernel is FULL of integer type/size mismatch bugs and
haphazard use of types like they don't matter. Many of these even
crept into the public APIs...

Rich


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]