netdev - Re: [PATCH net] net: dsa: sja1105: fix use-after-free after calling of_find_compatible

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Aug 2021 23:01:26 +0000
From:   Alvin Šipraga <ALSI@...g-olufsen.dk>
To:     Vladimir Oltean <olteanv@...il.com>
CC:     Vladimir Oltean <vladimir.oltean@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Frank Rowand <frowand.list@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Saravana Kannan <saravanak@...gle.com>
Subject: Re: [PATCH net] net: dsa: sja1105: fix use-after-free after calling
 of_find_compatible_node, or worse

Hi Vladimir,

On 8/18/21 12:31 AM, Vladimir Oltean wrote:
> 
> So it's a circular dependency? Switch cannot finish probing because it
> cannot connect to PHY, which cannot probe because switch has not
> finished probing, which....
> 
> So how is it supposed to be solved then? Intuitively the 'mdio_bus SMI-0:00'
> device should not be added to the deferred list, it should have everything
> it needs right now (after all, it works without fw_devlink). No?

If I understand correctly, it's just the probing of the PHY driver that 
is getting deferred. This happens both when everything is builtin, and 
when the switch driver (realtek-smi in my case) is built as a module. 
Additionally, when the switch is finished probing, the PHY driver 
doesn't actually get probed. I'm not sure if this is because it has been 
replaced with the Generic PHY driver in the mean time.

What's strange is that if I unload the switch driver module and reload 
it, this time the PHY driver _does_ get probed on demand. I have 
attached a debug log which illustrates the difference. Please note that 
this is using a subdriver of realtek-smi which I have not yet posted to 
the list, but the semantics are the pretty much same as rtl8366rb. See here:

1st load:

[   83.659809] device_add:3270: device: 'SMI-0:00': device_add
[   83.659883] bus_add_device:447: bus: 'mdio_bus': add device SMI-0:00
[   83.659970] device_pm_add:136: PM: Adding info for mdio_bus:SMI-0:00
[   83.660122] device_add:3270: device: 
'platform:ethernet-switch--mdio_bus:SMI-0:00': device_add
[   83.660274] devices_kset_move_last:2701: devices_kset: Moving 
SMI-0:00 to end of list
[   83.660282] device_pm_move_last:203: PM: Moving mdio_bus:SMI-0:00 to 
end of list
[   83.660293] device_link_add:859: mdio_bus SMI-0:00: Linked as a 
consumer to ethernet-switch
[   83.660350] __driver_probe_device:736: bus: 'mdio_bus': 
__driver_probe_device: matched device SMI-0:00 with driver RTL8365MB-VC 
Gigabit Ethernet
[   83.660365] device_links_check_suppliers:1001: mdio_bus SMI-0:00: 
probe deferral - supplier ethernet-switch not ready

2nd load:

[  138.945901] device_add:3270: device: 'SMI-0:00': device_add
[  138.946003] bus_add_device:447: bus: 'mdio_bus': add device SMI-0:00
[  138.946097] device_pm_add:136: PM: Adding info for mdio_bus:SMI-0:00
[  138.946309] __driver_probe_device:736: bus: 'mdio_bus': 
__driver_probe_device: matched device SMI-0:00 with driver RTL8365MB-VC 
Gigabit Ethernet
[  138.946326] really_probe:561: bus: 'mdio_bus': really_probe: probing 
driver RTL8365MB-VC Gigabit Ethernet with device SMI-0:00

I don't know the gory details of the driver core so I am reluctant to 
provide an explanation, but I hope it makes things clear.

Kind regards,
Alvin

View attachment "dmesg.txt" of type "text/plain" (37089 bytes)