Merge branch 'collection-of-dsa-bug-fixes'

Vladimir Oltean says:

====================
Collection of DSA bug fixes

Prompted by Russell King's 3 DSA bug reports from Friday (linked in
their respective patches: 1, 2 and 3), I am providing fixes to those, as
well as flushing the queue with 2 other bug fixes I had.

1: fix NULL pointer dereference during mv88e6xxx driver unbind, on old
   switch models which lack PVT and/or STU. Seen on the ZII dev board
   rev B.
2: fix failure to delete bridge port VLANs on old mv88e6xxx chips which
   lack STU. Seen on the same board.
3: fix WARN_ON() and resource leak in DSA core on driver unbind. Seen on
   the same board but is a much more widespread issue.
4: fix use-after-free during probing of DSA trees with >= 3 switches,
   if -EPROBE_DEFER exists. In principle issue also exists for the ZII
   board, I reproduced on Turris MOX.
5: fix incorrect use of refcount API in DSA core for those switches
   which use tag_8021q (felix, sja1105, vsc73xx). Returning an error
   when attempting to delete a tag_8021q VLAN prints a WARN_ON(), which
   is harmless but might be problematic with CONFIG_PANIC_ON_OOPS.
====================

Link: https://patch.msgid.link/20250414212708.2948164-1-vladimir.oltean@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2025-04-16 18:14:45 -07:00
commit 7e49e6448c
4 changed files with 64 additions and 13 deletions

View File

@ -1852,6 +1852,8 @@ static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
if (!chip->info->ops->vtu_getnext)
return -EOPNOTSUPP;
memset(entry, 0, sizeof(*entry));
entry->vid = vid ? vid - 1 : mv88e6xxx_max_vid(chip);
entry->valid = false;
@ -1960,7 +1962,16 @@ static int mv88e6xxx_mst_put(struct mv88e6xxx_chip *chip, u8 sid)
struct mv88e6xxx_mst *mst, *tmp;
int err;
if (!sid)
/* If the SID is zero, it is for a VLAN mapped to the default MSTI,
* and mv88e6xxx_stu_setup() made sure it is always present, and thus,
* should not be removed here.
*
* If the chip lacks STU support, numerically the "sid" variable will
* happen to also be zero, but we don't want to rely on that fact, so
* we explicitly test that first. In that case, there is also nothing
* to do here.
*/
if (!mv88e6xxx_has_stu(chip) || !sid)
return 0;
list_for_each_entry_safe(mst, tmp, &chip->msts, node) {

View File

@ -736,7 +736,8 @@ void mv88e6xxx_teardown_devlink_regions_global(struct dsa_switch *ds)
int i;
for (i = 0; i < ARRAY_SIZE(mv88e6xxx_regions); i++)
dsa_devlink_region_destroy(chip->regions[i]);
if (chip->regions[i])
dsa_devlink_region_destroy(chip->regions[i]);
}
void mv88e6xxx_teardown_devlink_regions_port(struct dsa_switch *ds, int port)

View File

@ -862,6 +862,16 @@ static void dsa_tree_teardown_lags(struct dsa_switch_tree *dst)
kfree(dst->lags);
}
static void dsa_tree_teardown_routing_table(struct dsa_switch_tree *dst)
{
struct dsa_link *dl, *next;
list_for_each_entry_safe(dl, next, &dst->rtable, list) {
list_del(&dl->list);
kfree(dl);
}
}
static int dsa_tree_setup(struct dsa_switch_tree *dst)
{
bool complete;
@ -879,7 +889,7 @@ static int dsa_tree_setup(struct dsa_switch_tree *dst)
err = dsa_tree_setup_cpu_ports(dst);
if (err)
return err;
goto teardown_rtable;
err = dsa_tree_setup_switches(dst);
if (err)
@ -911,14 +921,14 @@ teardown_switches:
dsa_tree_teardown_switches(dst);
teardown_cpu_ports:
dsa_tree_teardown_cpu_ports(dst);
teardown_rtable:
dsa_tree_teardown_routing_table(dst);
return err;
}
static void dsa_tree_teardown(struct dsa_switch_tree *dst)
{
struct dsa_link *dl, *next;
if (!dst->setup)
return;
@ -932,10 +942,7 @@ static void dsa_tree_teardown(struct dsa_switch_tree *dst)
dsa_tree_teardown_cpu_ports(dst);
list_for_each_entry_safe(dl, next, &dst->rtable, list) {
list_del(&dl->list);
kfree(dl);
}
dsa_tree_teardown_routing_table(dst);
pr_info("DSA: tree %d torn down\n", dst->index);
@ -1478,12 +1485,44 @@ static int dsa_switch_parse(struct dsa_switch *ds, struct dsa_chip_data *cd)
static void dsa_switch_release_ports(struct dsa_switch *ds)
{
struct dsa_mac_addr *a, *tmp;
struct dsa_port *dp, *next;
struct dsa_vlan *v, *n;
dsa_switch_for_each_port_safe(dp, next, ds) {
WARN_ON(!list_empty(&dp->fdbs));
WARN_ON(!list_empty(&dp->mdbs));
WARN_ON(!list_empty(&dp->vlans));
/* These are either entries that upper layers lost track of
* (probably due to bugs), or installed through interfaces
* where one does not necessarily have to remove them, like
* ndo_dflt_fdb_add().
*/
list_for_each_entry_safe(a, tmp, &dp->fdbs, list) {
dev_info(ds->dev,
"Cleaning up unicast address %pM vid %u from port %d\n",
a->addr, a->vid, dp->index);
list_del(&a->list);
kfree(a);
}
list_for_each_entry_safe(a, tmp, &dp->mdbs, list) {
dev_info(ds->dev,
"Cleaning up multicast address %pM vid %u from port %d\n",
a->addr, a->vid, dp->index);
list_del(&a->list);
kfree(a);
}
/* These are entries that upper layers have lost track of,
* probably due to bugs, but also due to dsa_port_do_vlan_del()
* having failed and the VLAN entry still lingering on.
*/
list_for_each_entry_safe(v, n, &dp->vlans, list) {
dev_info(ds->dev,
"Cleaning up vid %u from port %d\n",
v->vid, dp->index);
list_del(&v->list);
kfree(v);
}
list_del(&dp->list);
kfree(dp);
}

View File

@ -197,7 +197,7 @@ static int dsa_port_do_tag_8021q_vlan_del(struct dsa_port *dp, u16 vid)
err = ds->ops->tag_8021q_vlan_del(ds, port, vid);
if (err) {
refcount_inc(&v->refcount);
refcount_set(&v->refcount, 1);
return err;
}