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

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Wed, Jun 21, 2023 at 06:12:40PM +0100, Florian Fainelli wrote:
> 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@vger.kernel.org
> > > > Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> > > > Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
> > > > Fixes: 01e5b728e9e4 ("net: phy: Add a binding for PHY LEDs")
> > > > Signed-off-by: Andrew Lunn <andrew@lunn.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.

Thanks for trying. I was wondering whether the work was being re-queued
after the flush_work(), but seemingly not.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!