netdev - Re: [PATCH net] net: phy: Manual remove LEDs to ensure correct ordering

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, 21 Jun 2023 18:12:40 +0100
From: Florian Fainelli <f.fainelli@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, netdev <netdev@...r.kernel.org>,
 Heiner Kallweit <hkallweit1@...il.com>, ansuelsmth@...il.com,
 stable@...r.kernel.org
Subject: Re: [PATCH net] net: phy: Manual remove LEDs to ensure correct
 ordering

Hi Russell,

On 6/21/2023 6:04 PM, Russell King (Oracle) wrote:
> On Wed, Jun 21, 2023 at 03:04:14PM +0100, Florian Fainelli wrote:
>> Hi Andrew,
>>
>> On 6/17/2023 4:55 PM, Andrew Lunn wrote:
>>> If the core is left to remove the LEDs via devm_, it is performed too
>>> late, after the PHY driver is removed from the PHY. This results in
>>> dereferencing a NULL pointer when the LED core tries to turn the LED
>>> off before destroying the LED.
>>>
>>> Manually unregister the LEDs at a safe point in phy_remove.
>>>
>>> Cc: stable@...r.kernel.org
>>> Reported-by: Florian Fainelli <f.fainelli@...il.com>
>>> Suggested-by: Florian Fainelli <f.fainelli@...il.com>
>>> Fixes: 01e5b728e9e4 ("net: phy: Add a binding for PHY LEDs")
>>> Signed-off-by: Andrew Lunn <andrew@...n.ch>
>>
>> Thanks for fixing this, this is an improvement, though I can still hit
>> another sort of use after free whereby the GENET driver removes the
>> mdio-bcm-unimac platform device and eventually cuts the clock to the MDIO
>> block thus causing the following:
> 
> Hi Florian,
> 
> Can you try setting trigger_data->led_cdev to NULL after the
> cancel_delayed_work_sync() in netdev_trig_deactivate() and see
> what the effect is?

Thanks for the suggestion, getting an identical trace as before with 
that change.
-- 
Florian