* [PATCH v3 0/2] HID: use spinlocks to safely schedule led workers @ 2023-02-09 23:58 Pietro Borrello 2023-02-09 23:58 ` [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers Pietro Borrello 2023-02-09 23:58 ` [PATCH v3 2/2] HID: asus: " Pietro Borrello 0 siblings, 2 replies; 11+ messages in thread From: Pietro Borrello @ 2023-02-09 23:58 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione Cc: Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Jiri Kosina, Roderick Colenbrander, linux-input, linux-kernel, Pietro Borrello I noticed a recurring pattern is present in multiple hid devices in the Linux tree, where the LED controller of a device schedules a work_struct to interact with the hardware. The work_struct is embedded in the device structure and thus, is freed at device removal. The issue is that a LED worker may be scheduled by a timer concurrently with device removal, causing the work_struct to be accessed after having been freed. I was able to trigger the issue in hid-bigbenff.c and hid-asus.c where the work_structs may be scheduled by the LED controller while the device is disconnecting, triggering use-after-frees. I can attach the reproducer, but it's very simple USB configuration, using the /dev/raw-gadget interface with some more USB interactions to manage LEDs configuration and pass checks in asus_kbd_init() and asus_kbd_get_functions() in case of hid-asus.c. I triggered the issue by connecting a device and immediately disconnecting it, so that the remove function runs before the LED one which remains pending. I am attaching multiple patches for asus and bigben drivers. The proposed patches introduce safe wrappers to schedule the workers safely with several spinlocks checks. I attach the (partial for brevity) ODEBUG dumps: ```hid-bigbenff.c [ 37.803135][ T1170] usb 1-1: USB disconnect, device number 2 [ 37.827979][ T1170] ODEBUG: free active (active state 0) object type: work_struct hint: bigben_worker+0x0/0x860 [ 37.829634][ T1170] WARNING: CPU: 0 PID: 1170 at lib/debugobjects.c:505 debug_check_no_obj_freed+0x43a/0x630 [ 37.830904][ T1170] Modules linked in: [ 37.831413][ T1170] CPU: 0 PID: 1170 Comm: kworker/0:3 Not tainted 6.1.0-rc4-dirty #43 [ 37.832465][ T1170] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 [ 37.833751][ T1170] Workqueue: usb_hub_wq hub_event [ 37.834409][ T1170] RIP: 0010:debug_check_no_obj_freed+0x43a/0x630 [ 37.835218][ T1170] Code: 48 89 ef e8 28 82 58 ff 49 8b 14 24 4c 8b 45 00 48 c7 c7 40 5f 09 87 48 c7 c6 60 5b 09 87 89 d9 4d 89 f9 31 c0 e8 46 25 ef fe <0f> 0b 4c 8b 64 24 20 48 ba 00 00 00 00 00 fc ff df ff 05 4f 7c 17 [ 37.837667][ T1170] RSP: 0018:ffffc900006fee60 EFLAGS: 00010246 [ 37.838503][ T1170] RAX: 0d2d19ffcded3d00 RBX: 0000000000000000 RCX: ffff888117fc9b00 [ 37.839519][ T1170] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 [ 37.840570][ T1170] RBP: ffffffff86e88380 R08: ffffffff8130793b R09: fffff520000dfd85 [ 37.841618][ T1170] R10: fffff520000dfd85 R11: 0000000000000000 R12: ffffffff87095fb8 [ 37.842649][ T1170] R13: ffff888117770ad8 R14: ffff888117770acc R15: ffffffff852b7420 [ 37.843728][ T1170] FS: 0000000000000000(0000) GS:ffff8881f6600000(0000) knlGS:0000000000000000 [ 37.844877][ T1170] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 37.845749][ T1170] CR2: 00007f992eaab380 CR3: 000000011834b000 CR4: 00000000001006f0 [ 37.846794][ T1170] Call Trace: [ 37.847245][ T1170] <TASK> [ 37.847643][ T1170] slab_free_freelist_hook+0x89/0x160 [ 37.848409][ T1170] ? devres_release_all+0x262/0x350 [ 37.849156][ T1170] __kmem_cache_free+0x71/0x110 [ 37.849829][ T1170] devres_release_all+0x262/0x350 [ 37.850478][ T1170] ? devres_release+0x90/0x90 [ 37.851118][ T1170] device_release_driver_internal+0x5e5/0x8a0 [ 37.851944][ T1170] bus_remove_device+0x2ea/0x400 [ 37.852611][ T1170] device_del+0x64f/0xb40 [ 37.853212][ T1170] ? kill_device+0x150/0x150 [ 37.853831][ T1170] ? print_irqtrace_events+0x1f0/0x1f0 [ 37.854564][ T1170] hid_destroy_device+0x66/0x100 [ 37.855226][ T1170] usbhid_disconnect+0x9a/0xc0 [ 37.855887][ T1170] usb_unbind_interface+0x1e1/0x890 ``` ``` hid-asus.c [ 77.409878][ T1169] usb 1-1: USB disconnect, device number 2 [ 77.423606][ T1169] ODEBUG: free active (active state 0) object type: work_struct hint: asus_kbd_backlight_work+0x0/0x2c0 [ 77.425222][ T1169] WARNING: CPU: 0 PID: 1169 at lib/debugobjects.c:505 debug_check_no_obj_freed+0x43a/0x630 [ 77.426599][ T1169] Modules linked in: [ 77.427322][ T1169] CPU: 0 PID: 1169 Comm: kworker/0:3 Not tainted 6.1.0-rc4-dirty #43 [ 77.428404][ T1169] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 [ 77.429644][ T1169] Workqueue: usb_hub_wq hub_event [ 77.430296][ T1169] RIP: 0010:debug_check_no_obj_freed+0x43a/0x630 [ 77.431142][ T1169] Code: 48 89 ef e8 28 82 58 ff 49 8b 14 24 4c 8b 45 00 48 c7 c7 40 5f 09 87 48 c7 c6 60 5b 09 87 89 d9 4d 89 f9 31 c0 e8 46 25 ef fe <0f> 0b 4c 8b 64 24 20 48 ba 00 00 00 00 00 fc ff df ff 05 4f 7c 17 [ 77.433691][ T1169] RSP: 0018:ffffc9000069ee60 EFLAGS: 00010246 [ 77.434470][ T1169] RAX: b85d2b40c12d7600 RBX: 0000000000000000 RCX: ffff888117a78000 [ 77.435507][ T1169] RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000 [ 77.436521][ T1169] RBP: ffffffff86e88380 R08: ffffffff8130793b R09: ffffed103ecc4ed6 [ 77.437582][ T1169] R10: ffffed103ecc4ed6 R11: 0000000000000000 R12: ffffffff87095fb8 [ 77.438593][ T1169] R13: ffff88810e348fe0 R14: ffff88810e348fd4 R15: ffffffff852b5780 [ 77.439667][ T1169] FS: 0000000000000000(0000) GS:ffff8881f6600000(0000) knlGS:0000000000000000 [ 77.440842][ T1169] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 77.441688][ T1169] CR2: 00007ffc05495ff0 CR3: 000000010cdf0000 CR4: 00000000001006f0 [ 77.442720][ T1169] Call Trace: [ 77.443167][ T1169] <TASK> [ 77.443555][ T1169] slab_free_freelist_hook+0x89/0x160 [ 77.444302][ T1169] ? devres_release_all+0x262/0x350 [ 77.444990][ T1169] __kmem_cache_free+0x71/0x110 [ 77.445638][ T1169] devres_release_all+0x262/0x350 [ 77.446309][ T1169] ? devres_release+0x90/0x90 [ 77.446978][ T1169] device_release_driver_internal+0x5e5/0x8a0 [ 77.447748][ T1169] bus_remove_device+0x2ea/0x400 [ 77.448421][ T1169] device_del+0x64f/0xb40 [ 77.448976][ T1169] ? kill_device+0x150/0x150 [ 77.449577][ T1169] ? print_irqtrace_events+0x1f0/0x1f0 [ 77.450307][ T1169] hid_destroy_device+0x66/0x100 [ 77.450938][ T1169] usbhid_disconnect+0x9a/0xc0 ``` Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it> --- Changes in v3: - use spinlocks to prevent workers scheduling - drop patches on sony & playstation hid drivers - Link to v2: https://lore.kernel.org/r/20230125-hid-unregister-leds-v2-0-689cc62fc878@diag.uniroma1.it Changes in v2: - dualshock4: Clarify UAF - dualsense: Clarify UAF - dualsense: Unregister multicolor led controller - Link to v1: https://lore.kernel.org/r/20230125-hid-unregister-leds-v1-0-9a5192dcef16@diag.uniroma1.it --- Pietro Borrello (2): HID: bigben: use spinlock to safely schedule workers HID: asus: use spinlock to safely schedule workers drivers/hid/hid-asus.c | 24 +++++++++++++++++++++--- drivers/hid/hid-bigbenff.c | 34 +++++++++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 8 deletions(-) --- base-commit: 2241ab53cbb5cdb08a6b2d4688feb13971058f65 change-id: 20230125-hid-unregister-leds-4cbf67099e1d Best regards, -- Pietro Borrello <borrello@diag.uniroma1.it> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers 2023-02-09 23:58 [PATCH v3 0/2] HID: use spinlocks to safely schedule led workers Pietro Borrello @ 2023-02-09 23:58 ` Pietro Borrello [not found] ` <20230210132017.2497-1-hdanton@sina.com> 2023-02-10 14:26 ` Benjamin Tissoires 2023-02-09 23:58 ` [PATCH v3 2/2] HID: asus: " Pietro Borrello 1 sibling, 2 replies; 11+ messages in thread From: Pietro Borrello @ 2023-02-09 23:58 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione Cc: Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Jiri Kosina, Roderick Colenbrander, linux-input, linux-kernel, Pietro Borrello Use spinlocks to deal with workers introducing a wrapper bigben_schedule_work(), and several spinlock checks. Otherwise, bigben_set_led() may schedule bigben->worker after the structure has been freed, causing a use-after-free. Fixes: 4eb1b01de5b9 ("HID: hid-bigbenff: fix race condition for scheduled work during removal") Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it> --- drivers/hid/hid-bigbenff.c | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c index e8b16665860d..28769aa7fed6 100644 --- a/drivers/hid/hid-bigbenff.c +++ b/drivers/hid/hid-bigbenff.c @@ -174,6 +174,7 @@ static __u8 pid0902_rdesc_fixed[] = { struct bigben_device { struct hid_device *hid; struct hid_report *report; + spinlock_t lock; bool removed; u8 led_state; /* LED1 = 1 .. LED4 = 8 */ u8 right_motor_on; /* right motor off/on 0/1 */ @@ -184,15 +185,24 @@ struct bigben_device { struct work_struct worker; }; +static inline void bigben_schedule_work(struct bigben_device *bigben) +{ + unsigned long flags; + + spin_lock_irqsave(&bigben->lock, flags); + if (!bigben->removed) + schedule_work(&bigben->worker); + spin_unlock_irqrestore(&bigben->lock, flags); +} static void bigben_worker(struct work_struct *work) { struct bigben_device *bigben = container_of(work, struct bigben_device, worker); struct hid_field *report_field = bigben->report->field[0]; + unsigned long flags; - if (bigben->removed || !report_field) - return; + spin_lock_irqsave(&bigben->lock, flags); if (bigben->work_led) { bigben->work_led = false; @@ -219,6 +229,8 @@ static void bigben_worker(struct work_struct *work) report_field->value[7] = 0x00; /* padding */ hid_hw_request(bigben->hid, bigben->report, HID_REQ_SET_REPORT); } + + spin_unlock_irqrestore(&bigben->lock, flags); } static int hid_bigben_play_effect(struct input_dev *dev, void *data, @@ -228,6 +240,7 @@ static int hid_bigben_play_effect(struct input_dev *dev, void *data, struct bigben_device *bigben = hid_get_drvdata(hid); u8 right_motor_on; u8 left_motor_force; + unsigned long flags; if (!bigben) { hid_err(hid, "no device data\n"); @@ -242,10 +255,13 @@ static int hid_bigben_play_effect(struct input_dev *dev, void *data, if (right_motor_on != bigben->right_motor_on || left_motor_force != bigben->left_motor_force) { + spin_lock_irqsave(&bigben->lock, flags); bigben->right_motor_on = right_motor_on; bigben->left_motor_force = left_motor_force; bigben->work_ff = true; - schedule_work(&bigben->worker); + spin_unlock_irqrestore(&bigben->lock, flags); + + bigben_schedule_work(bigben); } return 0; @@ -259,6 +275,7 @@ static void bigben_set_led(struct led_classdev *led, struct bigben_device *bigben = hid_get_drvdata(hid); int n; bool work; + unsigned long flags; if (!bigben) { hid_err(hid, "no device data\n"); @@ -267,6 +284,7 @@ static void bigben_set_led(struct led_classdev *led, for (n = 0; n < NUM_LEDS; n++) { if (led == bigben->leds[n]) { + spin_lock_irqsave(&bigben->lock, flags); if (value == LED_OFF) { work = (bigben->led_state & BIT(n)); bigben->led_state &= ~BIT(n); @@ -274,10 +292,11 @@ static void bigben_set_led(struct led_classdev *led, work = !(bigben->led_state & BIT(n)); bigben->led_state |= BIT(n); } + spin_unlock_irqrestore(&bigben->lock, flags); if (work) { bigben->work_led = true; - schedule_work(&bigben->worker); + bigben_schedule_work(bigben); } return; } @@ -307,8 +326,12 @@ static enum led_brightness bigben_get_led(struct led_classdev *led) static void bigben_remove(struct hid_device *hid) { struct bigben_device *bigben = hid_get_drvdata(hid); + unsigned long flags; + spin_lock_irqsave(&bigben->lock, flags); bigben->removed = true; + spin_unlock_irqrestore(&bigben->lock, flags); + cancel_work_sync(&bigben->worker); hid_hw_stop(hid); } @@ -362,6 +385,7 @@ static int bigben_probe(struct hid_device *hid, set_bit(FF_RUMBLE, hidinput->input->ffbit); INIT_WORK(&bigben->worker, bigben_worker); + spin_lock_init(&bigben->lock); error = input_ff_create_memless(hidinput->input, NULL, hid_bigben_play_effect); @@ -402,7 +426,7 @@ static int bigben_probe(struct hid_device *hid, bigben->left_motor_force = 0; bigben->work_led = true; bigben->work_ff = true; - schedule_work(&bigben->worker); + bigben_schedule_work(bigben); hid_info(hid, "LED and force feedback support for BigBen gamepad\n"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
[parent not found: <20230210132017.2497-1-hdanton@sina.com>]
* Re: [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers [not found] ` <20230210132017.2497-1-hdanton@sina.com> @ 2023-02-10 14:11 ` Benjamin Tissoires [not found] ` <20230211020041.2613-1-hdanton@sina.com> 0 siblings, 1 reply; 11+ messages in thread From: Benjamin Tissoires @ 2023-02-10 14:11 UTC (permalink / raw) To: Hillf Danton Cc: Pietro Borrello, Jiri Kosina, Hanno Zulla, linux-input, linux-kernel On Fri, Feb 10, 2023 at 2:24 PM Hillf Danton <hdanton@sina.com> wrote: > > On Thu, 09 Feb 2023 23:58:55 +0000 Pietro Borrello <borrello@diag.uniroma1.it> > > Use spinlocks to deal with workers introducing a wrapper > > bigben_schedule_work(), and several spinlock checks. > > Otherwise, bigben_set_led() may schedule bigben->worker after the > > structure has been freed, causing a use-after-free. > > > > Fixes: 4eb1b01de5b9 ("HID: hid-bigbenff: fix race condition for scheduled work during removal") > > Given the flag added in 4eb1b01de5b9 and the spinlock added in this > patchset, devm_led_classdev_register() looks to not work for you. Actually, looking at the code now, it is clear that we need that lock. The current code is happily changing the struct bigben_device from multiple contexts, and pulls that without any barrier in the work struct which should produce some interesting results :) And we can probably abuse that lock to prevent scheduling a new work as it is done in hid-playstation.c I'll comment in the patch which parts need to be changed, because it is true that this patch is definitely not mergeable as such and will need another revision. > > How about replacing the advanced devm_ method with the traditional plain > pair of led_classdev_un/register(), with the flag mentioned cut off but > without bothering to add another lock? > As mentioned above, the lock is needed anyway, and will probably need to be added in a separate patch. Reverting to a non devm version of the led class would complexify the driver for the error paths, and is probably not the best move IMO. Cheers, Benjamin ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20230211020041.2613-1-hdanton@sina.com>]
* Re: [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers [not found] ` <20230211020041.2613-1-hdanton@sina.com> @ 2023-02-13 8:25 ` Benjamin Tissoires [not found] ` <20230213103658.3091-1-hdanton@sina.com> 0 siblings, 1 reply; 11+ messages in thread From: Benjamin Tissoires @ 2023-02-13 8:25 UTC (permalink / raw) To: Hillf Danton Cc: Pietro Borrello, Jiri Kosina, Hanno Zulla, linux-input, linux-kernel On Sat, Feb 11, 2023 at 3:01 AM Hillf Danton <hdanton@sina.com> wrote: > > On Fri, 10 Feb 2023 15:11:26 +0100 Benjamin Tissoires <benjamin.tissoires@redhat.com> > > On Fri, Feb 10, 2023 at 2:24 PM Hillf Danton <hdanton@sina.com> wrote: > > > On Thu, 09 Feb 2023 23:58:55 +0000 Pietro Borrello <borrello@diag.uniroma1.it> > > > > Use spinlocks to deal with workers introducing a wrapper > > > > bigben_schedule_work(), and several spinlock checks. > > > > Otherwise, bigben_set_led() may schedule bigben->worker after the > > > > structure has been freed, causing a use-after-free. > > > > > > > > Fixes: 4eb1b01de5b9 ("HID: hid-bigbenff: fix race condition for scheduled work during removal") > > > > > > Given the flag added in 4eb1b01de5b9 and the spinlock added in this > > > patchset, devm_led_classdev_register() looks to not work for you. > > > > Actually, looking at the code now, it is clear that we need that lock. > > The current code is happily changing the struct bigben_device from > > multiple contexts, and pulls that without any barrier in the work > > struct which should produce some interesting results :) > > > > And we can probably abuse that lock to prevent scheduling a new work > > as it is done in hid-playstation.c > > > > I'll comment in the patch which parts need to be changed, because it > > is true that this patch is definitely not mergeable as such and will > > need another revision. > > > > > > > > How about replacing the advanced devm_ method with the traditional plain > > > pair of led_classdev_un/register(), with the flag mentioned cut off but > > > without bothering to add another lock? > > > > > > > As mentioned above, the lock is needed anyway, and will probably need > > to be added in a separate patch. > > Reverting to a non devm version of the led class would complexify the > > driver for the error paths, and is probably not the best move IMO. > > After this patch, > > cpu 0 cpu 2 > --- --- > bigben_remove() > spin_lock_irqsave(&bigben->lock, flags); > bigben->removed = true; > spin_unlock_irqrestore(&bigben->lock, flags); > > spin_lock_irqsave(&bigben->lock, flags); > > what makes it safe for cpu2 to acquire lock after the removed flag is true? The remove flag is just a way to prevent any other workqueue from starting. It doesn't mean that the struct bigben has been freed, so acquiring a lock at that point is fine. We then rely on 2 things: - devm_class_led to be released before struct bigben, because it was devm-allocated *after* the struct bigben was devm-allocated - we prevent any new workqueue to start and we guarantee that any running workqueue is terminated before leaving the .remove function. Given that the ledclass is gracefully shutting down all of its potential queues, we don't have any other possibility to have an unsafe call AFAIU. Cheers, Benjamin ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20230213103658.3091-1-hdanton@sina.com>]
* Re: [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers [not found] ` <20230213103658.3091-1-hdanton@sina.com> @ 2023-02-13 10:47 ` Benjamin Tissoires 0 siblings, 0 replies; 11+ messages in thread From: Benjamin Tissoires @ 2023-02-13 10:47 UTC (permalink / raw) To: Hillf Danton Cc: Pietro Borrello, Jiri Kosina, Hanno Zulla, linux-input, linux-kernel On Mon, Feb 13, 2023 at 11:37 AM Hillf Danton <hdanton@sina.com> wrote: > > On Mon, 13 Feb 2023 09:25:37 +0100 Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > > The remove flag is just a way to prevent any other workqueue from > > starting. It doesn't mean that the struct bigben has been freed, so > > acquiring a lock at that point is fine. > > We then rely on 2 things: > > - devm_class_led to be released before struct bigben, because it was > > devm-allocated *after* the struct bigben was devm-allocated > > This is the current behavior and it is intact after this patch. > > > - we prevent any new workqueue to start and we guarantee that any > > running workqueue is terminated before leaving the .remove function. > > If spinlock is added for not scheduling new workqueue then it is not > needed, because the removed flag is set before running workqueue is > terminated. Checking the flag is enough upon queuing new work. > I tend to disagree (based on Pietro's v4: - no worker is running - a led sysfs call is made - the line "if (!bigben->removed)" is true - this gets interrupted/or another CPU kicks in for the next one -> .remove gets called - bigben->removed is set to false - cancel_work_sync() called the led call continues, and schedules the work .removes terminates, and devm kicks in, killing led_class and struct bigben while the workqueue is running. So having a simple spinlocks ensures the atomicity between checking for bigben->removed and scheduling a workqueue. Cheers, Benjamin ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers 2023-02-09 23:58 ` [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers Pietro Borrello [not found] ` <20230210132017.2497-1-hdanton@sina.com> @ 2023-02-10 14:26 ` Benjamin Tissoires 2023-02-11 22:27 ` Pietro Borrello 1 sibling, 1 reply; 11+ messages in thread From: Benjamin Tissoires @ 2023-02-10 14:26 UTC (permalink / raw) To: Pietro Borrello Cc: Jiri Kosina, Hanno Zulla, Carlo Caione, Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Hillf Danton, Roderick Colenbrander, linux-input, linux-kernel On Feb 09 2023, Pietro Borrello wrote: > Use spinlocks to deal with workers introducing a wrapper > bigben_schedule_work(), and several spinlock checks. > Otherwise, bigben_set_led() may schedule bigben->worker after the > structure has been freed, causing a use-after-free. > > Fixes: 4eb1b01de5b9 ("HID: hid-bigbenff: fix race condition for scheduled work during removal") > Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it> > --- > drivers/hid/hid-bigbenff.c | 34 +++++++++++++++++++++++++++++----- > 1 file changed, 29 insertions(+), 5 deletions(-) > > diff --git a/drivers/hid/hid-bigbenff.c b/drivers/hid/hid-bigbenff.c > index e8b16665860d..28769aa7fed6 100644 > --- a/drivers/hid/hid-bigbenff.c > +++ b/drivers/hid/hid-bigbenff.c > @@ -174,6 +174,7 @@ static __u8 pid0902_rdesc_fixed[] = { > struct bigben_device { > struct hid_device *hid; > struct hid_report *report; > + spinlock_t lock; > bool removed; > u8 led_state; /* LED1 = 1 .. LED4 = 8 */ > u8 right_motor_on; /* right motor off/on 0/1 */ > @@ -184,15 +185,24 @@ struct bigben_device { > struct work_struct worker; > }; > > +static inline void bigben_schedule_work(struct bigben_device *bigben) > +{ > + unsigned long flags; > + > + spin_lock_irqsave(&bigben->lock, flags); > + if (!bigben->removed) > + schedule_work(&bigben->worker); > + spin_unlock_irqrestore(&bigben->lock, flags); > +} > > static void bigben_worker(struct work_struct *work) > { > struct bigben_device *bigben = container_of(work, > struct bigben_device, worker); > struct hid_field *report_field = bigben->report->field[0]; > + unsigned long flags; > > - if (bigben->removed || !report_field) You are removing an important test here: if (!report_field), please keep it. > - return; > + spin_lock_irqsave(&bigben->lock, flags); > > if (bigben->work_led) { > bigben->work_led = false; > @@ -219,6 +229,8 @@ static void bigben_worker(struct work_struct *work) > report_field->value[7] = 0x00; /* padding */ > hid_hw_request(bigben->hid, bigben->report, HID_REQ_SET_REPORT); > } > + > + spin_unlock_irqrestore(&bigben->lock, flags); Ouch, having hid_hw_request() called whithin a spinlock is definitely not something that should be done. However, the spinlock should be protecting 2 kinds of things: - any access to any value of struct bigben_device, but in an atomic way (i.e. copy everything you need locally in a spinlock, then release it and never read that struct again in that function). - the access to bigben->removed, which should be checked only in bigben_schedule_work() and in the .remove() function. Please note that this is what the playstation driver does: it prepares the report under the spinlock (which is really fast) before sending the report to the device which can be slow and be interrupted. With that being said, it is clear that we need 2 patches for this one: - the first one introduces the spinlock and protects the concurrent accesses to struct bigben_device (which is roughly everything below with the changes I just said) - the second one introduces bigben_schedule_work() and piggy backs on top of that new lock. Cheers, Benjamin > } > > static int hid_bigben_play_effect(struct input_dev *dev, void *data, > @@ -228,6 +240,7 @@ static int hid_bigben_play_effect(struct input_dev *dev, void *data, > struct bigben_device *bigben = hid_get_drvdata(hid); > u8 right_motor_on; > u8 left_motor_force; > + unsigned long flags; > > if (!bigben) { > hid_err(hid, "no device data\n"); > @@ -242,10 +255,13 @@ static int hid_bigben_play_effect(struct input_dev *dev, void *data, > > if (right_motor_on != bigben->right_motor_on || > left_motor_force != bigben->left_motor_force) { > + spin_lock_irqsave(&bigben->lock, flags); > bigben->right_motor_on = right_motor_on; > bigben->left_motor_force = left_motor_force; > bigben->work_ff = true; > - schedule_work(&bigben->worker); > + spin_unlock_irqrestore(&bigben->lock, flags); > + > + bigben_schedule_work(bigben); > } > > return 0; > @@ -259,6 +275,7 @@ static void bigben_set_led(struct led_classdev *led, > struct bigben_device *bigben = hid_get_drvdata(hid); > int n; > bool work; > + unsigned long flags; > > if (!bigben) { > hid_err(hid, "no device data\n"); > @@ -267,6 +284,7 @@ static void bigben_set_led(struct led_classdev *led, > > for (n = 0; n < NUM_LEDS; n++) { > if (led == bigben->leds[n]) { > + spin_lock_irqsave(&bigben->lock, flags); > if (value == LED_OFF) { > work = (bigben->led_state & BIT(n)); > bigben->led_state &= ~BIT(n); > @@ -274,10 +292,11 @@ static void bigben_set_led(struct led_classdev *led, > work = !(bigben->led_state & BIT(n)); > bigben->led_state |= BIT(n); > } > + spin_unlock_irqrestore(&bigben->lock, flags); > > if (work) { > bigben->work_led = true; > - schedule_work(&bigben->worker); > + bigben_schedule_work(bigben); > } > return; > } > @@ -307,8 +326,12 @@ static enum led_brightness bigben_get_led(struct led_classdev *led) > static void bigben_remove(struct hid_device *hid) > { > struct bigben_device *bigben = hid_get_drvdata(hid); > + unsigned long flags; > > + spin_lock_irqsave(&bigben->lock, flags); > bigben->removed = true; > + spin_unlock_irqrestore(&bigben->lock, flags); > + > cancel_work_sync(&bigben->worker); > hid_hw_stop(hid); > } > @@ -362,6 +385,7 @@ static int bigben_probe(struct hid_device *hid, > set_bit(FF_RUMBLE, hidinput->input->ffbit); > > INIT_WORK(&bigben->worker, bigben_worker); > + spin_lock_init(&bigben->lock); > > error = input_ff_create_memless(hidinput->input, NULL, > hid_bigben_play_effect); > @@ -402,7 +426,7 @@ static int bigben_probe(struct hid_device *hid, > bigben->left_motor_force = 0; > bigben->work_led = true; > bigben->work_ff = true; > - schedule_work(&bigben->worker); > + bigben_schedule_work(bigben); > > hid_info(hid, "LED and force feedback support for BigBen gamepad\n"); > > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers 2023-02-10 14:26 ` Benjamin Tissoires @ 2023-02-11 22:27 ` Pietro Borrello 0 siblings, 0 replies; 11+ messages in thread From: Pietro Borrello @ 2023-02-11 22:27 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jiri Kosina, Hanno Zulla, Carlo Caione, Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Hillf Danton, Roderick Colenbrander, linux-input, linux-kernel On Fri, 10 Feb 2023 at 15:26, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > [...] > > > > - if (bigben->removed || !report_field) > > You are removing an important test here: if (!report_field), please keep > it. To my understanding, that check was added in commit 918aa1ef104d ("HID: bigbenff: prevent null pointer dereference") to prevent the NULL pointer crash, only fixing the crash point. However, the true root cause was a missing check for output reports patched in commit c7bf714f8755 ("HID: check empty report_list in bigben_probe()"), where the type-confused report list_entry was overlapping with a NULL pointer, which was then causing the crash. Let me know if there is any other path that may result in having a report with no fields. In that case, it would make sense to keep the check. > > > - return; > > + spin_lock_irqsave(&bigben->lock, flags); > > > > if (bigben->work_led) { > > bigben->work_led = false; > > @@ -219,6 +229,8 @@ static void bigben_worker(struct work_struct *work) > > report_field->value[7] = 0x00; /* padding */ > > hid_hw_request(bigben->hid, bigben->report, HID_REQ_SET_REPORT); > > } > > + > > + spin_unlock_irqrestore(&bigben->lock, flags); > > Ouch, having hid_hw_request() called whithin a spinlock is definitely not > something that should be done. > > However, the spinlock should be protecting 2 kinds of things: > - any access to any value of struct bigben_device, but in an atomic way > (i.e. copy everything you need locally in a spinlock, then release it > and never read that struct again in that function). > - the access to bigben->removed, which should be checked only in > bigben_schedule_work() and in the .remove() function. > > Please note that this is what the playstation driver does: it prepares > the report under the spinlock (which is really fast) before sending the > report to the device which can be slow and be interrupted. > > With that being said, it is clear that we need 2 patches for this one: > - the first one introduces the spinlock and protects the concurrent > accesses to struct bigben_device (which is roughly everything below > with the changes I just said) > - the second one introduces bigben_schedule_work() and piggy backs on > top of that new lock. Thanks for clarifying. I will work on a v4 patch. Best regards, Pietro ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/2] HID: asus: use spinlock to safely schedule workers 2023-02-09 23:58 [PATCH v3 0/2] HID: use spinlocks to safely schedule led workers Pietro Borrello 2023-02-09 23:58 ` [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers Pietro Borrello @ 2023-02-09 23:58 ` Pietro Borrello 2023-02-10 14:58 ` Benjamin Tissoires 2023-02-14 7:58 ` Dan Carpenter 1 sibling, 2 replies; 11+ messages in thread From: Pietro Borrello @ 2023-02-09 23:58 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione Cc: Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Jiri Kosina, Roderick Colenbrander, linux-input, linux-kernel, Pietro Borrello Use spinlocks to deal with workers introducing a wrapper asus_schedule_work(), and several spinlock checks. Otherwise, asus_kbd_backlight_set() may schedule led->work after the structure has been freed, causing a use-after-free. Fixes: af22a610bc38 ("HID: asus: support backlight on USB keyboards") Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it> --- drivers/hid/hid-asus.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index f99752b998f3..30e194803bd7 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c @@ -98,6 +98,7 @@ struct asus_kbd_leds { struct hid_device *hdev; struct work_struct work; unsigned int brightness; + spinlock_t lock; bool removed; }; @@ -490,13 +491,23 @@ static int rog_nkey_led_init(struct hid_device *hdev) return ret; } +static void asus_schedule_work(struct asus_kbd_leds *led) +{ + unsigned long flags; + + spin_lock_irqsave(&led->lock, flags); + if (!led->removed) + schedule_work(&led->work); + spin_unlock_irqrestore(&led->lock, flags); +} + static void asus_kbd_backlight_set(struct led_classdev *led_cdev, enum led_brightness brightness) { struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds, cdev); led->brightness = brightness; - schedule_work(&led->work); + asus_schedule_work(led); } static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev) @@ -512,15 +523,17 @@ static void asus_kbd_backlight_work(struct work_struct *work) struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work); u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 }; int ret; + unsigned long flags; - if (led->removed) - return; + spin_lock_irqsave(&led->lock, flags); buf[4] = led->brightness; ret = asus_kbd_set_report(led->hdev, buf, sizeof(buf)); if (ret < 0) hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret); + + spin_unlock_irqrestore(&led->lock, flags); } /* WMI-based keyboard backlight LED control (via asus-wmi driver) takes @@ -584,6 +597,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev) drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set; drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get; INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work); + spin_lock_init(&drvdata->kbd_backlight->lock); ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev); if (ret < 0) { @@ -1119,9 +1133,13 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id) static void asus_remove(struct hid_device *hdev) { struct asus_drvdata *drvdata = hid_get_drvdata(hdev); + unsigned long flags; if (drvdata->kbd_backlight) { + spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags); drvdata->kbd_backlight->removed = true; + spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags); + cancel_work_sync(&drvdata->kbd_backlight->work); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] HID: asus: use spinlock to safely schedule workers 2023-02-09 23:58 ` [PATCH v3 2/2] HID: asus: " Pietro Borrello @ 2023-02-10 14:58 ` Benjamin Tissoires 2023-02-14 7:58 ` Dan Carpenter 1 sibling, 0 replies; 11+ messages in thread From: Benjamin Tissoires @ 2023-02-10 14:58 UTC (permalink / raw) To: Pietro Borrello Cc: Jiri Kosina, Hanno Zulla, Carlo Caione, Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Jiri Kosina, Roderick Colenbrander, linux-input, linux-kernel On Feb 09 2023, Pietro Borrello wrote: > Use spinlocks to deal with workers introducing a wrapper > asus_schedule_work(), and several spinlock checks. > Otherwise, asus_kbd_backlight_set() may schedule led->work after the > structure has been freed, causing a use-after-free. > > Fixes: af22a610bc38 ("HID: asus: support backlight on USB keyboards") > Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it> > --- > drivers/hid/hid-asus.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c > index f99752b998f3..30e194803bd7 100644 > --- a/drivers/hid/hid-asus.c > +++ b/drivers/hid/hid-asus.c > @@ -98,6 +98,7 @@ struct asus_kbd_leds { > struct hid_device *hdev; > struct work_struct work; > unsigned int brightness; > + spinlock_t lock; > bool removed; > }; > > @@ -490,13 +491,23 @@ static int rog_nkey_led_init(struct hid_device *hdev) > return ret; > } > > +static void asus_schedule_work(struct asus_kbd_leds *led) > +{ > + unsigned long flags; > + > + spin_lock_irqsave(&led->lock, flags); > + if (!led->removed) > + schedule_work(&led->work); > + spin_unlock_irqrestore(&led->lock, flags); > +} > + > static void asus_kbd_backlight_set(struct led_classdev *led_cdev, > enum led_brightness brightness) > { > struct asus_kbd_leds *led = container_of(led_cdev, struct asus_kbd_leds, > cdev); > led->brightness = brightness; > - schedule_work(&led->work); > + asus_schedule_work(led); > } > > static enum led_brightness asus_kbd_backlight_get(struct led_classdev *led_cdev) > @@ -512,15 +523,17 @@ static void asus_kbd_backlight_work(struct work_struct *work) > struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work); > u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 }; > int ret; > + unsigned long flags; > > - if (led->removed) > - return; > + spin_lock_irqsave(&led->lock, flags); > > buf[4] = led->brightness; > > ret = asus_kbd_set_report(led->hdev, buf, sizeof(buf)); > if (ret < 0) > hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret); > + > + spin_unlock_irqrestore(&led->lock, flags); Same as in 1/2, please only keep "buf[4] = led->brightness;" under spinlock. Which also raises the question on why the other accesses of led->brightness are not protected by the spinlock :) Note that we could use an atomic to not use the spinlock, but we need the spinlock anyway... Cheers, Benjamin > } > > /* WMI-based keyboard backlight LED control (via asus-wmi driver) takes > @@ -584,6 +597,7 @@ static int asus_kbd_register_leds(struct hid_device *hdev) > drvdata->kbd_backlight->cdev.brightness_set = asus_kbd_backlight_set; > drvdata->kbd_backlight->cdev.brightness_get = asus_kbd_backlight_get; > INIT_WORK(&drvdata->kbd_backlight->work, asus_kbd_backlight_work); > + spin_lock_init(&drvdata->kbd_backlight->lock); > > ret = devm_led_classdev_register(&hdev->dev, &drvdata->kbd_backlight->cdev); > if (ret < 0) { > @@ -1119,9 +1133,13 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id) > static void asus_remove(struct hid_device *hdev) > { > struct asus_drvdata *drvdata = hid_get_drvdata(hdev); > + unsigned long flags; > > if (drvdata->kbd_backlight) { > + spin_lock_irqsave(&drvdata->kbd_backlight->lock, flags); > drvdata->kbd_backlight->removed = true; > + spin_unlock_irqrestore(&drvdata->kbd_backlight->lock, flags); > + > cancel_work_sync(&drvdata->kbd_backlight->work); > } > > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] HID: asus: use spinlock to safely schedule workers 2023-02-09 23:58 ` [PATCH v3 2/2] HID: asus: " Pietro Borrello 2023-02-10 14:58 ` Benjamin Tissoires @ 2023-02-14 7:58 ` Dan Carpenter 2023-02-14 17:22 ` Pietro Borrello 1 sibling, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2023-02-14 7:58 UTC (permalink / raw) To: oe-kbuild, Pietro Borrello, Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione Cc: lkp, oe-kbuild-all, Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Roderick Colenbrander, linux-input, linux-kernel, Pietro Borrello Hi Pietro, url: https://github.com/intel-lab-lkp/linux/commits/Pietro-Borrello/HID-bigben-use-spinlock-to-safely-schedule-workers/20230210-080058 base: 2241ab53cbb5cdb08a6b2d4688feb13971058f65 patch link: https://lore.kernel.org/r/20230125-hid-unregister-leds-v3-2-0a52ac225e00%40diag.uniroma1.it patch subject: [PATCH v3 2/2] HID: asus: use spinlock to safely schedule workers config: riscv-randconfig-m031-20230212 (https://download.01.org/0day-ci/archive/20230214/202302141039.anZLT940-lkp@intel.com/config) compiler: riscv32-linux-gcc (GCC) 12.1.0 If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <error27@gmail.com> | Link: https://lore.kernel.org/r/202302141039.anZLT940-lkp@intel.com/ smatch warnings: drivers/hid/hid-asus.c:532 asus_kbd_backlight_work() warn: sleeping in atomic context vim +532 drivers/hid/hid-asus.c af22a610bc3850 Carlo Caione 2017-04-06 521 static void asus_kbd_backlight_work(struct work_struct *work) af22a610bc3850 Carlo Caione 2017-04-06 522 { af22a610bc3850 Carlo Caione 2017-04-06 523 struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work); af22a610bc3850 Carlo Caione 2017-04-06 524 u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 }; af22a610bc3850 Carlo Caione 2017-04-06 525 int ret; 31e578b641b3b3 Pietro Borrello 2023-02-09 526 unsigned long flags; af22a610bc3850 Carlo Caione 2017-04-06 527 31e578b641b3b3 Pietro Borrello 2023-02-09 528 spin_lock_irqsave(&led->lock, flags); ^^^^^^^^^^^^^^^^^ Holding a spinlock. af22a610bc3850 Carlo Caione 2017-04-06 529 af22a610bc3850 Carlo Caione 2017-04-06 530 buf[4] = led->brightness; af22a610bc3850 Carlo Caione 2017-04-06 531 af22a610bc3850 Carlo Caione 2017-04-06 @532 ret = asus_kbd_set_report(led->hdev, buf, sizeof(buf)); ^^^^^^^^^^^^^^^^^^^ asus_kbd_set_report() does a GFP_KERNEL allocation. af22a610bc3850 Carlo Caione 2017-04-06 533 if (ret < 0) af22a610bc3850 Carlo Caione 2017-04-06 534 hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret); 31e578b641b3b3 Pietro Borrello 2023-02-09 535 31e578b641b3b3 Pietro Borrello 2023-02-09 536 spin_unlock_irqrestore(&led->lock, flags); af22a610bc3850 Carlo Caione 2017-04-06 537 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] HID: asus: use spinlock to safely schedule workers 2023-02-14 7:58 ` Dan Carpenter @ 2023-02-14 17:22 ` Pietro Borrello 0 siblings, 0 replies; 11+ messages in thread From: Pietro Borrello @ 2023-02-14 17:22 UTC (permalink / raw) To: Dan Carpenter Cc: oe-kbuild, Jiri Kosina, Benjamin Tissoires, Hanno Zulla, Carlo Caione, lkp, oe-kbuild-all, Cristiano Giuffrida, Bos, H.J., Jakob Koschel, Roderick Colenbrander, linux-input, linux-kernel On Tue, 14 Feb 2023 at 08:58, Dan Carpenter <error27@gmail.com> wrote: > > Hi Pietro, > > url: https://github.com/intel-lab-lkp/linux/commits/Pietro-Borrello/HID-bigben-use-spinlock-to-safely-schedule-workers/20230210-080058 > base: 2241ab53cbb5cdb08a6b2d4688feb13971058f65 > patch link: https://lore.kernel.org/r/20230125-hid-unregister-leds-v3-2-0a52ac225e00%40diag.uniroma1.it > patch subject: [PATCH v3 2/2] HID: asus: use spinlock to safely schedule workers > config: riscv-randconfig-m031-20230212 (https://download.01.org/0day-ci/archive/20230214/202302141039.anZLT940-lkp@intel.com/config) > compiler: riscv32-linux-gcc (GCC) 12.1.0 > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot <lkp@intel.com> > | Reported-by: Dan Carpenter <error27@gmail.com> > | Link: https://lore.kernel.org/r/202302141039.anZLT940-lkp@intel.com/ > > smatch warnings: > drivers/hid/hid-asus.c:532 asus_kbd_backlight_work() warn: sleeping in atomic context > > vim +532 drivers/hid/hid-asus.c > > af22a610bc3850 Carlo Caione 2017-04-06 521 static void asus_kbd_backlight_work(struct work_struct *work) > af22a610bc3850 Carlo Caione 2017-04-06 522 { > af22a610bc3850 Carlo Caione 2017-04-06 523 struct asus_kbd_leds *led = container_of(work, struct asus_kbd_leds, work); > af22a610bc3850 Carlo Caione 2017-04-06 524 u8 buf[] = { FEATURE_KBD_REPORT_ID, 0xba, 0xc5, 0xc4, 0x00 }; > af22a610bc3850 Carlo Caione 2017-04-06 525 int ret; > 31e578b641b3b3 Pietro Borrello 2023-02-09 526 unsigned long flags; > af22a610bc3850 Carlo Caione 2017-04-06 527 > 31e578b641b3b3 Pietro Borrello 2023-02-09 528 spin_lock_irqsave(&led->lock, flags); > ^^^^^^^^^^^^^^^^^ > Holding a spinlock. > > af22a610bc3850 Carlo Caione 2017-04-06 529 > af22a610bc3850 Carlo Caione 2017-04-06 530 buf[4] = led->brightness; > af22a610bc3850 Carlo Caione 2017-04-06 531 > af22a610bc3850 Carlo Caione 2017-04-06 @532 ret = asus_kbd_set_report(led->hdev, buf, sizeof(buf)); > ^^^^^^^^^^^^^^^^^^^ > asus_kbd_set_report() does a GFP_KERNEL allocation. > > > af22a610bc3850 Carlo Caione 2017-04-06 533 if (ret < 0) > af22a610bc3850 Carlo Caione 2017-04-06 534 hid_err(led->hdev, "Asus failed to set keyboard backlight: %d\n", ret); > 31e578b641b3b3 Pietro Borrello 2023-02-09 535 > 31e578b641b3b3 Pietro Borrello 2023-02-09 536 spin_unlock_irqrestore(&led->lock, flags); > af22a610bc3850 Carlo Caione 2017-04-06 537 } > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests > Hello, Thank you, but the issue has already been fixed in v4. Link: https://lore.kernel.org/lkml/20230125-hid-unregister-leds-v4-3-7860c5763c38@diag.uniroma1.it/T/#m8983ca3d0fbf656a40011a77c8988e3d16cac671 Best regards, Pietro ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-02-14 17:23 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-09 23:58 [PATCH v3 0/2] HID: use spinlocks to safely schedule led workers Pietro Borrello 2023-02-09 23:58 ` [PATCH v3 1/2] HID: bigben: use spinlock to safely schedule workers Pietro Borrello [not found] ` <20230210132017.2497-1-hdanton@sina.com> 2023-02-10 14:11 ` Benjamin Tissoires [not found] ` <20230211020041.2613-1-hdanton@sina.com> 2023-02-13 8:25 ` Benjamin Tissoires [not found] ` <20230213103658.3091-1-hdanton@sina.com> 2023-02-13 10:47 ` Benjamin Tissoires 2023-02-10 14:26 ` Benjamin Tissoires 2023-02-11 22:27 ` Pietro Borrello 2023-02-09 23:58 ` [PATCH v3 2/2] HID: asus: " Pietro Borrello 2023-02-10 14:58 ` Benjamin Tissoires 2023-02-14 7:58 ` Dan Carpenter 2023-02-14 17:22 ` Pietro Borrello
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).