| Message ID | 20231124041529.3450079-2-gerg@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a524eabcd72d28425d9db242cf375d0389d74eba |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [PATCHv2,1/2] net: dsa: mv88e6xxx: fix marvell 6350 switch probing | expand |
On Fri, Nov 24, 2023 at 02:15:29PM +1000, Greg Ungerer wrote: > As of commit b92143d4420f ("net: dsa: mv88e6xxx: add infrastructure for > phylink_pcs") probing of a Marvell 88e6350 switch causes a NULL pointer > de-reference like this example: > > ... > mv88e6085 d0072004.mdio-mii:11: switch 0x3710 detected: Marvell 88E6350, revision 2 > 8<--- cut here --- > Unable to handle kernel NULL pointer dereference at virtual address 00000000 when read > [00000000] *pgd=00000000 > Internal error: Oops: 5 [#1] ARM > Modules linked in: > CPU: 0 PID: 8 Comm: kworker/u2:0 Not tainted 6.7.0-rc2-dirty #26 > Hardware name: Marvell Armada 370/XP (Device Tree) > Workqueue: events_unbound deferred_probe_work_func > PC is at mv88e6xxx_port_setup+0x1c/0x44 > LR is at dsa_port_devlink_setup+0x74/0x154 > pc : [<c057ea24>] lr : [<c0819598>] psr: a0000013 > sp : c184fce0 ip : c542b8f4 fp : 00000000 > r10: 00000001 r9 : c542a540 r8 : c542bc00 > r7 : c542b838 r6 : c5244580 r5 : 00000005 r4 : c5244580 > r3 : 00000000 r2 : c542b840 r1 : 00000005 r0 : c1a02040 > ... > > The Marvell 6350 switch has no SERDES interface and so has no > corresponding pcs_ops defined for it. But during probing a call is made > to mv88e6xxx_port_setup() which unconditionally expects pcs_ops to exist - > though the presence of the pcs_ops->pcs_init function is optional. > > Modify code to check for pcs_ops first, before checking for and calling > pcs_ops->pcs_init. Modify checking and use of pcs_ops->pcs_teardown > which may potentially suffer the same problem. > > Fixes: b92143d4420f ("net: dsa: mv88e6xxx: add infrastructure for phylink_pcs") > Signed-off-by: Greg Ungerer <gerg@kernel.org> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index d8a67bf4e595..07a22c74fe81 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -3892,7 +3892,8 @@ static int mv88e6xxx_port_setup(struct dsa_switch *ds, int port) struct mv88e6xxx_chip *chip = ds->priv; int err; - if (chip->info->ops->pcs_ops->pcs_init) { + if (chip->info->ops->pcs_ops && + chip->info->ops->pcs_ops->pcs_init) { err = chip->info->ops->pcs_ops->pcs_init(chip, port); if (err) return err; @@ -3907,7 +3908,8 @@ static void mv88e6xxx_port_teardown(struct dsa_switch *ds, int port) mv88e6xxx_teardown_devlink_regions_port(ds, port); - if (chip->info->ops->pcs_ops->pcs_teardown) + if (chip->info->ops->pcs_ops && + chip->info->ops->pcs_ops->pcs_teardown) chip->info->ops->pcs_ops->pcs_teardown(chip, port); }
As of commit b92143d4420f ("net: dsa: mv88e6xxx: add infrastructure for phylink_pcs") probing of a Marvell 88e6350 switch causes a NULL pointer de-reference like this example: ... mv88e6085 d0072004.mdio-mii:11: switch 0x3710 detected: Marvell 88E6350, revision 2 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 00000000 when read [00000000] *pgd=00000000 Internal error: Oops: 5 [#1] ARM Modules linked in: CPU: 0 PID: 8 Comm: kworker/u2:0 Not tainted 6.7.0-rc2-dirty #26 Hardware name: Marvell Armada 370/XP (Device Tree) Workqueue: events_unbound deferred_probe_work_func PC is at mv88e6xxx_port_setup+0x1c/0x44 LR is at dsa_port_devlink_setup+0x74/0x154 pc : [<c057ea24>] lr : [<c0819598>] psr: a0000013 sp : c184fce0 ip : c542b8f4 fp : 00000000 r10: 00000001 r9 : c542a540 r8 : c542bc00 r7 : c542b838 r6 : c5244580 r5 : 00000005 r4 : c5244580 r3 : 00000000 r2 : c542b840 r1 : 00000005 r0 : c1a02040 ... The Marvell 6350 switch has no SERDES interface and so has no corresponding pcs_ops defined for it. But during probing a call is made to mv88e6xxx_port_setup() which unconditionally expects pcs_ops to exist - though the presence of the pcs_ops->pcs_init function is optional. Modify code to check for pcs_ops first, before checking for and calling pcs_ops->pcs_init. Modify checking and use of pcs_ops->pcs_teardown which may potentially suffer the same problem. Fixes: b92143d4420f ("net: dsa: mv88e6xxx: add infrastructure for phylink_pcs") Signed-off-by: Greg Ungerer <gerg@kernel.org> --- drivers/net/dsa/mv88e6xxx/chip.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) v2: apply same change to use of pcs_ops->teardown