Download raw body.
sys/net/pf_ioctl.c fix two MSAN findings
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.
sys/net/pf_ioctl.c fix two MSAN findings