All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Ren Wei <n05ec@lzu.edu.cn>
Cc: netfilter-devel@vger.kernel.org, fw@strlen.de, phil@nwl.cc,
	stephane.ml.bryant@gmail.com, yuantan098@gmail.com,
	yifanwucs@gmail.com, tomapufckgml@gmail.com, bird@lzu.edu.cn,
	royenheart@gmail.com
Subject: Re: [PATCH nf 1/1] netfilter: nf_queue: hold bridge skb->dev while queued
Date: Tue, 12 May 2026 12:33:37 +0200	[thread overview]
Message-ID: <agMCAScREzJjke_u@chamomile> (raw)
In-Reply-To: <ca7ee343bbcb44905e1f5b853df2f3a5b7d40548.1778493188.git.royenheart@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]

On Tue, May 12, 2026 at 03:57:25PM +0800, Ren Wei wrote:
> From: Haoze Xie <royenheart@gmail.com>
> 
> br_pass_frame_up() rewrites skb->dev from the ingress port to the bridge
> master before queueing bridge LOCAL_IN packets. NFQUEUE only holds
> references on state.in/out and bridge physdevs, so a queued bridge
> packet can retain a freed bridge master in skb->dev until reinjection.
> 
> When the verdict is reinjected later, br_netif_receive_skb() re-enters
> the receive path with skb->dev still pointing at the freed bridge master,
> triggering a use-after-free.
> 
> Store skb->dev in the queue entry for bridge builds, hold a reference on
> it for the queue lifetime, and use the saved device when dropping queued
> packets during NETDEV_DOWN handling.
> 
> Fixes: ac2863445686 ("netfilter: bridge: add nf_afinfo to enable queuing to userspace")
> Cc: stable@kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
>  include/net/netfilter/nf_queue.h | 1 +
>  net/netfilter/nf_queue.c         | 5 +++++
>  net/netfilter/nfnetlink_queue.c  | 3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h
> index d17035d14d96..1e7eb8e85932 100644
> --- a/include/net/netfilter/nf_queue.h
> +++ b/include/net/netfilter/nf_queue.h
> @@ -17,6 +17,7 @@ struct nf_queue_entry {
>  	unsigned int		id;
>  	unsigned int		hook_index;	/* index in hook_entries->hook[] */
>  #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
> +	struct net_device	*skb_dev;

patch is not correct, this is only fixing it for br_netfilter.

>  	struct net_device	*physin;
>  	struct net_device	*physout;
>  #endif

Maybe normalize this special case with this patch instead? I will
propose it to the bridge maintainer.

It is strange that skb->dev != indev.

I have to take a second look, but I don't a usecase where skb->dev is
used in the netfilter tree can could break.

[-- Attachment #2: fix.patch --]
[-- Type: text/x-diff, Size: 1373 bytes --]

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 2cbae0f9ae1f..6f61e31f51f3 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -26,7 +26,11 @@
 static int
 br_netif_receive_skb(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
+	struct net_device *brdev = BR_INPUT_SKB_CB(skb)->brdev;
+
+	skb->dev = brdev;
 	br_drop_fake_rtable(skb);
+
 	return netif_receive_skb(skb);
 }
 
@@ -57,7 +61,6 @@ static int br_pass_frame_up(struct sk_buff *skb, bool promisc)
 	}
 
 	indev = skb->dev;
-	skb->dev = brdev;
 	skb = br_handle_vlan(br, NULL, vg, skb);
 	if (!skb)
 		return NET_RX_DROP;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 84a180927eb7..ab1e180b5049 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -482,6 +482,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
 			       struct net_bridge_vlan_group *vg,
 			       struct sk_buff *skb)
 {
+	struct net_device *brdev = BR_INPUT_SKB_CB(skb)->brdev;
 	struct pcpu_sw_netstats *stats;
 	struct net_bridge_vlan *v;
 	u16 vid;
@@ -502,7 +503,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
 	 * pass the packet as is.
 	 */
 	if (!v || !br_vlan_should_use(v)) {
-		if ((br->dev->flags & IFF_PROMISC) && skb->dev == br->dev) {
+		if ((br->dev->flags & IFF_PROMISC) && brdev == br->dev) {
 			goto out;
 		} else {
 			kfree_skb(skb);

  reply	other threads:[~2026-05-12 10:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1778493188.git.royenheart@gmail.com>
2026-05-12  7:57 ` [PATCH nf 1/1] netfilter: nf_queue: hold bridge skb->dev while queued Ren Wei
2026-05-12 10:33   ` Pablo Neira Ayuso [this message]
2026-05-12 11:03     ` Pablo Neira Ayuso
2026-05-12 11:24   ` Pablo Neira Ayuso
2026-05-12 11:29     ` Florian Westphal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agMCAScREzJjke_u@chamomile \
    --to=pablo@netfilter.org \
    --cc=bird@lzu.edu.cn \
    --cc=fw@strlen.de \
    --cc=n05ec@lzu.edu.cn \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    --cc=royenheart@gmail.com \
    --cc=stephane.ml.bryant@gmail.com \
    --cc=tomapufckgml@gmail.com \
    --cc=yifanwucs@gmail.com \
    --cc=yuantan098@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.