Skip to contentSkip to conversation

r8169: keep LED device name valid after setup

PATCH Applied Zhixing Chen 3 participants 3 messages Aug 13 – Aug 17

Subsystems (3)
Networking Network drivers 8169 10/100/1000 gigabit Ethernet driver
lore
Share & tools
lore

Zhixing Chen

author
#0 lore raw

PATCH 1/1 r8169: keep LED device name valid after setup

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 <[email protected]>

---

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(-)
drivers/net/ethernet/realtek/r8169_leds.c +5 -6
index 1999e81f0bca..5a2067be3095 100644
@@ -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

Signature · 1 line
2.34.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · 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.

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 <[email protected]>

Andrew

maintainer Applied
#2 lore raw
In reply to Zhixing Chen
Quoted 10 lines · On Thu, 13 Aug 2026 18:07:11 +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.

Applied

Signature · 1 line
pw-bot: accept