* [PATCH net 0/2] net: dsa: Avoid cross-chip syncing of VLAN filtering @ 2022-01-24 21:09 Tobias Waldekranz 2022-01-24 21:09 ` [PATCH net 1/2] net: dsa: Move VLAN filtering syncing out of dsa_switch_bridge_leave Tobias Waldekranz ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Tobias Waldekranz @ 2022-01-24 21:09 UTC (permalink / raw) To: davem, kuba; +Cc: netdev This bug has been latent in the source for quite some time, I suspect due to the homogeneity of both typical configurations and hardware. On singlechip systems, this would never be triggered. The only reason I saw it on my multichip system was because not all chips had the same number of ports, which means that the misdemeanor alien call turned into a felony array-out-of-bounds access. Tobias Waldekranz (2): net: dsa: Move VLAN filtering syncing out of dsa_switch_bridge_leave net: dsa: Avoid cross-chip syncing of VLAN filtering net/dsa/switch.c | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net 1/2] net: dsa: Move VLAN filtering syncing out of dsa_switch_bridge_leave 2022-01-24 21:09 [PATCH net 0/2] net: dsa: Avoid cross-chip syncing of VLAN filtering Tobias Waldekranz @ 2022-01-24 21:09 ` Tobias Waldekranz 2022-01-25 0:05 ` Vladimir Oltean 2022-01-24 21:09 ` [PATCH net 2/2] net: dsa: Avoid cross-chip syncing of VLAN filtering Tobias Waldekranz 2022-01-25 18:01 ` [PATCH net 0/2] " Jakub Kicinski 2 siblings, 1 reply; 9+ messages in thread From: Tobias Waldekranz @ 2022-01-24 21:09 UTC (permalink / raw) To: davem, kuba Cc: netdev, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, linux-kernel Most of dsa_switch_bridge_leave was, in fact, dealing with the syncing of VLAN filtering for switches on which that is a global setting. Separate the two phases to prepare for the cross-chip related bugfix in the following commit. Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> --- net/dsa/switch.c | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/net/dsa/switch.c b/net/dsa/switch.c index e3c7d2627a61..9f9b70d6070a 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -113,26 +113,15 @@ static int dsa_switch_bridge_join(struct dsa_switch *ds, return dsa_tag_8021q_bridge_join(ds, info); } -static int dsa_switch_bridge_leave(struct dsa_switch *ds, - struct dsa_notifier_bridge_info *info) +static int dsa_switch_sync_vlan_filtering(struct dsa_switch *ds, + struct dsa_notifier_bridge_info *info) { - struct dsa_switch_tree *dst = ds->dst; struct netlink_ext_ack extack = {0}; bool change_vlan_filtering = false; bool vlan_filtering; 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) && - ds->ops->crosschip_bridge_leave) - ds->ops->crosschip_bridge_leave(ds, info->tree_index, - info->sw_index, info->port, - info->bridge); - if (ds->needs_standalone_vlan_filtering && !br_vlan_enabled(info->bridge.dev)) { change_vlan_filtering = true; @@ -172,6 +161,29 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds, return err; } + return 0; +} + +static int dsa_switch_bridge_leave(struct dsa_switch *ds, + struct dsa_notifier_bridge_info *info) +{ + struct dsa_switch_tree *dst = ds->dst; + 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) && + ds->ops->crosschip_bridge_leave) + ds->ops->crosschip_bridge_leave(ds, info->tree_index, + info->sw_index, info->port, + info->bridge); + + err = dsa_switch_sync_vlan_filtering(ds, info); + if (err) + return err; + return dsa_tag_8021q_bridge_leave(ds, info); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/2] net: dsa: Move VLAN filtering syncing out of dsa_switch_bridge_leave 2022-01-24 21:09 ` [PATCH net 1/2] net: dsa: Move VLAN filtering syncing out of dsa_switch_bridge_leave Tobias Waldekranz @ 2022-01-25 0:05 ` Vladimir Oltean 0 siblings, 0 replies; 9+ messages in thread From: Vladimir Oltean @ 2022-01-25 0:05 UTC (permalink / raw) To: Tobias Waldekranz Cc: davem, kuba, netdev, Andrew Lunn, Vivien Didelot, Florian Fainelli, linux-kernel On Mon, Jan 24, 2022 at 10:09:43PM +0100, Tobias Waldekranz wrote: > Most of dsa_switch_bridge_leave was, in fact, dealing with the syncing > of VLAN filtering for switches on which that is a global > setting. Separate the two phases to prepare for the cross-chip related > bugfix in the following commit. > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net 2/2] net: dsa: Avoid cross-chip syncing of VLAN filtering 2022-01-24 21:09 [PATCH net 0/2] net: dsa: Avoid cross-chip syncing of VLAN filtering Tobias Waldekranz 2022-01-24 21:09 ` [PATCH net 1/2] net: dsa: Move VLAN filtering syncing out of dsa_switch_bridge_leave Tobias Waldekranz @ 2022-01-24 21:09 ` Tobias Waldekranz 2022-01-25 0:04 ` Vladimir Oltean 2022-01-25 18:01 ` [PATCH net 0/2] " Jakub Kicinski 2 siblings, 1 reply; 9+ messages in thread From: Tobias Waldekranz @ 2022-01-24 21:09 UTC (permalink / raw) To: davem, kuba Cc: netdev, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, linux-kernel Changes to VLAN filtering are not applicable to cross-chip notifications. On a system like this: .-----. .-----. .-----. | sw1 +---+ sw2 +---+ sw3 | '-1-2-' '-1-2-' '-1-2-' Before this change, upon sw1p1 leaving a bridge, a call to dsa_port_vlan_filtering would also be made to sw2p1 and sw3p1. In this scenario: .---------. .-----. .-----. | sw1 +---+ sw2 +---+ sw3 | '-1-2-3-4-' '-1-2-' '-1-2-' When sw1p4 would leave a bridge, dsa_port_vlan_filtering would be called for sw2 and sw3 with a non-existing port - leading to array out-of-bounds accesses and crashes on mv88e6xxx. Fixes: d371b7c92d19 ("net: dsa: Unset vlan_filtering when ports leave the bridge") Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> --- net/dsa/switch.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 9f9b70d6070a..517cc83d13cc 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -180,9 +180,11 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds, info->sw_index, info->port, info->bridge); - err = dsa_switch_sync_vlan_filtering(ds, info); - if (err) - return err; + if (ds->dst->index == info->tree_index && ds->index == info->sw_index) { + err = dsa_switch_sync_vlan_filtering(ds, info); + if (err) + return err; + } return dsa_tag_8021q_bridge_leave(ds, info); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net 2/2] net: dsa: Avoid cross-chip syncing of VLAN filtering 2022-01-24 21:09 ` [PATCH net 2/2] net: dsa: Avoid cross-chip syncing of VLAN filtering Tobias Waldekranz @ 2022-01-25 0:04 ` Vladimir Oltean 0 siblings, 0 replies; 9+ messages in thread From: Vladimir Oltean @ 2022-01-25 0:04 UTC (permalink / raw) To: Tobias Waldekranz Cc: davem, kuba, netdev, Andrew Lunn, Vivien Didelot, Florian Fainelli, linux-kernel On Mon, Jan 24, 2022 at 10:09:44PM +0100, Tobias Waldekranz wrote: > Changes to VLAN filtering are not applicable to cross-chip > notifications. Yes, it seems so. In a cross-chip setup, ports will individually leave the bridge, leaving every switch a chance to unset VLAN filtering. We have this check in dsa_port_vlan_filtering(), so it's easy to forget that the function is called more times than actually needed: if (dsa_port_is_vlan_filtering(dp) == vlan_filtering) return 0; Sorry. > On a system like this: > > .-----. .-----. .-----. > | sw1 +---+ sw2 +---+ sw3 | > '-1-2-' '-1-2-' '-1-2-' > > Before this change, upon sw1p1 leaving a bridge, a call to > dsa_port_vlan_filtering would also be made to sw2p1 and sw3p1. > > In this scenario: > > .---------. .-----. .-----. > | sw1 +---+ sw2 +---+ sw3 | > '-1-2-3-4-' '-1-2-' '-1-2-' > > When sw1p4 would leave a bridge, dsa_port_vlan_filtering would be > called for sw2 and sw3 with a non-existing port - leading to array > out-of-bounds accesses and crashes on mv88e6xxx. > > Fixes: d371b7c92d19 ("net: dsa: Unset vlan_filtering when ports leave the bridge") > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > --- > net/dsa/switch.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/net/dsa/switch.c b/net/dsa/switch.c > index 9f9b70d6070a..517cc83d13cc 100644 > --- a/net/dsa/switch.c > +++ b/net/dsa/switch.c > @@ -180,9 +180,11 @@ static int dsa_switch_bridge_leave(struct dsa_switch *ds, > info->sw_index, info->port, > info->bridge); > > - err = dsa_switch_sync_vlan_filtering(ds, info); > - if (err) > - return err; > + if (ds->dst->index == info->tree_index && ds->index == info->sw_index) { > + err = dsa_switch_sync_vlan_filtering(ds, info); > + if (err) > + return err; > + } As net-next material, we could probably move this call to dsa_port_switchdev_unsync_attrs() where there's even a comment that references it, and do away with the targeted switch check. > > return dsa_tag_8021q_bridge_leave(ds, info); > } > -- > 2.25.1 > Reviewed-by: Vladimir Oltean <olteanv@gmail.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 0/2] net: dsa: Avoid cross-chip syncing of VLAN filtering 2022-01-24 21:09 [PATCH net 0/2] net: dsa: Avoid cross-chip syncing of VLAN filtering Tobias Waldekranz 2022-01-24 21:09 ` [PATCH net 1/2] net: dsa: Move VLAN filtering syncing out of dsa_switch_bridge_leave Tobias Waldekranz 2022-01-24 21:09 ` [PATCH net 2/2] net: dsa: Avoid cross-chip syncing of VLAN filtering Tobias Waldekranz @ 2022-01-25 18:01 ` Jakub Kicinski 2022-01-25 19:05 ` Tobias Waldekranz 2 siblings, 1 reply; 9+ messages in thread From: Jakub Kicinski @ 2022-01-25 18:01 UTC (permalink / raw) To: Tobias Waldekranz; +Cc: davem, netdev On Mon, 24 Jan 2022 22:09:42 +0100 Tobias Waldekranz wrote: > This bug has been latent in the source for quite some time, I suspect > due to the homogeneity of both typical configurations and hardware. > > On singlechip systems, this would never be triggered. The only reason > I saw it on my multichip system was because not all chips had the same > number of ports, which means that the misdemeanor alien call turned > into a felony array-out-of-bounds access. Applied, thanks, 934d0f039959 ("Merge branch 'dsa-avoid-cross-chip-vlan-sync'") in net-next. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 0/2] net: dsa: Avoid cross-chip syncing of VLAN filtering 2022-01-25 18:01 ` [PATCH net 0/2] " Jakub Kicinski @ 2022-01-25 19:05 ` Tobias Waldekranz 2022-01-25 20:41 ` Jakub Kicinski 0 siblings, 1 reply; 9+ messages in thread From: Tobias Waldekranz @ 2022-01-25 19:05 UTC (permalink / raw) To: Jakub Kicinski; +Cc: davem, netdev On Tue, Jan 25, 2022 at 10:01, Jakub Kicinski <kuba@kernel.org> wrote: > On Mon, 24 Jan 2022 22:09:42 +0100 Tobias Waldekranz wrote: >> This bug has been latent in the source for quite some time, I suspect >> due to the homogeneity of both typical configurations and hardware. >> >> On singlechip systems, this would never be triggered. The only reason >> I saw it on my multichip system was because not all chips had the same >> number of ports, which means that the misdemeanor alien call turned >> into a felony array-out-of-bounds access. > > Applied, thanks, 934d0f039959 ("Merge branch > 'dsa-avoid-cross-chip-vlan-sync'") in net-next. Thank you! Is there a particular reason that this was applied to net-next? I guess my question is really: will it still be considered for upcoming stable kernel releases? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 0/2] net: dsa: Avoid cross-chip syncing of VLAN filtering 2022-01-25 19:05 ` Tobias Waldekranz @ 2022-01-25 20:41 ` Jakub Kicinski 2022-01-25 22:20 ` Tobias Waldekranz 0 siblings, 1 reply; 9+ messages in thread From: Jakub Kicinski @ 2022-01-25 20:41 UTC (permalink / raw) To: Tobias Waldekranz; +Cc: davem, netdev On Tue, 25 Jan 2022 20:05:45 +0100 Tobias Waldekranz wrote: > On Tue, Jan 25, 2022 at 10:01, Jakub Kicinski <kuba@kernel.org> wrote: > > On Mon, 24 Jan 2022 22:09:42 +0100 Tobias Waldekranz wrote: > >> This bug has been latent in the source for quite some time, I suspect > >> due to the homogeneity of both typical configurations and hardware. > >> > >> On singlechip systems, this would never be triggered. The only reason > >> I saw it on my multichip system was because not all chips had the same > >> number of ports, which means that the misdemeanor alien call turned > >> into a felony array-out-of-bounds access. > > > > Applied, thanks, 934d0f039959 ("Merge branch > > 'dsa-avoid-cross-chip-vlan-sync'") in net-next. > > Is there a particular reason that this was applied to net-next? Not sure, there were issues with kernel.org infra during the night, could be unintentional. > I guess my question is really: will it still be considered for > upcoming stable kernel releases? Only after the next merge window, but yes. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 0/2] net: dsa: Avoid cross-chip syncing of VLAN filtering 2022-01-25 20:41 ` Jakub Kicinski @ 2022-01-25 22:20 ` Tobias Waldekranz 0 siblings, 0 replies; 9+ messages in thread From: Tobias Waldekranz @ 2022-01-25 22:20 UTC (permalink / raw) To: Jakub Kicinski; +Cc: davem, netdev On Tue, Jan 25, 2022 at 12:41, Jakub Kicinski <kuba@kernel.org> wrote: > On Tue, 25 Jan 2022 20:05:45 +0100 Tobias Waldekranz wrote: >> On Tue, Jan 25, 2022 at 10:01, Jakub Kicinski <kuba@kernel.org> wrote: >> > On Mon, 24 Jan 2022 22:09:42 +0100 Tobias Waldekranz wrote: >> >> This bug has been latent in the source for quite some time, I suspect >> >> due to the homogeneity of both typical configurations and hardware. >> >> >> >> On singlechip systems, this would never be triggered. The only reason >> >> I saw it on my multichip system was because not all chips had the same >> >> number of ports, which means that the misdemeanor alien call turned >> >> into a felony array-out-of-bounds access. >> > >> > Applied, thanks, 934d0f039959 ("Merge branch >> > 'dsa-avoid-cross-chip-vlan-sync'") in net-next. >> >> Is there a particular reason that this was applied to net-next? > > Not sure, there were issues with kernel.org infra during the night, > could be unintentional. Ahh ok, hope it gets sorted quickly! >> I guess my question is really: will it still be considered for >> upcoming stable kernel releases? > > Only after the next merge window, but yes. I had a feeling that would be the case. Aright, not optimal, but not a big deal either. Thanks for taking the time to respond. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-01-25 22:20 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-24 21:09 [PATCH net 0/2] net: dsa: Avoid cross-chip syncing of VLAN filtering Tobias Waldekranz 2022-01-24 21:09 ` [PATCH net 1/2] net: dsa: Move VLAN filtering syncing out of dsa_switch_bridge_leave Tobias Waldekranz 2022-01-25 0:05 ` Vladimir Oltean 2022-01-24 21:09 ` [PATCH net 2/2] net: dsa: Avoid cross-chip syncing of VLAN filtering Tobias Waldekranz 2022-01-25 0:04 ` Vladimir Oltean 2022-01-25 18:01 ` [PATCH net 0/2] " Jakub Kicinski 2022-01-25 19:05 ` Tobias Waldekranz 2022-01-25 20:41 ` Jakub Kicinski 2022-01-25 22:20 ` Tobias Waldekranz
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.