Re: [PATCH] netfilter: nf_tables: fix use-after-free on ops->dev

[Helen Koike <koike@igalia.com>]

Hi Pablo,

Thanks for your quick reply, please see my comments below.

On 3/2/26 8:08 PM, Pablo Neira Ayuso wrote:
> On Mon, Mar 02, 2026 at 06:26:05PM -0300, Helen Koike wrote:
>> struct nf_hook_ops has a pointer to dev, which can be used by
>> __nf_unregister_net_hook() after it has been freed by tun_chr_close().
>>
>> Fix it  by calling dev_hold() when saving dev to ops struct.
> 
> Sorry, I don't think this patch works, dev_hold()/dev_put() use
> per_cpu area.

In my understanding, being a percpu_ref shouldn't affect the behavior, 
since after a percpu_ref_kill() it switches to the usual mode with 
shared atomic_t counter.

But if I understood correctly from your comment below, the proper 
solution would be to fix the order that the hooks are released, is my 
understanding correct?

> 
> The nf_tables_flowtable_event() function used to release the hook, but
> now things have changed since there is auto-hook registration.

I'll investigate this further. Thanks a lot for the pointer (and sorry 
for the noise).

Regards,
Helen

> 
>> Reported-by: syzbot+bb9127e278fa198e110c@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=bb9127e278fa198e110c
>> Signed-off-by: Helen Koike <koike@igalia.com>
>> ---
>>   net/netfilter/nf_tables_api.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
>> index fd7f7e4e2a43..00b5f900a51d 100644
>> --- a/net/netfilter/nf_tables_api.c
>> +++ b/net/netfilter/nf_tables_api.c
>> @@ -352,6 +352,7 @@ static void nft_netdev_hook_free_ops(struct nft_hook *hook)
>>   
>>   	list_for_each_entry_safe(ops, next, &hook->ops_list, list) {
>>   		list_del(&ops->list);
>> +		dev_put(ops->dev);
>>   		kfree(ops);
>>   	}
>>   }
>> @@ -2374,6 +2375,7 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
>>   			err = -ENOMEM;
>>   			goto err_hook_free;
>>   		}
>> +		dev_hold(dev);
>>   		ops->dev = dev;
>>   		list_add_tail(&ops->list, &hook->ops_list);
>>   	}
>> -- 
>> 2.53.0
>>