rpms/kernel
9
4
Fork 6
Code Issues Pull Requests Releases Activity
c10
kernel/1166-netfilter-revalidate-bridge-ports.patch

229 lines
7.6 KiB
Diff
Raw Permalink Blame History

From 43330a1e8aace6b5a8de9aba127e9e394ab49b0f Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Tue, 2 Jun 2026 17:04:25 +0200
Subject: [PATCH] netfilter: revalidate bridge ports
[ Upstream commit ccb9fd4b87538ccf19ccff78ee26700526d94867 ]
ebt_redirect_tg() dereferences br_port_get_rcu() return without a
NULL check, causing a kernel panic when the bridge port has been
removed between the original hook invocation and an NFQUEUE
reinject.
A mere NULL check isn't sufficient, however. As sashiko review
points out userspace can not only remove the port from the bridge,
it could also place the device in a different virtual device, e.g.
macvlan.
If this happens, we must drop the packet, there is no way for us to
reinject it into the bridge path.
Switch to _upper API, we don't need the bridge port structure.
Also, this fix keeps another bug intact:
Both nfnetlink_log and nfnetlink_queue use CONFIG_BRIDGE_NETFILTER
too aggressive, which prevents certain logging features when queueing
in bridge family: NETFILTER_FAMILY_BRIDGE can be enabled while the old
CONFIG_BRIDGE_NETFILTER cruft is off.
Fixes tag is a common ancestor, this was always broken.
Fixes: f350a0a87374 ("bridge: use rx_handler_data pointer to store net_bridge_port pointer")
Reported-by: Ji'an Zhou <eilaimemedsnaimel@gmail.com>
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
diff --git a/net/bridge/netfilter/ebt_dnat.c b/net/bridge/netfilter/ebt_dnat.c
index 3fda71a..73f185c 100644
--- a/net/bridge/netfilter/ebt_dnat.c
+++ b/net/bridge/netfilter/ebt_dnat.c
@@ -39,7 +39,9 @@ ebt_dnat_tg(struct sk_buff *skb, const struct xt_action_param *par)
dev = xt_in(par);
break;
case NF_BR_PRE_ROUTING:
- dev = br_port_get_rcu(xt_in(par))->br->dev;
+ dev = netdev_master_upper_dev_get_rcu(xt_in(par));
+ if (!dev) /* bridge port removed? */
+ return EBT_DROP;
break;
default:
dev = NULL;
diff --git a/net/bridge/netfilter/ebt_redirect.c b/net/bridge/netfilter/ebt_redirect.c
index 3077905..83486cd 100644
--- a/net/bridge/netfilter/ebt_redirect.c
+++ b/net/bridge/netfilter/ebt_redirect.c
@@ -24,12 +24,18 @@ ebt_redirect_tg(struct sk_buff *skb, const struct xt_action_param *par)
if (skb_ensure_writable(skb, 0))
return EBT_DROP;
- if (xt_hooknum(par) != NF_BR_BROUTING)
- /* rcu_read_lock()ed by nf_hook_thresh */
- ether_addr_copy(eth_hdr(skb)->h_dest,
- br_port_get_rcu(xt_in(par))->br->dev->dev_addr);
- else
+ if (xt_hooknum(par) != NF_BR_BROUTING) {
+ const struct net_device *dev;
+
+ dev = netdev_master_upper_dev_get_rcu(xt_in(par));
+ if (!dev)
+ return EBT_DROP;
+
+ ether_addr_copy(eth_hdr(skb)->h_dest, dev->dev_addr);
+ } else {
ether_addr_copy(eth_hdr(skb)->h_dest, xt_in(par)->dev_addr);
+ }
+
skb->pkt_type = PACKET_HOST;
return info->target;
}
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index b1f3eda..25a30bf 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -450,6 +450,23 @@ static int nfulnl_put_bridge(struct nfulnl_instance *inst, const struct sk_buff
return -1;
}
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
+static int nflog_put_master_ifindex(struct sk_buff *nlskb, int attr,
+ const struct net_device *dev)
+{
+ const struct net_device *upper;
+
+ if (dev && !netif_is_bridge_port(dev))
+ return 0;
+
+ upper = netdev_master_upper_dev_get_rcu((struct net_device *)dev);
+ if (upper && nla_put_be32(nlskb, attr, htonl(upper->ifindex)))
+ return -EMSGSIZE;
+
+ return 0;
+}
+#endif
+
/* This is an inline function, we don't really care about a long
* list of arguments */
static inline int
@@ -504,8 +521,7 @@ __build_packet_message(struct nfnl_log_net *log,
/* rcu_read_lock()ed by nf_hook_thresh or
* nf_log_packet.
*/
- nla_put_be32(inst->skb, NFULA_IFINDEX_INDEV,
- htonl(br_port_get_rcu(indev)->br->dev->ifindex)))
+ nflog_put_master_ifindex(inst->skb, NFULA_IFINDEX_INDEV, indev))
goto nla_put_failure;
} else {
int physinif;
@@ -541,8 +557,7 @@ __build_packet_message(struct nfnl_log_net *log,
/* rcu_read_lock()ed by nf_hook_thresh or
* nf_log_packet.
*/
- nla_put_be32(inst->skb, NFULA_IFINDEX_OUTDEV,
- htonl(br_port_get_rcu(outdev)->br->dev->ifindex)))
+ nflog_put_master_ifindex(inst->skb, NFULA_IFINDEX_OUTDEV, outdev))
goto nla_put_failure;
} else {
struct net_device *physoutdev;
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 8d4fa10..87ac5c8 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -369,10 +369,47 @@ static void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
nf_queue_entry_free(entry);
}
+static bool nf_bridge_port_valid(const struct net_device *dev)
+{
+ if (!dev)
+ return true;
+
+ return netif_is_bridge_port(dev);
+}
+
+/* queued skbs leave rcu protection. We bump device refcount so that
+ * the device cannot go away. However, while packet was out the port
+ * could have been removed from the bridge.
+ *
+ * Ensure in+outdev are still part of a bridge at reinject time.
+ *
+ * The device rx_handler_data could even be pointing at data that is
+ * not a net_bridge_port structure.
+ */
+static bool nf_bridge_ports_valid(const struct nf_queue_entry *entry)
+{
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
+ if (!nf_bridge_port_valid(entry->physin) ||
+ !nf_bridge_port_valid(entry->physout))
+ return false;
+#endif
+ if (entry->state.pf != PF_BRIDGE)
+ return true;
+
+ if (!nf_bridge_port_valid(entry->state.in) ||
+ !nf_bridge_port_valid(entry->state.out))
+ return false;
+
+ return true;
+}
+
static void nfqnl_reinject(struct nf_queue_entry *entry, unsigned int verdict)
{
const struct nf_ct_hook *ct_hook;
+ if (!nf_bridge_ports_valid(entry))
+ verdict = NF_DROP;
+
if (verdict == NF_ACCEPT ||
verdict == NF_REPEAT ||
verdict == NF_STOP) {
@@ -548,6 +585,23 @@ static int nf_queue_checksum_help(struct sk_buff *entskb)
return skb_checksum_help(entskb);
}
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
+static int nfqnl_put_master_ifindex(struct sk_buff *nlskb, int attr,
+ const struct net_device *dev)
+{
+ const struct net_device *upper;
+
+ if (dev && !netif_is_bridge_port(dev))
+ return 0;
+
+ upper = netdev_master_upper_dev_get_rcu((struct net_device *)dev);
+ if (upper && nla_put_be32(nlskb, attr, htonl(upper->ifindex)))
+ return -EMSGSIZE;
+
+ return 0;
+}
+#endif
+
static struct sk_buff *
nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
struct nf_queue_entry *entry,
@@ -681,10 +735,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
* netfilter_bridge) */
if (nla_put_be32(skb, NFQA_IFINDEX_PHYSINDEV,
htonl(indev->ifindex)) ||
- /* this is the bridge group "brX" */
- /* rcu_read_lock()ed by __nf_queue */
- nla_put_be32(skb, NFQA_IFINDEX_INDEV,
- htonl(br_port_get_rcu(indev)->br->dev->ifindex)))
+ nfqnl_put_master_ifindex(skb, NFQA_IFINDEX_INDEV, indev))
goto nla_put_failure;
} else {
int physinif;
@@ -715,10 +766,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
* netfilter_bridge) */
if (nla_put_be32(skb, NFQA_IFINDEX_PHYSOUTDEV,
htonl(outdev->ifindex)) ||
- /* this is the bridge group "brX" */
- /* rcu_read_lock()ed by __nf_queue */
- nla_put_be32(skb, NFQA_IFINDEX_OUTDEV,
- htonl(br_port_get_rcu(outdev)->br->dev->ifindex)))
+ nfqnl_put_master_ifindex(skb, NFQA_IFINDEX_OUTDEV, outdev))
goto nla_put_failure;
} else {
int physoutif;
Reference in New Issue View Git Blame Copy Permalink