Re: nfnetlink_queue crashes kernel

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Fri, Apr 03, 2026 at 02:22:01PM +0200, Florian Westphal wrote:
> Hello,
> 
> nfnetlink_queue is currently broken in at least two ways.
> 1). kernel will crash under pressure because
>     struct nf_queue_entry is freed via kfree, but parallel
>     cpu can still observe this while walking the (global) rhashtable.
> 
> 2) a4400a5b343d ("netfilter: nfnetlink_queue: nfqnl_instance GFP_ATOMIC -> GFP_KERNEL_ACCOUNT allocation") should have updated the spinlock to use the _bh variant, if the queue exists we risk deadlock via softirq recursion.
> 
> Minimal fix, that I am not a fan of:
> 
> diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h
> --- a/include/net/netfilter/nf_queue.h
> +++ b/include/net/netfilter/nf_queue.h
> @@ -24,6 +24,7 @@ struct nf_queue_entry {
>  	bool			nf_ct_is_unconfirmed;
>  	u16			size; /* sizeof(entry) + saved route keys */
>  	u16			queue_num;
> +	struct rcu_head		head;
>  
>  	/* extra space to store route keys */
>  };
> diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
> index 7f12e56e6e52..385d1fe704ae 100644
> --- a/net/netfilter/nf_queue.c
> +++ b/net/netfilter/nf_queue.c
> @@ -74,7 +74,7 @@ static void nf_queue_entry_release_refs(struct nf_queue_entry *entry)
>  void nf_queue_entry_free(struct nf_queue_entry *entry)
>  {
>  	nf_queue_entry_release_refs(entry);
> -	kfree(entry);
> +	kfree_rcu(entry, head);
>  }
>  EXPORT_SYMBOL_GPL(nf_queue_entry_free);
>  
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index 47f7f62906e2..fc44ea4e5128 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -190,7 +190,7 @@ instance_create(struct nfnl_queue_net *q, u_int16_t queue_num, u32 portid)
>  	spin_lock_init(&inst->lock);
>  	INIT_LIST_HEAD(&inst->queue_list);
>  
> -	spin_lock(&q->instances_lock);
> +	spin_lock_bh(&q->instances_lock);
>  	if (instance_lookup(q, queue_num)) {
>  		err = -EEXIST;
>  		goto out_unlock;
> @@ -204,7 +204,7 @@ instance_create(struct nfnl_queue_net *q, u_int16_t queue_num, u32 portid)
>  	h = instance_hashfn(queue_num);
>  	hlist_add_head_rcu(&inst->hlist, &q->instance_table[h]);
>  
> -	spin_unlock(&q->instances_lock);
> +	spin_unlock_bh(&q->instances_lock);
>  
>  	return inst;
>  
> 
> 
> 
> A probably better fix is to make the rhashtable perqueue, which is
> much more intrusive at this late stage.
> 
> I prefer to revert both changes and not accept a reworked hashtable
> until we have an extension to nft_queue.sh selftest.

+1, then take a bit more time to get this right.