* [PATCH nf v5 1/1] bridge: br_netfilter: pin bridge device while NFQUEUE holds fake dst
@ 2026-06-08 5:43 Ren Wei
2026-07-04 5:25 ` Haoze Xie
0 siblings, 1 reply; 4+ messages in thread
From: Ren Wei @ 2026-06-08 5:43 UTC (permalink / raw)
To: netfilter-devel
Cc: pablo, fw, phil, yuantan098, yifanwucs, tomapufckgml, bird,
royenheart, n05ec
From: Haoze Xie <royenheart@gmail.com>
The bridge netfilter fake rtable is embedded in struct net_bridge and is
attached to bridged packets with skb_dst_set_noref(). If such a packet is
queued to NFQUEUE, __nf_queue() upgrades that fake dst with
skb_dst_force().
At that point the queued skb can hold a real dst reference after bridge
teardown has started. The problem is not that every bridged packet needs
its own dst reference. The problem is that NFQUEUE can keep the bridge
private fake dst alive after unregister begins.
Fix this by keeping the bridge fake dst model unchanged and pinning the
bridge master device only while the packet sits in NFQUEUE. Record the
bridge device in nf_queue_entry when the queued skb carries a bridge fake
dst, take a device reference for the queue lifetime, and drop it when the
queue entry is freed.
Also make sure queued entries are reaped when that bridge device goes
down, and drop the redundant nf_bridge_info_exists() test from the fake
dst detection.
This keeps netdev_priv(br->dev) alive until verdict completion, so the
embedded fake rtable and its metrics backing storage cannot be freed out
from under dst_release(). It also avoids the constant refcount bump and
avoids using ipv4-specific dst helpers for IPv6 bridge traffic.
Fixes: 34666d467cbf ("netfilter: bridge: move br_netfilter out of the core")
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>
Signed-off-by: Haoze Xie <royenheart@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
Changes in v5:
- drop the redundant nf_bridge_info_exists() test in the fake-dst bridge
device lookup
- teach dev_cmp() to reap queued entries that hold the bridge device
reference when that device goes down
- v4 Link: https://lore.kernel.org/all/cbc3a29c0654e8fcee30cb021d57883fed77fafc.1780630094.git.royenheart@gmail.com/
Changes in v4:
- inline the bridge fake-dst device lookup into
__nf_queue_entry_init_physdevs()
- drop the extra helper introduced in v3 and keep the queue-entry setup
local
- use dst_dev_rcu() as suggested during review
- drop the unnecessary blackhole_netdev special case
- expand the comment to state explicitly that dst_hold() cannot protect
the embedded fake rtable backing storage
- v3 Link: https://lore.kernel.org/all/fe4fc3d462679ba10bf85e574921ecf861000d66.1780590147.git.royenheart@gmail.com/
Changes in v3:
- drop the per-packet fake dst refcounting from v2
- stop using ipv4-specific dst helpers for the fake dst
- keep the existing bridge fake rtable model unchanged on the fast path
- pin the bridge master device only when NFQUEUE upgrades a fake dst
into an asynchronous queued reference
- v2 Link: https://lore.kernel.org/all/831936f111e6e1f435f4f6247d07fe6a6624d271.1779680014.git.royenheart@gmail.com/
changes in v2:
- spell out how NFQUEUE upgrades the fake dst into a real reference
- switch to rt_dst_alloc() instead of br_netfilter-private dst_ops state
- detach the bridge device with dst_dev_put() during teardown
- keep the ref-holding contract local to bridge_parent_rtable()
- v1 Link: https://lore.kernel.org/all/783d76ac83917b7302c1ec647794bd773bb1875a.1778687139.git.royenheart@gmail.com/
include/net/netfilter/nf_queue.h | 1 +
net/netfilter/nf_queue.c | 14 ++++++++++++++
net/netfilter/nfnetlink_queue.c | 3 +++
3 files changed, 18 insertions(+)
diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h
index 3978c3174cdb..fc3e81c07364 100644
--- a/include/net/netfilter/nf_queue.h
+++ b/include/net/netfilter/nf_queue.h
@@ -18,6 +18,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 *bridge_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 57b450024a99..73363ceedebe 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -68,6 +68,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->bridge_dev);
dev_put(entry->physin);
dev_put(entry->physout);
#endif
@@ -84,6 +85,8 @@ static void __nf_queue_entry_init_physdevs(struct nf_queue_entry *entry)
{
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
const struct sk_buff *skb = entry->skb;
+ struct dst_entry *dst = skb_dst(skb);
+ struct net_device *dev = NULL;
if (nf_bridge_info_exists(skb)) {
entry->physin = nf_bridge_get_physindev(skb, entry->state.net);
@@ -92,6 +95,16 @@ static void __nf_queue_entry_init_physdevs(struct nf_queue_entry *entry)
entry->physin = NULL;
entry->physout = NULL;
}
+
+ if (entry->state.pf == NFPROTO_BRIDGE &&
+ dst && (dst->flags & DST_FAKE_RTABLE))
+ dev = dst_dev_rcu(dst);
+
+ /* Must hold a reference on the bridge device: dst_hold() protects
+ * the dst itself, but the fake rtable is embedded in bridge-private
+ * storage that netdevice teardown can free independently.
+ */
+ entry->bridge_dev = dev;
#endif
}
@@ -108,6 +121,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->bridge_dev);
dev_hold(entry->physin);
dev_hold(entry->physout);
#endif
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 60ab88d45096..1c73c511a682 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -1214,6 +1214,9 @@ dev_cmp(struct nf_queue_entry *entry, unsigned long ifindex)
if (physinif == ifindex || physoutif == ifindex)
return 1;
+
+ if (entry->bridge_dev && entry->bridge_dev->ifindex == ifindex)
+ return 1;
#endif
if (entry->skb_dev && entry->skb_dev->ifindex == ifindex)
return 1;
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nf v5 1/1] bridge: br_netfilter: pin bridge device while NFQUEUE holds fake dst
2026-06-08 5:43 [PATCH nf v5 1/1] bridge: br_netfilter: pin bridge device while NFQUEUE holds fake dst Ren Wei
@ 2026-07-04 5:25 ` Haoze Xie
2026-07-04 6:27 ` Florian Westphal
0 siblings, 1 reply; 4+ messages in thread
From: Haoze Xie @ 2026-07-04 5:25 UTC (permalink / raw)
To: Florian Westphal
Cc: pablo, phil, yuantan098, yifanwucs, tomapufckgml, bird,
netfilter-devel, Ren Wei
On 6/8/2026 1:43 PM, Ren Wei wrote:
> From: Haoze Xie <royenheart@gmail.com>
>
> The bridge netfilter fake rtable is embedded in struct net_bridge and is
> attached to bridged packets with skb_dst_set_noref(). If such a packet is
> queued to NFQUEUE, __nf_queue() upgrades that fake dst with
> skb_dst_force().
>
> At that point the queued skb can hold a real dst reference after bridge
> teardown has started. The problem is not that every bridged packet needs
> its own dst reference. The problem is that NFQUEUE can keep the bridge
> private fake dst alive after unregister begins.
>
> Fix this by keeping the bridge fake dst model unchanged and pinning the
> bridge master device only while the packet sits in NFQUEUE. Record the
> bridge device in nf_queue_entry when the queued skb carries a bridge fake
> dst, take a device reference for the queue lifetime, and drop it when the
> queue entry is freed.
>
> Also make sure queued entries are reaped when that bridge device goes
> down, and drop the redundant nf_bridge_info_exists() test from the fake
> dst detection.
>
> This keeps netdev_priv(br->dev) alive until verdict completion, so the
> embedded fake rtable and its metrics backing storage cannot be freed out
> from under dst_release(). It also avoids the constant refcount bump and
> avoids using ipv4-specific dst helpers for IPv6 bridge traffic.
>
> Fixes: 34666d467cbf ("netfilter: bridge: move br_netfilter out of the core")
> 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>
> Signed-off-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> Changes in v5:
> - drop the redundant nf_bridge_info_exists() test in the fake-dst bridge
> device lookup
> - teach dev_cmp() to reap queued entries that hold the bridge device
> reference when that device goes down
> - v4 Link: https://lore.kernel.org/all/cbc3a29c0654e8fcee30cb021d57883fed77fafc.1780630094.git.royenheart@gmail.com/
> Changes in v4:
> - inline the bridge fake-dst device lookup into
> __nf_queue_entry_init_physdevs()
> - drop the extra helper introduced in v3 and keep the queue-entry setup
> local
> - use dst_dev_rcu() as suggested during review
> - drop the unnecessary blackhole_netdev special case
> - expand the comment to state explicitly that dst_hold() cannot protect
> the embedded fake rtable backing storage
> - v3 Link: https://lore.kernel.org/all/fe4fc3d462679ba10bf85e574921ecf861000d66.1780590147.git.royenheart@gmail.com/
> Changes in v3:
> - drop the per-packet fake dst refcounting from v2
> - stop using ipv4-specific dst helpers for the fake dst
> - keep the existing bridge fake rtable model unchanged on the fast path
> - pin the bridge master device only when NFQUEUE upgrades a fake dst
> into an asynchronous queued reference
> - v2 Link: https://lore.kernel.org/all/831936f111e6e1f435f4f6247d07fe6a6624d271.1779680014.git.royenheart@gmail.com/
> changes in v2:
> - spell out how NFQUEUE upgrades the fake dst into a real reference
> - switch to rt_dst_alloc() instead of br_netfilter-private dst_ops state
> - detach the bridge device with dst_dev_put() during teardown
> - keep the ref-holding contract local to bridge_parent_rtable()
> - v1 Link: https://lore.kernel.org/all/783d76ac83917b7302c1ec647794bd773bb1875a.1778687139.git.royenheart@gmail.com/
>
> include/net/netfilter/nf_queue.h | 1 +
> net/netfilter/nf_queue.c | 14 ++++++++++++++
> net/netfilter/nfnetlink_queue.c | 3 +++
> 3 files changed, 18 insertions(+)
>
> diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h
> index 3978c3174cdb..fc3e81c07364 100644
> --- a/include/net/netfilter/nf_queue.h
> +++ b/include/net/netfilter/nf_queue.h
> @@ -18,6 +18,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 *bridge_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 57b450024a99..73363ceedebe 100644
> --- a/net/netfilter/nf_queue.c
> +++ b/net/netfilter/nf_queue.c
> @@ -68,6 +68,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->bridge_dev);
> dev_put(entry->physin);
> dev_put(entry->physout);
> #endif
> @@ -84,6 +85,8 @@ static void __nf_queue_entry_init_physdevs(struct nf_queue_entry *entry)
> {
> #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
> const struct sk_buff *skb = entry->skb;
> + struct dst_entry *dst = skb_dst(skb);
> + struct net_device *dev = NULL;
>
> if (nf_bridge_info_exists(skb)) {
> entry->physin = nf_bridge_get_physindev(skb, entry->state.net);
> @@ -92,6 +95,16 @@ static void __nf_queue_entry_init_physdevs(struct nf_queue_entry *entry)
> entry->physin = NULL;
> entry->physout = NULL;
> }
> +
> + if (entry->state.pf == NFPROTO_BRIDGE &&
> + dst && (dst->flags & DST_FAKE_RTABLE))
> + dev = dst_dev_rcu(dst);
> +
> + /* Must hold a reference on the bridge device: dst_hold() protects
> + * the dst itself, but the fake rtable is embedded in bridge-private
> + * storage that netdevice teardown can free independently.
> + */
> + entry->bridge_dev = dev;
> #endif
> }
>
> @@ -108,6 +121,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->bridge_dev);
> dev_hold(entry->physin);
> dev_hold(entry->physout);
> #endif
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index 60ab88d45096..1c73c511a682 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -1214,6 +1214,9 @@ dev_cmp(struct nf_queue_entry *entry, unsigned long ifindex)
>
> if (physinif == ifindex || physoutif == ifindex)
> return 1;
> +
> + if (entry->bridge_dev && entry->bridge_dev->ifindex == ifindex)
> + return 1;
> #endif
> if (entry->skb_dev && entry->skb_dev->ifindex == ifindex)
> return 1;
Hi, is there any follow up about this patch?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf v5 1/1] bridge: br_netfilter: pin bridge device while NFQUEUE holds fake dst
2026-07-04 5:25 ` Haoze Xie
@ 2026-07-04 6:27 ` Florian Westphal
2026-07-04 7:50 ` Yuan Tan
0 siblings, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2026-07-04 6:27 UTC (permalink / raw)
To: Haoze Xie
Cc: pablo, phil, yuantan098, yifanwucs, tomapufckgml, bird,
netfilter-devel, Ren Wei
Haoze Xie <royenheart@gmail.com> wrote:
> > --- a/net/netfilter/nfnetlink_queue.c
> > +++ b/net/netfilter/nfnetlink_queue.c
> > @@ -1214,6 +1214,9 @@ dev_cmp(struct nf_queue_entry *entry, unsigned long ifindex)
> >
> > if (physinif == ifindex || physoutif == ifindex)
> > return 1;
> > +
> > + if (entry->bridge_dev && entry->bridge_dev->ifindex == ifindex)
> > + return 1;
> > #endif
> > if (entry->skb_dev && entry->skb_dev->ifindex == ifindex)
> > return 1;
>
> Hi, is there any follow up about this patch?
What do you mean, can you elaborate?
What is wrong with
c9c9b37f8c55 ("netfilter: nf_queue: pin bridge device while NFQUEUE holds fake dst")
If the patch is not sufficient, please submit a followup.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf v5 1/1] bridge: br_netfilter: pin bridge device while NFQUEUE holds fake dst
2026-07-04 6:27 ` Florian Westphal
@ 2026-07-04 7:50 ` Yuan Tan
0 siblings, 0 replies; 4+ messages in thread
From: Yuan Tan @ 2026-07-04 7:50 UTC (permalink / raw)
To: Florian Westphal
Cc: Haoze Xie, pablo, phil, yifanwucs, tomapufckgml, bird,
netfilter-devel, Ren Wei
On Fri, Jul 3, 2026 at 11:27 PM Florian Westphal <fw@strlen.de> wrote:
>
> Haoze Xie <royenheart@gmail.com> wrote:
> > > --- a/net/netfilter/nfnetlink_queue.c
> > > +++ b/net/netfilter/nfnetlink_queue.c
> > > @@ -1214,6 +1214,9 @@ dev_cmp(struct nf_queue_entry *entry, unsigned long ifindex)
> > >
> > > if (physinif == ifindex || physoutif == ifindex)
> > > return 1;
> > > +
> > > + if (entry->bridge_dev && entry->bridge_dev->ifindex == ifindex)
> > > + return 1;
> > > #endif
> > > if (entry->skb_dev && entry->skb_dev->ifindex == ifindex)
> > > return 1;
> >
> > Hi, is there any follow up about this patch?
>
> What do you mean, can you elaborate?
>
> What is wrong with
>
> c9c9b37f8c55 ("netfilter: nf_queue: pin bridge device while NFQUEUE holds fake dst")
>
> If the patch is not sufficient, please submit a followup.
Sorry, we thought this patch had not been reviewed or merged.
The patch is good and we just notice this patch has already been merged:
https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git/commit/?id=c9c9b37f8c5505224e8d206184df3bb668ee00cf
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-04 7:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-08 5:43 [PATCH nf v5 1/1] bridge: br_netfilter: pin bridge device while NFQUEUE holds fake dst Ren Wei
2026-07-04 5:25 ` Haoze Xie
2026-07-04 6:27 ` Florian Westphal
2026-07-04 7:50 ` Yuan Tan
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.