From: Thomas Martitz <tmartitz-oss@avm.de>
To: Roopa Prabhu <roopa@nvidia.com>,
Nikolay Aleksandrov <razor@blackwall.org>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Johannes Nixdorf <jnixdorf-oss@avm.de>,
Thomas Martitz <tmartitz-oss@avm.de>,
bridge@lists.linux.dev, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: [RFC PATCH net-next] net: bridge: drop packets with a local source
Date: Thu, 19 Sep 2024 10:58:02 +0200 [thread overview]
Message-ID: <20240919085803.105430-1-tmartitz-oss@avm.de> (raw)
Currently, there is only a warning if a packet enters the bridge
that has the bridge's or one port's MAC address as source.
Clearly this indicates a network loop (or even spoofing) so we
generally do not want to process the packet. Therefore, move the check
already done for 802.1x scenarios up and do it unconditionally.
For example, a common scenario we see in the field:
In a accidental network loop scenario, if an IGMP join
loops back to us, it would cause mdb entries to stay indefinitely
even if there's no actual join from the outside. Therefore
this change can effectively prevent multicast storms, at least
for simple loops.
Signed-off-by: Thomas Martitz <tmartitz-oss@avm.de>
---
net/bridge/br_fdb.c | 4 +---
net/bridge/br_input.c | 17 ++++++++++-------
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index ad7a42b505ef..f97203c56394 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -900,9 +900,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
if (likely(fdb)) {
/* attempt to update an entry for a local interface */
if (unlikely(test_bit(BR_FDB_LOCAL, &fdb->flags))) {
- if (net_ratelimit())
- br_warn(br, "received packet on %s with own address as source address (addr:%pM, vlan:%u)\n",
- source->dev->name, addr, vid);
+ return;
} else {
unsigned long now = jiffies;
bool fdb_modified = false;
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index ceaa5a89b947..06db92d03dd3 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -77,7 +77,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
{
struct net_bridge_port *p = br_port_get_rcu(skb->dev);
enum br_pkt_type pkt_type = BR_PKT_UNICAST;
- struct net_bridge_fdb_entry *dst = NULL;
+ struct net_bridge_fdb_entry *fdb_src, *dst = NULL;
struct net_bridge_mcast_port *pmctx;
struct net_bridge_mdb_entry *mdst;
bool local_rcv, mcast_hit = false;
@@ -108,10 +108,14 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
&state, &vlan))
goto out;
- if (p->flags & BR_PORT_LOCKED) {
- struct net_bridge_fdb_entry *fdb_src =
- br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
-
+ fdb_src = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
+ if (fdb_src && test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
+ /* Spoofer or short-curcuit on the network. Drop the packet. */
+ if (net_ratelimit())
+ br_warn(br, "received packet on %s with own address as source address (addr:%pM, vlan:%u)\n",
+ p->dev->name, eth_hdr(skb)->h_source, vid);
+ goto drop;
+ } else if (p->flags & BR_PORT_LOCKED) {
if (!fdb_src) {
/* FDB miss. Create locked FDB entry if MAB is enabled
* and drop the packet.
@@ -120,8 +124,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
br_fdb_update(br, p, eth_hdr(skb)->h_source,
vid, BIT(BR_FDB_LOCKED));
goto drop;
- } else if (READ_ONCE(fdb_src->dst) != p ||
- test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
+ } else if (READ_ONCE(fdb_src->dst) != p) {
/* FDB mismatch. Drop the packet without roaming. */
goto drop;
} else if (test_bit(BR_FDB_LOCKED, &fdb_src->flags)) {
--
2.46.1
next reply other threads:[~2024-09-19 8:59 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-19 8:58 Thomas Martitz [this message]
2024-09-19 10:33 ` Nikolay Aleksandrov
2024-09-19 11:13 ` Thomas Martitz
2024-09-20 6:42 ` Nikolay Aleksandrov
2024-09-20 13:28 ` Thomas Martitz
2024-09-22 18:22 ` Nikolay Aleksandrov
2024-09-23 7:26 ` Thomas Martitz
2024-09-23 13:03 ` Nikolay Aleksandrov
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=20240919085803.105430-1-tmartitz-oss@avm.de \
--to=tmartitz-oss@avm.de \
--cc=20240911125820.471469-1-tmartitz-oss@avm.de \
--cc=bridge@lists.linux.dev \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jnixdorf-oss@avm.de \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=razor@blackwall.org \
--cc=roopa@nvidia.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
all inboxes | Powered by JetHome®