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]

Message-ID: <8a41a15a-b832-3e66-d10a-df29f1a4c880@gmail.com>
Date: Wed, 21 Jun 2023 15:04:14 +0100
From: Florian Fainelli <f.fainelli@...il.com>
To: Andrew Lunn <andrew@...n.ch>, netdev <netdev@...r.kernel.org>
Cc: Heiner Kallweit <hkallweit1@...il.com>, ansuelsmth@...il.com,
 Russell King <rmk+kernel@...linux.org.uk>, stable@...r.kernel.org
Subject: Re: [PATCH net] net: phy: Manual remove LEDs to ensure correct
 ordering

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:

# reboot -f
[   18.162000] bcmgenet 8f00000.ethernet eth0: Link is Down
[   18.305163] SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
[   18.305170] GISB: target abort at 0x8f00e14 [R ], core: cpu_0
[   18.305180] CPU: 2 PID: 41 Comm: kworker/2:1 Not tainted 
6.4.0-rc5-next-20230607-gc7a93fa22690 #98
[   18.305187] Hardware name: BCM972180HB_V20 (DT)
[   18.305191] Workqueue: events set_brightness_delayed
[   18.305214] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS 
BTYPE=--)
[   18.305220] pc : el1_abort+0x30/0x5c
[   18.305230] lr : el1_abort+0x24/0x5c
[   18.305235] sp : ffffffc082b73a90
[   18.305236] x29: ffffffc082b73a90 x28: ffffff8002fad780 x27: 
0000000000000000
[   18.305243] x26: 0000000000000000 x25: 0000000000000000 x24: 
ffffff807dbb340d
[   18.305250] x23: 0000000060000005 x22: ffffffc08066d9ac x21: 
0000000096000210
[   18.305256] x20: ffffffc082b55e14 x19: ffffffc082b73ad0 x18: 
0000000000000000
[   18.305263] x17: 74656e2f74656e72 x16: 656874652e303030 x15: 
303066382f626472
[   18.305269] x14: ffffff8004a84cd8 x13: 6e69622f7273752f x12: 
0000000000000000
[   18.305275] x11: ffffff8002d1c710 x10: 0000000000000870 x9 : 
ffffffc080667e34
[   18.305282] x8 : ffffff8003d44a80 x7 : fefefefefefefeff x6 : 
000073746e657665
[   18.305288] x5 : ffffff8003d44a80 x4 : ffffffc082b73ad0 x3 : 
0000000000000025
[   18.305294] x2 : 000000000000001c x1 : 0000000004208060 x0 : 
0000000000000000
[   18.305303] Kernel panic - not syncing: Asynchronous SError Interrupt
[   18.305306] CPU: 2 PID: 41 Comm: kworker/2:1 Not tainted 
6.4.0-rc5-next-20230607-gc7a93fa22690 #98
[   18.305311] Hardware name: BCM972180HB_V20 (DT)
[   18.305314] Workqueue: events set_brightness_delayed
[   18.305319] Call trace:
[   18.305321]  dump_backtrace+0xdc/0x114
[   18.305329]  show_stack+0x1c/0x28
[   18.305333]  dump_stack_lvl+0x44/0x58
[   18.305339]  dump_stack+0x14/0x1c
[   18.305342]  panic+0x128/0x2f8
[   18.305350]  nmi_panic+0x50/0x70
[   18.305356]  arm64_serror_panic+0x74/0x80
[   18.305361]  do_serror+0x2c/0x5c
[   18.305366]  el1h_64_error_handler+0x30/0x44
[   18.305372]  el1h_64_error+0x64/0x68
[   18.305378]  el1_abort+0x30/0x5c
[   18.305383]  el1h_64_sync_handler+0x64/0xc8
[   18.305389]  el1h_64_sync+0x64/0x68
[   18.305392]  readl_relaxed+0x0/0x8
[   18.305401]  __mdiobus_write+0x3c/0x94
[   18.305409]  mdiobus_write+0x4c/0x70
[   18.305415]  phy_write+0x1c/0x24
[   18.305419]  bcm_phy_read_shadow+0x24/0x40
[   18.305423]  bcm_phy_led_brightness_set+0x40/0x94
[   18.305428]  phy_led_set_brightness+0x48/0x68
[   18.305434]  set_brightness_delayed_set_brightness+0x44/0x7c
[   18.305443]  set_brightness_delayed+0xc4/0x1a4
[   18.305447]  process_one_work+0x1c0/0x284
[   18.305455]  process_scheduled_works+0x44/0x48
[   18.305461]  worker_thread+0x1e8/0x264
[   18.305467]  kthread+0xcc/0xdc
[   18.305474]  ret_from_fork+0x10/0x20
[   18.311812] Kernel Offset: disabled
[   18.311814] CPU features: 0x00000003,00010000,0000420b
[   18.311818] Memory Limit: none
[   18.566507] ---[ end Kernel panic - not syncing: Asynchronous SError 
Interrupt ]---

still not clear to me how the workqueue managed to execute and not 
finish before we unregistered the PHY device.
-- 
Florian