| 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
| ||
|
Date: Thu, 18 Aug 2022 18:49:03 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: netdev@...r.kernel.org
Cc: Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Kevin Hilman <khilman@...nel.org>,
Ulf Hansson <ulf.hansson@...aro.org>,
Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: [RFC PATCH net-next 02/10] net: dsa: introduce and use robust form of dsa_tree_notify()
Cross-chip notifier chains being broken mid-way by a switch returning an
error is a recurring problem, and some callers of dsa_tree_notify() and
derivatives (dsa_port_notify, dsa_broadcast) deal with this more
gracefully than others.
Not dealing gracefully means not doing anything (and letting the tree
have an inconsistent state), while dealing "gracefully" means emitting
one more notifier which contains the old state. However, even this has
its own potential problems, since in some cases, switch drivers do not
expect to receive a call that requests a state change to their existing
state - see commit 0c4e31c0722a ("net: dsa: felix: suppress non-changes
to the tagging protocol").
The right thing would be to roll back the switches that succeeded,
and leave the one that failed, and the ones where the notifier did not
execute, alone. This is achieved using dsa_tree_notify_robust().
Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
net/dsa/dsa2.c | 56 +++++++++++++++++++++++++++++-----------------
net/dsa/dsa_priv.h | 2 ++
2 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index cac48a741f27..50b87419342f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -46,6 +46,28 @@ int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v)
return notifier_to_errno(err);
}
+/**
+ * dsa_tree_notify_robust - Run code for all switches in a tree, with rollback.
+ * @dst: collection of struct dsa_switch devices to notify.
+ * @e: event, must be of type DSA_NOTIFIER_*
+ * @v: event-specific value.
+ * @e_rollback: event, must be of type DSA_NOTIFIER_*
+ * @v_rollback: event-specific value.
+ *
+ * Like dsa_tree_notify(), except makes sure that switches are restored to the
+ * previous state in case the notifier call chain fails mid way.
+ */
+int dsa_tree_notify_robust(struct dsa_switch_tree *dst, unsigned long e,
+ void *v, unsigned long e_rollback, void *v_rollback)
+{
+ struct raw_notifier_head *nh = &dst->nh;
+ int err;
+
+ err = raw_notifier_call_chain_robust(nh, e, e_rollback, v, v_rollback);
+
+ return notifier_to_errno(err);
+}
+
/**
* dsa_broadcast - Notify all DSA trees in the system.
* @e: event, must be of type DSA_NOTIFIER_*
@@ -1215,22 +1237,18 @@ static int dsa_tree_bind_tag_proto(struct dsa_switch_tree *dst,
* to the new tagger
*/
info.tag_ops = tag_ops;
- err = dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_CONNECT, &info);
- if (err && err != -EOPNOTSUPP)
- goto out_disconnect;
+ err = dsa_tree_notify_robust(dst, DSA_NOTIFIER_TAG_PROTO_CONNECT, &info,
+ DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
+ if (err && err != -EOPNOTSUPP) {
+ dst->tag_ops = old_tag_ops;
+ return err;
+ }
/* Notify the old tagger about the disconnection from this tree */
info.tag_ops = old_tag_ops;
dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
return 0;
-
-out_disconnect:
- info.tag_ops = tag_ops;
- dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, &info);
- dst->tag_ops = old_tag_ops;
-
- return err;
}
/* Since the dsa/tagging sysfs device attribute is per master, the assumption
@@ -1242,7 +1260,7 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
const struct dsa_device_ops *tag_ops,
const struct dsa_device_ops *old_tag_ops)
{
- struct dsa_notifier_tag_proto_info info;
+ struct dsa_notifier_tag_proto_info info, old_info;
struct dsa_port *dp;
int err = -EBUSY;
@@ -1267,23 +1285,19 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
/* Notify the tag protocol change */
info.tag_ops = tag_ops;
- err = dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO, &info);
+ old_info.tag_ops = old_tag_ops;
+ err = dsa_tree_notify_robust(dst, DSA_NOTIFIER_TAG_PROTO, &info,
+ DSA_NOTIFIER_TAG_PROTO, &old_info);
if (err)
- goto out_unwind_tagger;
+ goto out_unlock;
err = dsa_tree_bind_tag_proto(dst, tag_ops);
if (err)
- goto out_unwind_tagger;
-
- rtnl_unlock();
+ dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO, &old_info);
- return 0;
-
-out_unwind_tagger:
- info.tag_ops = old_tag_ops;
- dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO, &info);
out_unlock:
rtnl_unlock();
+
return err;
}
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index d9722e49864b..9db660aeee93 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -543,6 +543,8 @@ void dsa_lag_unmap(struct dsa_switch_tree *dst, struct dsa_lag *lag);
struct dsa_lag *dsa_tree_lag_find(struct dsa_switch_tree *dst,
const struct net_device *lag_dev);
int dsa_tree_notify(struct dsa_switch_tree *dst, unsigned long e, void *v);
+int dsa_tree_notify_robust(struct dsa_switch_tree *dst, unsigned long e,
+ void *v, unsigned long e_rollback, void *v_rollback);
int dsa_broadcast(unsigned long e, void *v);
int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
struct net_device *master,
--
2.34.1
Powered by blists - more mailing lists