* [PATCH nf 1/1] netfilter: nf_queue: hold bridge skb->dev while queued [not found] <cover.1778493188.git.royenheart@gmail.com> @ 2026-05-12 7:57 ` Ren Wei 2026-05-12 10:33 ` Pablo Neira Ayuso 2026-05-12 11:24 ` Pablo Neira Ayuso 0 siblings, 2 replies; 5+ messages in thread From: Ren Wei @ 2026-05-12 7:57 UTC (permalink / raw) To: netfilter-devel Cc: pablo, fw, phil, stephane.ml.bryant, yuantan098, yifanwucs, tomapufckgml, bird, royenheart, n05ec 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; struct net_device *physin; struct net_device *physout; #endif diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c index a6c81c04b3a5..08da3f4700da 100644 --- a/net/netfilter/nf_queue.c +++ b/net/netfilter/nf_queue.c @@ -67,6 +67,7 @@ static void nf_queue_entry_release_refs(struct nf_queue_entry *entry) nf_queue_sock_put(state->sk); #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) + dev_put(entry->skb_dev); dev_put(entry->physin); dev_put(entry->physout); #endif @@ -106,6 +107,7 @@ bool nf_queue_entry_get_refs(struct nf_queue_entry *entry) dev_hold(state->out); #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) + dev_hold(entry->skb_dev); dev_hold(entry->physin); dev_hold(entry->physout); #endif @@ -207,6 +209,9 @@ static int __nf_queue(struct sk_buff *skb, const struct nf_hook_state *state, .size = sizeof(*entry) + route_key_size, }; +#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) + entry->skb_dev = skb->dev; +#endif __nf_queue_entry_init_physdevs(entry); if (!nf_queue_entry_get_refs(entry)) { diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c index 58304fd1f70f..b64a59bb4ba7 100644 --- a/net/netfilter/nfnetlink_queue.c +++ b/net/netfilter/nfnetlink_queue.c @@ -1206,6 +1206,9 @@ dev_cmp(struct nf_queue_entry *entry, unsigned long ifindex) #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) int physinif, physoutif; + if (entry->skb_dev && entry->skb_dev->ifindex == ifindex) + return 1; + physinif = nf_bridge_get_physinif(entry->skb); physoutif = nf_bridge_get_physoutif(entry->skb); -- 2.53.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH nf 1/1] netfilter: nf_queue: hold bridge skb->dev while queued 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 2026-05-12 11:03 ` Pablo Neira Ayuso 2026-05-12 11:24 ` Pablo Neira Ayuso 1 sibling, 1 reply; 5+ messages in thread From: Pablo Neira Ayuso @ 2026-05-12 10:33 UTC (permalink / raw) To: Ren Wei Cc: netfilter-devel, fw, phil, stephane.ml.bryant, yuantan098, yifanwucs, tomapufckgml, bird, royenheart [-- 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); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH nf 1/1] netfilter: nf_queue: hold bridge skb->dev while queued 2026-05-12 10:33 ` Pablo Neira Ayuso @ 2026-05-12 11:03 ` Pablo Neira Ayuso 0 siblings, 0 replies; 5+ messages in thread From: Pablo Neira Ayuso @ 2026-05-12 11:03 UTC (permalink / raw) To: Ren Wei Cc: netfilter-devel, fw, phil, stephane.ml.bryant, yuantan098, yifanwucs, tomapufckgml, bird, royenheart On Tue, May 12, 2026 at 12:33:37PM +0200, Pablo Neira Ayuso wrote: > 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. Scratch this proposal, it also breaks. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH nf 1/1] netfilter: nf_queue: hold bridge skb->dev while queued 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 @ 2026-05-12 11:24 ` Pablo Neira Ayuso 2026-05-12 11:29 ` Florian Westphal 1 sibling, 1 reply; 5+ messages in thread From: Pablo Neira Ayuso @ 2026-05-12 11:24 UTC (permalink / raw) To: Ren Wei Cc: netfilter-devel, fw, phil, stephane.ml.bryant, yuantan098, yifanwucs, tomapufckgml, bird, royenheart [-- Attachment #1: Type: text/plain, Size: 813 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. Next attempt: Maybe hold reference on skb->dev... [-- Attachment #2: fix.patch --] [-- Type: text/x-diff, Size: 656 bytes --] diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c index a6c81c04b3a5..26a4db5e17d4 100644 --- a/net/netfilter/nf_queue.c +++ b/net/netfilter/nf_queue.c @@ -66,6 +66,7 @@ static void nf_queue_entry_release_refs(struct nf_queue_entry *entry) if (state->sk) nf_queue_sock_put(state->sk); + dev_put(entry->skb->dev); #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) dev_put(entry->physin); dev_put(entry->physout); @@ -104,6 +105,7 @@ bool nf_queue_entry_get_refs(struct nf_queue_entry *entry) dev_hold(state->in); dev_hold(state->out); + dev_hold(entry->skb->dev); #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) dev_hold(entry->physin); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH nf 1/1] netfilter: nf_queue: hold bridge skb->dev while queued 2026-05-12 11:24 ` Pablo Neira Ayuso @ 2026-05-12 11:29 ` Florian Westphal 0 siblings, 0 replies; 5+ messages in thread From: Florian Westphal @ 2026-05-12 11:29 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: Ren Wei, netfilter-devel, phil, stephane.ml.bryant, yuantan098, yifanwucs, tomapufckgml, bird, royenheart Pablo Neira Ayuso <pablo@netfilter.org> wrote: > 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. > > Next attempt: Maybe hold reference on skb->dev... > diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c > index a6c81c04b3a5..26a4db5e17d4 100644 > --- a/net/netfilter/nf_queue.c > +++ b/net/netfilter/nf_queue.c > @@ -66,6 +66,7 @@ static void nf_queue_entry_release_refs(struct nf_queue_entry *entry) > if (state->sk) > nf_queue_sock_put(state->sk); > > + dev_put(entry->skb->dev); > #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) > dev_put(entry->physin); > dev_put(entry->physout); > @@ -104,6 +105,7 @@ bool nf_queue_entry_get_refs(struct nf_queue_entry *entry) > > dev_hold(state->in); > dev_hold(state->out); > + dev_hold(entry->skb->dev); We should also extend net/netfilter/nfnetlink_queue.c:dev_cmp() to consider skb->dev, if set. And I think skb->dev can be NULL here in output path. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-12 11:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[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
2026-05-12 11:03 ` Pablo Neira Ayuso
2026-05-12 11:24 ` Pablo Neira Ayuso
2026-05-12 11:29 ` Florian Westphal
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.