netdev - Re: NPD in phy_led_set

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: Wed, 7 Jun 2023 15:29:46 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 Heiner Kallweit <hkallweit1@...il.com>,
 Christian Marangi <ansuelsmth@...il.com>, Vladimir Oltean
 <olteanv@...il.com>, Pavel Machek <pavel@....cz>, Lee Jones
 <lee@...nel.org>, "linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>
Subject: Re: NPD in phy_led_set_brightness+0x3c

On 6/7/23 14:32, Andrew Lunn wrote:
>> There is no trigger being configured for either LED therefore it is not
>> clear to me why the workqueue is being kicked in the first place?
> 
> Since setting LEDs is a sleepable action, it gets offloaded to a
> workqueue.
> 
> My guess is, something in led_classdev_unregister() is triggering it,
> maybe to put the LED into a known state before pulling the
> plug. However, i don't see what.
> 
> I'm also wondering about ordering. The LED is registered with
> devm_led_classdev_register_ext(). So maybe led_classdev_unregister()
> is getting called too late? So maybe we need to replace devm_ with
> manual cleanup.
> 
> However, i've done lots of reboots while developing this code, so its
> interesting you can trigger this, and i've not seen it.

led_brightness_set is the member of phydev->drv which has become NULL:

(gdb) print /x (int)&((struct phy_driver *)0)->led_brightness_set
$1 = 0x1f0

so this would indeed look like an use-after-free here. If you tested 
with a PHYLINK enabled driver you might have no seen due to 
phylink_disconnect_phy() being called with RTNL held?
-- 
Florian