All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] r8169: keep LED device name valid after setup
@ 2026-08-13 10:07 Zhixing Chen
  2026-08-13 15:17 ` Andrew Lunn
  0 siblings, 1 reply; 2+ messages in thread
From: Zhixing Chen @ 2026-08-13 10:07 UTC (permalink / raw)
  To: Heiner Kallweit, nic_swsd
  Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, Zhixing Chen

rtl8168_setup_ldev() and rtl8125_setup_led_ldev() build the LED device
name in a stack buffer and assign it to led_cdev->name.

The LED class device registration path reads led_cdev->name after it has
been assigned, and struct led_classdev stores the name as part of the LED
class device state. Do not keep a pointer to a setup function's stack
buffer there.

Store the name in struct r8169_led_classdev instead, so it remains valid
for the lifetime of the LED class device.

Signed-off-by: Zhixing Chen <running910@gmail.com>
---

I noticed this while reviewing the r8169 driver code. The LED name is
stored in led_cdev->name, but the current buffer lifetime does not match
the LED class device lifetime.

I have not seen a crash from this, but led_cdev->name points to a stack
buffer after setup returns. Storing the name in the per-LED private object
keeps the pointer lifetime aligned with the LED class device.

---
 drivers/net/ethernet/realtek/r8169_leds.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_leds.c b/drivers/net/ethernet/realtek/r8169_leds.c
index 1999e81f0bca..5a2067be3095 100644
--- a/drivers/net/ethernet/realtek/r8169_leds.c
+++ b/drivers/net/ethernet/realtek/r8169_leds.c
@@ -31,6 +31,7 @@ struct r8169_led_classdev {
 	struct led_classdev led;
 	struct net_device *ndev;
 	int index;
+	char name[LED_MAX_NAME_SIZE];
 };
 
 #define lcdev_to_r8169_ldev(lcdev) container_of(lcdev, struct r8169_led_classdev, led)
@@ -131,13 +132,12 @@ static void rtl8168_setup_ldev(struct r8169_led_classdev *ldev,
 {
 	struct rtl8169_private *tp = netdev_priv(ndev);
 	struct led_classdev *led_cdev = &ldev->led;
-	char led_name[LED_MAX_NAME_SIZE];
 
 	ldev->ndev = ndev;
 	ldev->index = index;
 
-	r8169_get_led_name(tp, index, led_name, LED_MAX_NAME_SIZE);
-	led_cdev->name = led_name;
+	r8169_get_led_name(tp, index, ldev->name, sizeof(ldev->name));
+	led_cdev->name = ldev->name;
 	led_cdev->hw_control_trigger = "netdev";
 	led_cdev->flags |= LED_RETAIN_AT_SHUTDOWN;
 	led_cdev->hw_control_is_supported = rtl8168_led_hw_control_is_supported;
@@ -230,13 +230,12 @@ static void rtl8125_setup_led_ldev(struct r8169_led_classdev *ldev,
 {
 	struct rtl8169_private *tp = netdev_priv(ndev);
 	struct led_classdev *led_cdev = &ldev->led;
-	char led_name[LED_MAX_NAME_SIZE];
 
 	ldev->ndev = ndev;
 	ldev->index = index;
 
-	r8169_get_led_name(tp, index, led_name, LED_MAX_NAME_SIZE);
-	led_cdev->name = led_name;
+	r8169_get_led_name(tp, index, ldev->name, sizeof(ldev->name));
+	led_cdev->name = ldev->name;
 	led_cdev->hw_control_trigger = "netdev";
 	led_cdev->flags |= LED_RETAIN_AT_SHUTDOWN;
 	led_cdev->hw_control_is_supported = rtl8125_led_hw_control_is_supported;

base-commit: f6057f06ef7afa9893ed33603f7917fa39d237b5
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net-next] r8169: keep LED device name valid after setup
  2026-08-13 10:07 [PATCH net-next] r8169: keep LED device name valid after setup Zhixing Chen
@ 2026-08-13 15:17 ` Andrew Lunn
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Lunn @ 2026-08-13 15:17 UTC (permalink / raw)
  To: Zhixing Chen
  Cc: Heiner Kallweit, nic_swsd, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev

On Thu, Aug 13, 2026 at 06:07:11PM +0800, Zhixing Chen wrote:
> rtl8168_setup_ldev() and rtl8125_setup_led_ldev() build the LED device
> name in a stack buffer and assign it to led_cdev->name.
> 
> The LED class device registration path reads led_cdev->name after it has
> been assigned, and struct led_classdev stores the name as part of the LED
> class device state. Do not keep a pointer to a setup function's stack
> buffer there.
> 
> Store the name in struct r8169_led_classdev instead, so it remains valid
> for the lifetime of the LED class device.

led_classdev_register_ext() does make a copy of name, in order that it
can mangle the name to fit the naming scheme. led-class.c only
accesses led_cdev->name in led_classdev_register_ext(). So i think the
current code is safe.

However, it could become an issue in the future, if the LED core code
ever changes.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-13 15:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 10:07 [PATCH net-next] r8169: keep LED device name valid after setup Zhixing Chen
2026-08-13 15:17 ` Andrew Lunn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.