From ff91e1b68490b97c18c649b769618815eb945f11 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Wed, 5 Jan 2022 15:18:11 +0200 Subject: net: dsa: fix incorrect function pointer check for MRP ring roles The cross-chip notifier boilerplate code meant to check the presence of ds->ops->port_mrp_add_ring_role before calling it, but checked ds->ops->port_mrp_add instead, before calling ds->ops->port_mrp_add_ring_role. Therefore, a driver which implements one operation but not the other would trigger a NULL pointer dereference. There isn't any such driver in DSA yet, so there is no reason to backport the change. Issue found through code inspection. Cc: Horatiu Vultur Fixes: c595c4330da0 ("net: dsa: add MRP support") Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- net/dsa/switch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 393f2d8a860a..260d8e7d6e5a 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -729,7 +729,7 @@ static int dsa_switch_mrp_add_ring_role(struct dsa_switch *ds, struct dsa_notifier_mrp_ring_role_info *info) { - if (!ds->ops->port_mrp_add) + if (!ds->ops->port_mrp_add_ring_role) return -EOPNOTSUPP; if (ds->index == info->sw_index) @@ -743,7 +743,7 @@ static int dsa_switch_mrp_del_ring_role(struct dsa_switch *ds, struct dsa_notifier_mrp_ring_role_info *info) { - if (!ds->ops->port_mrp_del) + if (!ds->ops->port_mrp_del_ring_role) return -EOPNOTSUPP; if (ds->index == info->sw_index) -- cgit From cad69019f2f8c8c36006fa67b4b5ce04ac2e262b Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Wed, 5 Jan 2022 15:18:12 +0200 Subject: net: dsa: remove cross-chip support for MRP The cross-chip notifiers for MRP are bypass operations, meaning that even though all switches in a tree are notified, only the switch specified in the info structure is targeted. We can eliminate the unnecessary complexity by deleting the cross-chip notifier logic and calling the ds->ops straight from port.c. Cc: Horatiu Vultur Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- net/dsa/dsa_priv.h | 18 --------------- net/dsa/port.c | 44 +++++++++++++++++-------------------- net/dsa/switch.c | 64 ------------------------------------------------------ 3 files changed, 20 insertions(+), 106 deletions(-) diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index b5ae21f172a8..c593d56c94b3 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -40,10 +40,6 @@ enum { DSA_NOTIFIER_TAG_PROTO, DSA_NOTIFIER_TAG_PROTO_CONNECT, DSA_NOTIFIER_TAG_PROTO_DISCONNECT, - DSA_NOTIFIER_MRP_ADD, - DSA_NOTIFIER_MRP_DEL, - DSA_NOTIFIER_MRP_ADD_RING_ROLE, - DSA_NOTIFIER_MRP_DEL_RING_ROLE, DSA_NOTIFIER_TAG_8021Q_VLAN_ADD, DSA_NOTIFIER_TAG_8021Q_VLAN_DEL, }; @@ -107,20 +103,6 @@ struct dsa_notifier_tag_proto_info { const struct dsa_device_ops *tag_ops; }; -/* DSA_NOTIFIER_MRP_* */ -struct dsa_notifier_mrp_info { - const struct switchdev_obj_mrp *mrp; - int sw_index; - int port; -}; - -/* DSA_NOTIFIER_MRP_* */ -struct dsa_notifier_mrp_ring_role_info { - const struct switchdev_obj_ring_role_mrp *mrp; - int sw_index; - int port; -}; - /* DSA_NOTIFIER_TAG_8021Q_VLAN_* */ struct dsa_notifier_tag_8021q_vlan_info { int tree_index; diff --git a/net/dsa/port.c b/net/dsa/port.c index 05677e016982..05be4577b044 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -907,49 +907,45 @@ int dsa_port_vlan_del(struct dsa_port *dp, int dsa_port_mrp_add(const struct dsa_port *dp, const struct switchdev_obj_mrp *mrp) { - struct dsa_notifier_mrp_info info = { - .sw_index = dp->ds->index, - .port = dp->index, - .mrp = mrp, - }; + struct dsa_switch *ds = dp->ds; + + if (!ds->ops->port_mrp_add) + return -EOPNOTSUPP; - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD, &info); + return ds->ops->port_mrp_add(ds, dp->index, mrp); } int dsa_port_mrp_del(const struct dsa_port *dp, const struct switchdev_obj_mrp *mrp) { - struct dsa_notifier_mrp_info info = { - .sw_index = dp->ds->index, - .port = dp->index, - .mrp = mrp, - }; + struct dsa_switch *ds = dp->ds; + + if (!ds->ops->port_mrp_del) + return -EOPNOTSUPP; - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL, &info); + return ds->ops->port_mrp_del(ds, dp->index, mrp); } int dsa_port_mrp_add_ring_role(const struct dsa_port *dp, const struct switchdev_obj_ring_role_mrp *mrp) { - struct dsa_notifier_mrp_ring_role_info info = { - .sw_index = dp->ds->index, - .port = dp->index, - .mrp = mrp, - }; + struct dsa_switch *ds = dp->ds; + + if (!ds->ops->port_mrp_add_ring_role) + return -EOPNOTSUPP; - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_ADD_RING_ROLE, &info); + return ds->ops->port_mrp_add_ring_role(ds, dp->index, mrp); } int dsa_port_mrp_del_ring_role(const struct dsa_port *dp, const struct switchdev_obj_ring_role_mrp *mrp) { - struct dsa_notifier_mrp_ring_role_info info = { - .sw_index = dp->ds->index, - .port = dp->index, - .mrp = mrp, - }; + struct dsa_switch *ds = dp->ds; + + if (!ds->ops->port_mrp_del_ring_role) + return -EOPNOTSUPP; - return dsa_port_notify(dp, DSA_NOTIFIER_MRP_DEL_RING_ROLE, &info); + return ds->ops->port_mrp_del_ring_role(ds, dp->index, mrp); } void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp, diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 260d8e7d6e5a..a164ec02b4e9 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -701,58 +701,6 @@ dsa_switch_disconnect_tag_proto(struct dsa_switch *ds, return 0; } -static int dsa_switch_mrp_add(struct dsa_switch *ds, - struct dsa_notifier_mrp_info *info) -{ - if (!ds->ops->port_mrp_add) - return -EOPNOTSUPP; - - if (ds->index == info->sw_index) - return ds->ops->port_mrp_add(ds, info->port, info->mrp); - - return 0; -} - -static int dsa_switch_mrp_del(struct dsa_switch *ds, - struct dsa_notifier_mrp_info *info) -{ - if (!ds->ops->port_mrp_del) - return -EOPNOTSUPP; - - if (ds->index == info->sw_index) - return ds->ops->port_mrp_del(ds, info->port, info->mrp); - - return 0; -} - -static int -dsa_switch_mrp_add_ring_role(struct dsa_switch *ds, - struct dsa_notifier_mrp_ring_role_info *info) -{ - if (!ds->ops->port_mrp_add_ring_role) - return -EOPNOTSUPP; - - if (ds->index == info->sw_index) - return ds->ops->port_mrp_add_ring_role(ds, info->port, - info->mrp); - - return 0; -} - -static int -dsa_switch_mrp_del_ring_role(struct dsa_switch *ds, - struct dsa_notifier_mrp_ring_role_info *info) -{ - if (!ds->ops->port_mrp_del_ring_role) - return -EOPNOTSUPP; - - if (ds->index == info->sw_index) - return ds->ops->port_mrp_del_ring_role(ds, info->port, - info->mrp); - - return 0; -} - static int dsa_switch_event(struct notifier_block *nb, unsigned long event, void *info) { @@ -826,18 +774,6 @@ static int dsa_switch_event(struct notifier_block *nb, case DSA_NOTIFIER_TAG_PROTO_DISCONNECT: err = dsa_switch_disconnect_tag_proto(ds, info); break; - case DSA_NOTIFIER_MRP_ADD: - err = dsa_switch_mrp_add(ds, info); - break; - case DSA_NOTIFIER_MRP_DEL: - err = dsa_switch_mrp_del(ds, info); - break; - case DSA_NOTIFIER_MRP_ADD_RING_ROLE: - err = dsa_switch_mrp_add_ring_role(ds, info); - break; - case DSA_NOTIFIER_MRP_DEL_RING_ROLE: - err = dsa_switch_mrp_del_ring_role(ds, info); - break; case DSA_NOTIFIER_TAG_8021Q_VLAN_ADD: err = dsa_switch_tag_8021q_vlan_add(ds, info); break; -- cgit From a68dc7b938fbbf3d4d74b81a9b5d0d6fdd78566c Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Wed, 5 Jan 2022 15:18:13 +0200 Subject: net: dsa: remove cross-chip support for HSR The cross-chip notifiers for HSR are bypass operations, meaning that even though all switches in a tree are notified, only the switch specified in the info structure is targeted. We can eliminate the unnecessary complexity by deleting the cross-chip notifier logic and calling the ds->ops straight from port.c. Cc: George McCollister Signed-off-by: Vladimir Oltean Reviewed-by: George McCollister Signed-off-by: David S. Miller --- net/dsa/dsa_priv.h | 9 --------- net/dsa/port.c | 29 +++++++++++++---------------- net/dsa/switch.c | 24 ------------------------ 3 files changed, 13 insertions(+), 49 deletions(-) diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index c593d56c94b3..760306f0012f 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h @@ -25,8 +25,6 @@ enum { DSA_NOTIFIER_FDB_DEL, DSA_NOTIFIER_HOST_FDB_ADD, DSA_NOTIFIER_HOST_FDB_DEL, - DSA_NOTIFIER_HSR_JOIN, - DSA_NOTIFIER_HSR_LEAVE, DSA_NOTIFIER_LAG_CHANGE, DSA_NOTIFIER_LAG_JOIN, DSA_NOTIFIER_LAG_LEAVE, @@ -125,13 +123,6 @@ struct dsa_switchdev_event_work { bool host_addr; }; -/* DSA_NOTIFIER_HSR_* */ -struct dsa_notifier_hsr_info { - struct net_device *hsr; - int sw_index; - int port; -}; - struct dsa_slave_priv { /* Copy of CPU port xmit for faster access in slave transmit hot path */ struct sk_buff * (*xmit)(struct sk_buff *skb, diff --git a/net/dsa/port.c b/net/dsa/port.c index 05be4577b044..bd78192e0e47 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c @@ -1317,16 +1317,15 @@ EXPORT_SYMBOL_GPL(dsa_port_get_phy_sset_count); int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr) { - struct dsa_notifier_hsr_info info = { - .sw_index = dp->ds->index, - .port = dp->index, - .hsr = hsr, - }; + struct dsa_switch *ds = dp->ds; int err; + if (!ds->ops->port_hsr_join) + return -EOPNOTSUPP; + dp->hsr_dev = hsr; - err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_JOIN, &info); + err = ds->ops->port_hsr_join(ds, dp->index, hsr); if (err) dp->hsr_dev = NULL; @@ -1335,20 +1334,18 @@ int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr) void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr) { - struct dsa_notifier_hsr_info info = { - .sw_index = dp->ds->index, - .port = dp->index, - .hsr = hsr, - }; + struct dsa_switch *ds = dp->ds; int err; dp->hsr_dev = NULL; - err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_LEAVE, &info); - if (err) - dev_err(dp->ds->dev, - "port %d failed to notify DSA_NOTIFIER_HSR_LEAVE: %pe\n", - dp->index, ERR_PTR(err)); + if (ds->ops->port_hsr_leave) { + err = ds->ops->port_hsr_leave(ds, dp->index, hsr); + if (err) + dev_err(dp->ds->dev, + "port %d failed to leave HSR %s: %pe\n", + dp->index, hsr->name, ERR_PTR(err)); + } } int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast) diff --git a/net/dsa/switch.c b/net/dsa/switch.c index a164ec02b4e9..e3c7d2627a61 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -437,24 +437,6 @@ static int dsa_switch_fdb_del(struct dsa_switch *ds, return dsa_port_do_fdb_del(dp, info->addr, info->vid); } -static int dsa_switch_hsr_join(struct dsa_switch *ds, - struct dsa_notifier_hsr_info *info) -{ - if (ds->index == info->sw_index && ds->ops->port_hsr_join) - return ds->ops->port_hsr_join(ds, info->port, info->hsr); - - return -EOPNOTSUPP; -} - -static int dsa_switch_hsr_leave(struct dsa_switch *ds, - struct dsa_notifier_hsr_info *info) -{ - if (ds->index == info->sw_index && ds->ops->port_hsr_leave) - return ds->ops->port_hsr_leave(ds, info->port, info->hsr); - - return -EOPNOTSUPP; -} - static int dsa_switch_lag_change(struct dsa_switch *ds, struct dsa_notifier_lag_info *info) { @@ -729,12 +711,6 @@ static int dsa_switch_event(struct notifier_block *nb, case DSA_NOTIFIER_HOST_FDB_DEL: err = dsa_switch_host_fdb_del(ds, info); break; - case DSA_NOTIFIER_HSR_JOIN: - err = dsa_switch_hsr_join(ds, info); - break; - case DSA_NOTIFIER_HSR_LEAVE: - err = dsa_switch_hsr_leave(ds, info); - break; case DSA_NOTIFIER_LAG_CHANGE: err = dsa_switch_lag_change(ds, info); break; -- cgit