linux-kernel - Re: [PATCH] netfilter: Consistently use NFPROTO_, not AF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aOeHRv_z-aBFQd4U@strlen.de>
Date: Thu, 9 Oct 2025 11:58:30 +0200
From: Florian Westphal <fw@...len.de>
To: Keno Fischer <keno@...iahub.com>
Cc: Pablo Neira Ayuso <pablo@...filter.org>,
	Jozsef Kadlecsik <kadlec@...filter.org>,
	netfilter-devel@...r.kernel.org, linux-kernel@...r.kernel.org,
	Phil Sutter <phil@....cc>
Subject: Re: [PATCH] netfilter: Consistently use NFPROTO_, not AF_

Keno Fischer <keno@...iahub.com> wrote:
> The uapi headers document `nfgen_family` as `AF_*`. However,
> this hasn't been technically true since 7e9c6eeb, which switched
> the interpretation of this field to `NFPROTO_*`.
> This is value-compatible on AF_INET (though note that this is
> NFPROTO_IPV4, *not* NFPROTO_INET), AF_INET6, AF_IPV6 and AF_DECnet,
> and AF_BRIDGE, but has since grown additional values.
> Now, because of the value compatibility between AF_ and
> NFPROTO_, it doesn't matter too much, but to the extent that
> the uapi headers constitute interface documentation, it can be
> misleading. For example, some userspace tooling, such as wireshark
> will print AF_UNIX for netlink packets that have an NFPROTO_INET
> family set. I will submit a patch for this downstream, but I wanted
> to cleanup the kernel side also. To that end, change the comment in
> the UAPI header and audit uses of AF_* in the netfilter code and
> switch them to NFPROTO_ unless calling non-netfilter APIs.

I'm not sold on this patch, lots of code churn for little gain.

> diff --git a/include/uapi/linux/netfilter/nfnetlink.h b/include/uapi/linux/netfilter/nfnetlink.h
> index 6cd58cd2a6f0..9d7fe3daf327 100644
> --- a/include/uapi/linux/netfilter/nfnetlink.h
> +++ b/include/uapi/linux/netfilter/nfnetlink.h
> @@ -32,7 +32,7 @@ enum nfnetlink_groups {
>  /* General form of address family dependent message.
>   */
>  struct nfgenmsg {
> -	__u8  nfgen_family;		/* AF_xxx */
> +	__u8  nfgen_family;		/* NFPROTO_xxx */
>  	__u8  version;		/* nfnetlink version */
>  	__be16    res_id;		/* resource id */
>  };

Maybe just chase down comments like this and leave the rest alone?

> @@ -690,7 +690,7 @@ module_param_call(hashsize, nf_conntrack_set_hashsize, param_get_uint,
>  		  &nf_conntrack_htable_size, 0600);
>  
>  MODULE_ALIAS("ip_conntrack");
> -MODULE_ALIAS("nf_conntrack-" __stringify(AF_INET));
> -MODULE_ALIAS("nf_conntrack-" __stringify(AF_INET6));
> +MODULE_ALIAS("nf_conntrack-" __stringify(NFPROTO_IPV4));
> +MODULE_ALIAS("nf_conntrack-" __stringify(NFPROTO_IPV6));

This breaks module autloading aliases, __stringify needs a define not enum:

alias:          nf_conntrack-NFPROTO_IPV6
alias:          nf_conntrack-NFPROTO_IPV4

... this NFPROTO_IPV6 should not be there, this needs to be kept as

alias:          nf_conntrack-10
alias:          nf_conntrack-2