Skip to content

Commit 430521a

Browse files
royenheartgregkh
authored andcommitted
netfilter: nf_queue: pin bridge device while NFQUEUE holds fake dst
commit c9c9b37 upstream. The br_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: 34666d4 ("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> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 4cb8b5f commit 430521a

3 files changed

Lines changed: 18 additions & 0 deletions

File tree

include/net/netfilter/nf_queue.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ struct nf_queue_entry {
1616
unsigned int id;
1717
unsigned int hook_index; /* index in hook_entries->hook[] */
1818
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
19+
struct net_device *bridge_dev;
1920
struct net_device *physin;
2021
struct net_device *physout;
2122
#endif

net/netfilter/nf_queue.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ static void nf_queue_entry_release_refs(struct nf_queue_entry *entry)
6767
nf_queue_sock_put(state->sk);
6868

6969
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
70+
dev_put(entry->bridge_dev);
7071
dev_put(entry->physin);
7172
dev_put(entry->physout);
7273
#endif
@@ -83,6 +84,8 @@ static void __nf_queue_entry_init_physdevs(struct nf_queue_entry *entry)
8384
{
8485
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
8586
const struct sk_buff *skb = entry->skb;
87+
struct dst_entry *dst = skb_dst(skb);
88+
struct net_device *dev = NULL;
8689

8790
if (nf_bridge_info_exists(skb)) {
8891
entry->physin = nf_bridge_get_physindev(skb, entry->state.net);
@@ -91,6 +94,16 @@ static void __nf_queue_entry_init_physdevs(struct nf_queue_entry *entry)
9194
entry->physin = NULL;
9295
entry->physout = NULL;
9396
}
97+
98+
if (entry->state.pf == NFPROTO_BRIDGE &&
99+
dst && (dst->flags & DST_FAKE_RTABLE))
100+
dev = dst_dev_rcu(dst);
101+
102+
/* Must hold a reference on the bridge device: dst_hold() protects
103+
* the dst itself, but the fake rtable is embedded in bridge-private
104+
* storage that netdevice teardown can free independently.
105+
*/
106+
entry->bridge_dev = dev;
94107
#endif
95108
}
96109

@@ -107,6 +120,7 @@ bool nf_queue_entry_get_refs(struct nf_queue_entry *entry)
107120
dev_hold(state->out);
108121

109122
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
123+
dev_hold(entry->bridge_dev);
110124
dev_hold(entry->physin);
111125
dev_hold(entry->physout);
112126
#endif

net/netfilter/nfnetlink_queue.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,9 @@ dev_cmp(struct nf_queue_entry *entry, unsigned long ifindex)
950950

951951
if (physinif == ifindex || physoutif == ifindex)
952952
return 1;
953+
954+
if (entry->bridge_dev && entry->bridge_dev->ifindex == ifindex)
955+
return 1;
953956
#endif
954957
if (entry->skb_dev && entry->skb_dev->ifindex == ifindex)
955958
return 1;

0 commit comments

Comments
 (0)