* [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
* [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 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
* 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.