From: "Theo de Raadt" <deraadt@openbsd.org>
Subject: Re: sys/net/pf_ioctl.c fix two MSAN findings
To: Johann =?utf-8?Q?H=C3=B6pfner?= <hoepf@cit.tum.de>
Cc: tech@openbsd.org
Date: Fri, 04 Sep 2026 21:08:37 -0600

Johann Höpfner <hoepf@cit.tum.de> wrote:

> Index: sys/net/pf_ioctl.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf_ioctl.c,v
> diff -u -p -r1.433 pf_ioctl.c
> --- sys/net/pf_ioctl.c	27 Jul 2026 19:02:48 -0000	1.433
> +++ sys/net/pf_ioctl.c	1 Sep 2026 15:19:39 -0000
> @@ -2892,6 +2892,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
>  	case DIOCSETSTATUSIF: {
>  		struct pfioc_iface	*pi = (struct pfioc_iface *)addr;
>  
> +		pi->pfiio_name[sizeof(pi->pfiio_name)-1] = '\0';
>  		NET_LOCK();
>  		PF_LOCK();
>  		if (pi->pfiio_name[0] == 0) {

I disagree.  Userland has issued a ioctl with a struct pfioc_iface,
which has the field

         char     pfiio_name[IFNAMSIZ];

That field is a intended to be a string.   It is not a byte array without a
terminal NUL.

If userland passes in a value which is not NUL-terminated, this is not a
string and the ioctl should fail.  For example "abcdefghijklmn10" in
[IFNAMSIZ = 16], without a NUL, is not a string, so it is invalid input,
and should return an error, maybe EINVAL, ESRCH, something like that.

If userland passes in memcpy(pi->pfiio_name, "abcdefghijklmn10",
IFNAMSIZ) without a NUL, your proposal is to delete the last character
'0' and make it a '\0" and carry on as it that's ok. The ioctl now acts
upon interface "abcdefghijklmn1", which I cannot agree with.

This is pretty much the "string truncation lead to an invariant" problem
I mentioned in my other responses.  And this is why strlcpy and strlcat,
like snprintf, report truncation with the return value.  Not that a
string function is really playing a part here.  The issue is that the
ioctl parameter is not completely validated at the ioctl boundary.