Commit 6d71021a authored by Luis Henriques's avatar Luis Henriques Committed by Jacek Anaszewski

leds: class: ensure workqueue is initialized before setting brightness

An application can try to set brightness before all the initialization is
done, in particular before the workqueue is initialized with the call to
led_init_core().  Here's a WARNING easy to trigger:

[   36.780813] WARNING: CPU: 3 PID: 1411 at ../kernel/workqueue.c:1444 __queue_work+0x37b/0x420
[   36.780815] Modules linked in: ...
[   36.780868] CPU: 3 PID: 1411 Comm: systemd-backlig Not tainted 4.16.9-1-default #1 openSUSE Tumbleweed (unreleased)
[   36.780868] Hardware name: Dell Inc. Precision 5510/0N8J4R, BIOS 1.6.1 12/11/2017
[   36.780870] RIP: 0010:__queue_work+0x37b/0x420
[   36.780871] RSP: 0018:ffffaced048b7d78 EFLAGS: 00010086
[   36.780873] RAX: 0000000000000000 RBX: ffffffffb3f01440 RCX: 0000000000000000
[   36.780873] RDX: ffffffffc05a90d8 RSI: 0000000000000000 RDI: ffff8eac7dce2700
[   36.780874] RBP: ffff8ea547c16400 R08: ffff8ea547800000 R09: ffff8eac7dc22700
[   36.780875] R10: 0000000000000000 R11: 0000000000000040 R12: 0000000000000003
[   36.780876] R13: 0000000000000200 R14: ffffffffc05a90d0 R15: ffff8eac7dce8600
[   36.780877] FS:  00007f871e61cf40(0000) GS:ffff8eac7dcc0000(0000) knlGS:0000000000000000
[   36.780878] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   36.780879] CR2: 000055c91115e308 CR3: 0000000883ee0005 CR4: 00000000003606e0
[   36.780880] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   36.780880] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   36.780881] Call Trace:
[   36.780886]  queue_work_on+0x81/0x90
[   36.780889]  brightness_store+0x5d/0x90
[   36.780892]  kernfs_fop_write+0x105/0x180
[   36.780894]  __vfs_write+0x26/0x150
[   36.780897]  ? common_file_perm+0x51/0x150
[   36.780900]  ? security_file_permission+0x3c/0xb0
[   36.780901]  vfs_write+0xad/0x1a0
[   36.780903]  SyS_write+0x42/0x90
[   36.780906]  do_syscall_64+0x76/0x140
[   36.780908]  entry_SYSCALL_64_after_hwframe+0x42/0xb7
[   36.780910] RIP: 0033:0x7f871dd04c94
[   36.780910] RSP: 002b:00007ffeb3a57d38 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[   36.780912] RAX: ffffffffffffffda RBX: 000055c91115c810 RCX: 00007f871dd04c94
[   36.780912] RDX: 0000000000000001 RSI: 000055c91115c810 RDI: 0000000000000004
[   36.780913] RBP: 00007ffeb3a57e10 R08: 0000000000000003 R09: 0000000000000000
[   36.780914] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[   36.780914] R13: 000055c911158f30 R14: 000055c90f3a9a4e R15: 0000000000000004
[   36.780917] Code: 74 18 e8 49 80 00 00 48 85 c0 74 0e 48 8b 40 20 48 3b 68 08
0f 84 c2 fc ff ff 0f 0b 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 <0f> 0b e9
82 fd ff ff 83 cd 02 49 8d 57 60 e9 69 fd ff ff 80 3d
[   36.780942] ---[ end trace 1fce4edad54c4017 ]---

This patch initializes and acquires the led_access mutex early in the
of_led_classdev_register function, so that any application trying to write
to sysfs to set brightness will block until initialization ends.
Signed-off-by: default avatarLuis Henriques <lhenriques@suse.com>
Acked-by: default avatarPavel Machek <pavel@ucw.cz>
Signed-off-by: default avatarJacek Anaszewski <jacek.anaszewski@gmail.com>
parent 57a53297
...@@ -260,10 +260,14 @@ int of_led_classdev_register(struct device *parent, struct device_node *np, ...@@ -260,10 +260,14 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,
if (ret < 0) if (ret < 0)
return ret; return ret;
mutex_init(&led_cdev->led_access);
mutex_lock(&led_cdev->led_access);
led_cdev->dev = device_create_with_groups(leds_class, parent, 0, led_cdev->dev = device_create_with_groups(leds_class, parent, 0,
led_cdev, led_cdev->groups, "%s", name); led_cdev, led_cdev->groups, "%s", name);
if (IS_ERR(led_cdev->dev)) if (IS_ERR(led_cdev->dev)) {
mutex_unlock(&led_cdev->led_access);
return PTR_ERR(led_cdev->dev); return PTR_ERR(led_cdev->dev);
}
led_cdev->dev->of_node = np; led_cdev->dev->of_node = np;
if (ret) if (ret)
...@@ -274,6 +278,7 @@ int of_led_classdev_register(struct device *parent, struct device_node *np, ...@@ -274,6 +278,7 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,
ret = led_add_brightness_hw_changed(led_cdev); ret = led_add_brightness_hw_changed(led_cdev);
if (ret) { if (ret) {
device_unregister(led_cdev->dev); device_unregister(led_cdev->dev);
mutex_unlock(&led_cdev->led_access);
return ret; return ret;
} }
} }
...@@ -285,7 +290,6 @@ int of_led_classdev_register(struct device *parent, struct device_node *np, ...@@ -285,7 +290,6 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,
#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
led_cdev->brightness_hw_changed = -1; led_cdev->brightness_hw_changed = -1;
#endif #endif
mutex_init(&led_cdev->led_access);
/* add to the list of leds */ /* add to the list of leds */
down_write(&leds_list_lock); down_write(&leds_list_lock);
list_add_tail(&led_cdev->node, &leds_list); list_add_tail(&led_cdev->node, &leds_list);
...@@ -302,6 +306,8 @@ int of_led_classdev_register(struct device *parent, struct device_node *np, ...@@ -302,6 +306,8 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,
led_trigger_set_default(led_cdev); led_trigger_set_default(led_cdev);
#endif #endif
mutex_unlock(&led_cdev->led_access);
dev_dbg(parent, "Registered led device: %s\n", dev_dbg(parent, "Registered led device: %s\n",
led_cdev->name); led_cdev->name);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment