| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20220314153410.31744-1-kabel@kernel.org>
Date: Mon, 14 Mar 2022 16:34:10 +0100
From: Marek Behún <kabel@...nel.org>
To: Vladimir Oltean <vladimir.oltean@....com>,
Tobias Waldekranz <tobias@...dekranz.com>
Cc: netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
Marek Behún <kabel@...nel.org>,
Jan Bětík <hagrid@...ne.us>
Subject: [PATCH net] net: dsa: fix panic when port leaves a bridge
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@...ne.us>
Signed-off-by: Marek Behún <kabel@...nel.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
Powered by blists - more mailing lists