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);
next prev parent 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox