Index | Thread | Search

From:
"Theo de Raadt" <deraadt@openbsd.org>
Subject:
Re: sys/net/pf_ioctl.c fix two MSAN findings
To:
Johann Höpfner <hoepf@cit.tum.de>
Cc:
tech@openbsd.org
Date:
Fri, 04 Sep 2026 18:44:30 -0600

Download raw body.

Thread
> In general, strlcpy with discarded return value seems like an
> antipattern to me. These cases should probably use a function that does
> not scan the source until the next null-byte, but returns as soon as the
> destination is full. As far as I know this is exactly what linux has
> strScpy for.

That's quite a paragraph, I wnat to dig it apart.

Is it your argument that strScpy return value should not be checked, ok
how about -- does not need to be checked?

I got a bit confused there.  You said it is an anti-pattern to not check
the return value.

Then you said it was preferrable to use something else, where if you
review the actual use patterns in the ecosystem source using search
engines, you'll it is quite common to NOT CHECK the return value of that
alternative either, which leads to undiscoverable truncation, which gets
passed to some other subsystem and creates just another kind of
invariant.

So tell me -- how do you feel about snprintf?  People often don't check
that return value either.

It is incredibly difficult to move the ecosystem.  I believe a component
of that is "oh this sucks because of reasons A and B" so "please use this
other thing which does not suck except it has the same problems".

The strlcpy manual page has this to say on the matter.

RETURN VALUES
     Besides quibbles over the return type (size_t versus int) and signal
     handler safety (snprintf(3) is not entirely safe on some systems), the
     following two are equivalent:

           n = strlcpy(dst, src, len);
           n = snprintf(dst, len, "%s", src);

     Like snprintf(3), the strlcpy() and strlcat() functions return the total
     length of the string they tried to create.  For strlcpy() that means the
     length of src.  For strlcat() that means the initial length of dst plus
     the length of src.

     If the return value is >= dstsize, the output string has been truncated.
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     It is the caller's responsibility to handle this.
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

But people don't read.  I'm going to insist, you don't read either,
because saying "strscpy" is WEAK, because if you read all the places it
gets used you'll see EXACTLY the same anti-pattern.

And it is PERVASIVE.

An hour ago, someone pointed out a setsockopt IP_TOS against an AF_INET6
socket.  Once again, no error return check.  We created something which
hardens against that, and bugs are being found.  We had people doing TCP
sockopts against AF_UNIX sockets.  Not checking the return value.

Noone gives a damn about return values.

And they don't give a damn about return values for strscpy either.

The anti-pattern is PERVASIVE failure to check return values but
creating other functions where people don't check the return value is
not solving the damn problem.

But the OTHER anti-pattern is to propose use of functions which people
ALSO don't error check, and which will never go anywhere because it is a
str* function hilariously returning an -errno -- when errno is typically
not in the namespace.  It is a Linux kernel internal function that is unlikely
to become popular because of these namespace issues -- but it does not solve
the pervasive problem you brought up first because noone gives a damn about
return values.

The word I'm looking for here is hubris.