From: "Marek Behún" <kabel@kernel.org>
To: Vladimir Oltean <vladimir.oltean@nxp.com>,
Tobias Waldekranz <tobias@waldekranz.com>
Cc: netdev@vger.kernel.org, "Andrew Lunn" <andrew@lunn.ch>,
"Marek Behún" <kabel@kernel.org>, "Jan Bětík" <hagrid@svine.us>
Subject: [PATCH net] net: dsa: fix panic when port leaves a bridge
Date: Mon, 14 Mar 2022 16:34:10 +0100 [thread overview]
Message-ID: <20220314153410.31744-1-kabel@kernel.org> (raw)
Fix a data structure breaking / NULL-pointer dereference in
dsa_switch_bridge_leave().
When a DSA port leaves a bridge, dsa_switch_bridge_leave() is called by
notifier for every DSA switch that contains ports that are in the
bridge.
But the part of the code that unsets vlan_filtering expects that the ds
argument refers to the same switch that contains the leaving port.
This leads to various problems, including a NULL pointer dereference,
which was observed on Turris MOX with 2 switches (one with 8 user ports
and another with 4 user ports).
Thus we need to move the vlan_filtering change code to the non-crosschip
branch.
Fixes: d371b7c92d190 ("net: dsa: Unset vlan_filtering when ports leave the bridge")
Reported-by: Jan Bětík <hagrid@svine.us>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
net/dsa/switch.c | 97 +++++++++++++++++++++++++++---------------------
1 file changed, 55 insertions(+), 42 deletions(-)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index e3c7d2627a61..38afb1e8fcb7 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -123,9 +123,61 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
struct dsa_port *dp;
int err;
- if (dst->index == info->tree_index && ds->index == info->sw_index &&
- ds->ops->port_bridge_leave)
- ds->ops->port_bridge_leave(ds, info->port, info->bridge);
+ if (dst->index == info->tree_index && ds->index == info->sw_index) {
+ if (ds->ops->port_bridge_leave)
+ ds->ops->port_bridge_leave(ds, info->port,
+ info->bridge);
+
+ /* This function is called by the notifier for every DSA switch
+ * that has ports in the bridge we are leaving, but vlan
+ * filtering on the port should be changed only once. Since the
+ * code expects that ds is the switch containing the leaving
+ * port, the following code must not be called in the crosschip
+ * branch, only here.
+ */
+
+ if (ds->needs_standalone_vlan_filtering &&
+ !br_vlan_enabled(info->bridge.dev)) {
+ change_vlan_filtering = true;
+ vlan_filtering = true;
+ } else if (!ds->needs_standalone_vlan_filtering &&
+ br_vlan_enabled(info->bridge.dev)) {
+ change_vlan_filtering = true;
+ vlan_filtering = false;
+ }
+
+ /* If the bridge was vlan_filtering, the bridge core doesn't
+ * trigger an event for changing vlan_filtering setting upon
+ * slave ports leaving it. That is a good thing, because that
+ * lets us handle it and also handle the case where the switch's
+ * vlan_filtering setting is global (not per port). When that
+ * happens, the correct moment to trigger the vlan_filtering
+ * callback is only when the last port leaves the last
+ * VLAN-aware bridge.
+ */
+ if (change_vlan_filtering && ds->vlan_filtering_is_global) {
+ dsa_switch_for_each_port(dp, ds) {
+ struct net_device *br =
+ dsa_port_bridge_dev_get(dp);
+
+ if (br && br_vlan_enabled(br)) {
+ change_vlan_filtering = false;
+ break;
+ }
+ }
+ }
+
+ if (change_vlan_filtering) {
+ err = dsa_port_vlan_filtering(dsa_to_port(ds,
+ info->port),
+ vlan_filtering, &extack);
+ if (extack._msg)
+ dev_err(ds->dev, "port %d: %s\n", info->port,
+ extack._msg);
+ if (err && err != -EOPNOTSUPP)
+ return err;
+ }
+ }
if ((dst->index != info->tree_index || ds->index != info->sw_index) &&
ds->ops->crosschip_bridge_leave)
@@ -133,45 +185,6 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds,
info->sw_index, info->port,
info->bridge);
- if (ds->needs_standalone_vlan_filtering &&
- !br_vlan_enabled(info->bridge.dev)) {
- change_vlan_filtering = true;
- vlan_filtering = true;
- } else if (!ds->needs_standalone_vlan_filtering &&
- br_vlan_enabled(info->bridge.dev)) {
- change_vlan_filtering = true;
- vlan_filtering = false;
- }
-
- /* If the bridge was vlan_filtering, the bridge core doesn't trigger an
- * event for changing vlan_filtering setting upon slave ports leaving
- * it. That is a good thing, because that lets us handle it and also
- * handle the case where the switch's vlan_filtering setting is global
- * (not per port). When that happens, the correct moment to trigger the
- * vlan_filtering callback is only when the last port leaves the last
- * VLAN-aware bridge.
- */
- if (change_vlan_filtering && ds->vlan_filtering_is_global) {
- dsa_switch_for_each_port(dp, ds) {
- struct net_device *br = dsa_port_bridge_dev_get(dp);
-
- if (br && br_vlan_enabled(br)) {
- change_vlan_filtering = false;
- break;
- }
- }
- }
-
- if (change_vlan_filtering) {
- err = dsa_port_vlan_filtering(dsa_to_port(ds, info->port),
- vlan_filtering, &extack);
- if (extack._msg)
- dev_err(ds->dev, "port %d: %s\n", info->port,
- extack._msg);
- if (err && err != -EOPNOTSUPP)
- return err;
- }
-
return dsa_tag_8021q_bridge_leave(ds, info);
}
--
2.34.1
next reply other threads:[~2022-03-14 15:34 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-14 15:34 Marek Behún [this message]
2022-03-14 15:41 ` [PATCH net] net: dsa: fix panic when port leaves a bridge Vladimir Oltean
2022-03-14 15:45 ` Vladimir Oltean
2022-03-14 16:06 ` Marek Behún
2022-03-14 15:48 ` Tobias Waldekranz
2022-03-14 16:05 ` Marek Behún
2022-03-14 16:17 ` Vladimir Oltean
2022-03-14 16:34 ` Marek Behún
2022-03-14 16:42 ` Vladimir Oltean
2022-03-15 0:04 ` Jakub Kicinski
2022-03-14 16:47 ` Marek Behún
2022-03-14 18:09 ` Marek Behún
2022-03-14 18:19 ` Tobias Waldekranz
2022-03-14 19:27 ` Marek Behún
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=20220314153410.31744-1-kabel@kernel.org \
--to=kabel@kernel.org \
--cc=andrew@lunn.ch \
--cc=hagrid@svine.us \
--cc=netdev@vger.kernel.org \
--cc=tobias@waldekranz.com \
--cc=vladimir.oltean@nxp.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;
as well as URLs for NNTP newsgroup(s).