Re: [PATCH net] net: dsa: sja1105: fix use-after-free after calling of_find_compatible_node, or worse
From: Alvin Šipraga <hidden>
Date: 2021-08-22 14:19:54
Hi Saravana, Thanks for the follow-up. I tested your change and it does the trick: there is no deferral and the PHY driver gets probed first-try during the mdiobus registration during the call to dsa_register_switch(). I tested with the switch, PHY, and tagging drivers all builtin, or all modules, and it worked in both cases. On 8/20/21 6:52 PM, Saravana Kannan wrote:
Hi Alvin, Can you give this a shot to see if it fixes your issue? It basically delays the registration of dsa_register_switch() until all the consumers of this switch have probed. So it has a couple of caveats:
Hm, weren't the only consumers the PHYs themselves? It seems like the main effect of your change is that - by doing the actual dsa_register_switch() call after the switch driver probe - the ethernet-switch (provider) is already probed, thereby allowing the PHY (consumer) to probe immediately.
1. I'm hoping the PHYs are the only consumers of this switch.
In my case that is true, if you count the mdio_bus as well: /sys/devices/platform/ethernet-switch# ls -l consumer\:* lrwxrwxrwx 1 root root 0 Aug 22 16:00 consumer:mdio_bus:SMI-0 -> ../../virtual/devlink/platform:ethernet-switch--mdio_bus:SMI-0 lrwxrwxrwx 1 root root 0 Aug 22 16:00 consumer:mdio_bus:SMI-0:00 -> ../../virtual/devlink/platform:ethernet-switch--mdio_bus:SMI-0:00 lrwxrwxrwx 1 root root 0 Aug 22 16:00 consumer:mdio_bus:SMI-0:01 -> ../../virtual/devlink/platform:ethernet-switch--mdio_bus:SMI-0:01 lrwxrwxrwx 1 root root 0 Aug 22 16:00 consumer:mdio_bus:SMI-0:02 -> ../../virtual/devlink/platform:ethernet-switch--mdio_bus:SMI-0:02 lrwxrwxrwx 1 root root 0 Aug 22 16:00 consumer:mdio_bus:SMI-0:03 -> ../../virtual/devlink/platform:ethernet-switch--mdio_bus:SMI-0:03
2. All of them have to probe successfully before the switch will register itself.
Yes.
3. If dsa_register_switch() fails, we can't defer the probe (because it already succeeded). But I'm not sure if it's a likely error code.
It's of course possible that dsa_register_switch() fails. Assuming fw_devlink is doing its job properly, I think the reason is most likely going to be something specific to the driver, such as a communication timeout with the switch hardware itself. I get the impression that you don't necessarily regard this change as a proper fix, so I'm happy to do further tests if you choose to investigate further. Kind regards, Alvin
quoted hunk ↗ jump to hunk
-Saravana+++ b/drivers/net/dsa/realtek-smi-core.c@@ -454,14 +454,16 @@ static int realtek_smi_probe(struct platform_device *pdev) smi->ds->priv = smi; smi->ds->ops = var->ds_ops; - ret = dsa_register_switch(smi->ds); - if (ret) { - dev_err(dev, "unable to register switch ret = %d\n", ret); - return ret; - } return 0; } +static void realtek_smi_sync_state(struct device *dev) +{ + struct realtek_smi *smi = dev_get_drvdata(dev); + if (dsa_register_switch(smi->ds)) + dev_err(dev, "unable to register switch ret = %d\n", ret); +} + static int realtek_smi_remove(struct platform_device *pdev) { struct realtek_smi *smi = dev_get_drvdata(&pdev->dev);@@ -492,6 +494,7 @@ static struct platform_driver realtek_smi_driver = { .driver = { .name = "realtek-smi", .of_match_table = of_match_ptr(realtek_smi_of_match), + .sync_state = realtek_smi_sync_state, }, .probe = realtek_smi_probe, .remove = realtek_smi_remove,