Re: [PATCH net] netfilter: nf_nat: avoid invalid nat_net pointer use on failed nf_nat_init()

[Mathias Krause <minipli@grsecurity.net>]

On 28.04.26 12:01, Pablo Neira Ayuso wrote:
> On Tue, Apr 28, 2026 at 11:09:17AM +0200, Mathias Krause wrote:
>> --- a/net/netfilter/nf_nat_core.c
>> +++ b/net/netfilter/nf_nat_core.c
>> @@ -1187,6 +1187,16 @@ int nf_nat_register_fn(struct net *net, u8 pf, const struct nf_hook_ops *ops,
>>  	struct nf_hook_ops *nat_ops;
>>  	int i, ret;
>>  
>> +#ifndef MODULE
>> +	/* If nf_nat_core is built-in and nf_nat_init() fails, dependent
>> +	 * modules like nft_chain_nat.ko may still call this function.
>> +	 * However, nat_net would be invalid, likely pointing to some other
>> +	 * per-net structure.
> 
> Hm, if nf_nat_init() fails, then nft_chain_nat should fail to load.

If nf_nat is a module, that is the case, yes. However, the failing case
had it built-in (CONFIG_NF_NAT=y) and there's little the kernel can do
when some init function fails -- the code and data will still be part of
vmlinux.

> 
> Maybe there is a different way to validate this dependency?

Yeah, maybe. Maybe move the 'nf_nat_hook != NULL' test to
nft_chain_nat_init()? It's exported already, so this should work --
assuming the respective init functions get called in the right order
(nf_nat_init() first, then nft_chain_nat_init()). But that should be the
case, even if both are built-in, according to the order in
net/netfilter/Makefile.

It's a little fragile, though. In case the link order changes one day,
the test would lead to a false positive, making nft_chain_nat fail for
the wrong reason.

Also, I'm uncertain if the link order is really that deterministic wrt.
init functions for LTO builds?

Mathias

> 
>> +	 */
>> +	if (WARN_ON_ONCE(!nf_nat_hook))
>> +		return -EOPNOTSUPP;
>> +#endif
>> +
>>  	if (WARN_ON_ONCE(pf >= ARRAY_SIZE(nat_net->nat_proto_net)))
>>  		return -EINVAL;
>>  
>> -- 
>> 2.47.3
>>