[PATCH net] netfilter: nfnetlink_queue: fix entry leak in bridge verdict error path

[PATCH net] netfilter: nfnetlink_queue: fix entry leak in bridge verdict error path

[Hyunwoo Kim]

nfqnl_recv_verdict() calls find_dequeue_entry() to remove the queue
entry from the queue data structures, taking ownership of the entry.
For PF_BRIDGE packets, it then calls nfqa_parse_bridge() to parse VLAN
attributes.  If nfqa_parse_bridge() returns an error (e.g. NFQA_VLAN
present but NFQA_VLAN_TCI missing), the function returns immediately
without freeing the dequeued entry or its sk_buff.

This leaks the nf_queue_entry, its associated sk_buff, and all held
references (net_device refcounts, struct net refcount).  Repeated
triggering exhausts kernel memory.

Fix this by dropping the entry via nfqnl_reinject() with NF_DROP verdict
on the error path, consistent with other error handling in this file.

Fixes: 8d45ff22f1b4 ("netfilter: bridge: nf queue verdict to use NFQA_VLAN and NFQA_L2HDR")
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 net/netfilter/nfnetlink_queue.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 7f5248b5f1ee..47f7f62906e2 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -1546,8 +1546,10 @@ static int nfqnl_recv_verdict(struct sk_buff *skb, const struct nfnl_info *info,
 
 	if (entry->state.pf == PF_BRIDGE) {
 		err = nfqa_parse_bridge(entry, nfqa);
-		if (err < 0)
+		if (err < 0) {
+			nfqnl_reinject(entry, NF_DROP);
 			return err;
+		}
 	}
 
 	if (nfqa[NFQA_PAYLOAD]) {
-- 
2.43.0

Re: [PATCH net] netfilter: nfnetlink_queue: fix entry leak in bridge verdict error path

[David Dull]

Hi Hyunwoo,

I reviewed the change and the reasoning looks correct to me.

nfqnl_recv_verdict() dequeues the entry using find_dequeue_entry(), which transfers ownership of the nf_queue_entry to this function. After that point the function becomes responsible for either reinjecting or freeing the entry.

In the PF_BRIDGE path the code calls nfqa_parse_bridge() to parse the VLAN attributes coming from userspace. If the attribute set is malformed (for example NFQA_VLAN present but NFQA_VLAN_TCI missing), nfqa_parse_bridge() returns an error. Before this patch, the function would return immediately in that situation.

Because the entry had already been dequeued, returning directly means the nf_queue_entry object and its associated sk_buff are never released. That also leaves any held references such as net_device and struct net references alive. If a userspace program repeatedly sends malformed verdict messages, this path could leak queue entries and eventually exhaust kernel memory.

Your change fixes this by calling nfqnl_reinject(entry, NF_DROP) before returning. This matches the error handling pattern used elsewhere in the file: once the entry is owned by the verdict handler, it must be reinjected or dropped so the resources are released correctly.

So the logic now becomes:

1. dequeue the entry
2. attempt bridge attribute parsing
3. if parsing fails, explicitly drop the packet via nfqnl_reinject()
4. return the error to the caller

That ensures the queue entry and skb are properly handled even in the malformed attribute case.

The Fixes tag also makes sense since the leak path was introduced when bridge verdict handling started using NFQA_VLAN/NFQA_L2HDR.

Overall the change is small, consistent with the existing reinjection model, and addresses a clear ownership leak in the error path.

Reviewed by : David Dull