* [PATCH] tty/vt: Fix possible deadlock in input_inject_event
@ 2025-09-28 13:08 pengyu
2025-09-29 4:54 ` Dmitry Torokhov
2025-09-30 6:31 ` kernel test robot
0 siblings, 2 replies; 10+ messages in thread
From: pengyu @ 2025-09-28 13:08 UTC (permalink / raw)
To: gregkh, jirislaby
Cc: legion, mingo, myrrhperiwinkle, tglx, dmitry.torokhov,
changlianzhi, linux-kernel, linux-serial, pengyu,
syzbot+79c403850e6816dc39cf
syzkaller testing revealed a potential deadlock involving keyboard
handling:
CPU0 CPU1 CPU2
---- ---- ----
read_lock(tasklist_lock); evdev_write
input_inject_event write_lock(tasklist_lock);
lock(&dev->event_lock);
read_lock(tasklist_lock);
<Interrupt>
kbd_bh() / kd_sound_helper()
input_inject_event
lock(&dev->event_lock); // Deadlock risk
The deadlock occurs because:
1. Both kbd_bh and kd_sound_helper run in interrupt context
2. tasklist_lock is interrupt-unsafe
3. When evdev_write holds both dev->event_lock and tasklist_lock,
interrupt context attempts to acquire dev->event_lock create deadlock
risks
Convert both kbd_bh and kd_sound_helper to use workqueues. This moves
input_inject_event execution to process context, where it's safe to
acquire locks that may be held by code using interrupt-unsafe locks.
Reported-by: syzbot+79c403850e6816dc39cf@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/66f6c8ce.050a0220.46d20.001c.GAE@google.com/T/#u
Fixes: fb09d0ac0772 ("tty: Fix the keyboard led light display problem")
Signed-off-by: pengyu <pengyu@kylinos.cn>
---
drivers/tty/vt/keyboard.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index ee1d9c448c7e..eb2afc86b502 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -131,8 +131,8 @@ static const unsigned char max_vals[] = {
static const int NR_TYPES = ARRAY_SIZE(max_vals);
-static void kbd_bh(struct tasklet_struct *unused);
-static DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh);
+static void kbd_bh(struct work_struct *unused);
+static DECLARE_WORK(keyboard_work, kbd_bh);
static struct input_handler kbd_handler;
static DEFINE_SPINLOCK(kbd_event_lock);
@@ -264,23 +264,23 @@ static int kd_sound_helper(struct input_handle *handle, void *data)
return 0;
}
-static void kd_nosound(struct timer_list *unused)
+static void kd_nosound(struct work_struct *unused)
{
static unsigned int zero;
input_handler_for_each_handle(&kbd_handler, &zero, kd_sound_helper);
}
-static DEFINE_TIMER(kd_mksound_timer, kd_nosound);
+static DECLARE_DELAYED_WORK(kd_mksound_worker, kd_nosound);
void kd_mksound(unsigned int hz, unsigned int ticks)
{
- timer_delete_sync(&kd_mksound_timer);
+ cancel_delayed_work_sync(&kd_mksound_worker);
input_handler_for_each_handle(&kbd_handler, &hz, kd_sound_helper);
if (hz && ticks)
- mod_timer(&kd_mksound_timer, jiffies + ticks);
+ schedule_delayed_work(&kd_mksound_worker, ticks);
}
EXPORT_SYMBOL(kd_mksound);
@@ -390,7 +390,7 @@ static void put_queue_utf8(struct vc_data *vc, u32 value)
/* FIXME: review locking for vt.c callers */
static void set_leds(void)
{
- tasklet_schedule(&keyboard_tasklet);
+ schedule_work(&keyboard_work);
}
/*
@@ -1024,10 +1024,10 @@ static int kbd_led_trigger_activate(struct led_classdev *cdev)
struct kbd_led_trigger *trigger =
container_of(cdev->trigger, struct kbd_led_trigger, trigger);
- tasklet_disable(&keyboard_tasklet);
+ cancel_work_sync(&keyboard_work);
if (ledstate != -1U)
led_set_brightness(cdev, ledstate & trigger->mask ? LED_FULL : LED_OFF);
- tasklet_enable(&keyboard_tasklet);
+ enable_work(&keyboard_work);
return 0;
}
@@ -1243,7 +1243,7 @@ void vt_kbd_con_stop(unsigned int console)
* handle the scenario when keyboard handler is not registered yet
* but we already getting updates from the VT to update led state.
*/
-static void kbd_bh(struct tasklet_struct *unused)
+static void kbd_bh(struct work_struct *unused)
{
unsigned int leds;
unsigned long flags;
@@ -1535,7 +1535,7 @@ static void kbd_event(struct input_handle *handle, unsigned int event_type,
spin_unlock(&kbd_event_lock);
- tasklet_schedule(&keyboard_tasklet);
+ schedule_work(&keyboard_work);
do_poke_blanked_console = 1;
schedule_console_callback();
}
@@ -1607,12 +1607,12 @@ static void kbd_disconnect(struct input_handle *handle)
*/
static void kbd_start(struct input_handle *handle)
{
- tasklet_disable(&keyboard_tasklet);
+ cancel_work_sync(&keyboard_work);
if (ledstate != -1U)
kbd_update_leds_helper(handle, &ledstate);
- tasklet_enable(&keyboard_tasklet);
+ enable_work(&keyboard_work);
}
static const struct input_device_id kbd_ids[] = {
@@ -1662,8 +1662,8 @@ int __init kbd_init(void)
if (error)
return error;
- tasklet_enable(&keyboard_tasklet);
- tasklet_schedule(&keyboard_tasklet);
+ enable_work(&keyboard_work);
+ schedule_work(&keyboard_work);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] tty/vt: Fix possible deadlock in input_inject_event
2025-09-28 13:08 [PATCH] tty/vt: Fix possible deadlock in input_inject_event pengyu
@ 2025-09-29 4:54 ` Dmitry Torokhov
2025-10-01 10:42 ` pengyu
2025-09-30 6:31 ` kernel test robot
1 sibling, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2025-09-29 4:54 UTC (permalink / raw)
To: pengyu
Cc: gregkh, jirislaby, legion, mingo, myrrhperiwinkle, tglx,
changlianzhi, linux-kernel, linux-serial,
syzbot+79c403850e6816dc39cf
Hi,
On Sun, Sep 28, 2025 at 09:08:19PM +0800, pengyu wrote:
> syzkaller testing revealed a potential deadlock involving keyboard
> handling:
>
> CPU0 CPU1 CPU2
> ---- ---- ----
> read_lock(tasklist_lock); evdev_write
> input_inject_event write_lock(tasklist_lock);
> lock(&dev->event_lock);
> read_lock(tasklist_lock);
> <Interrupt>
> kbd_bh() / kd_sound_helper()
> input_inject_event
> lock(&dev->event_lock); // Deadlock risk
>
> The deadlock occurs because:
> 1. Both kbd_bh and kd_sound_helper run in interrupt context
> 2. tasklist_lock is interrupt-unsafe
> 3. When evdev_write holds both dev->event_lock and tasklist_lock,
> interrupt context attempts to acquire dev->event_lock create deadlock
> risks
>
> Convert both kbd_bh and kd_sound_helper to use workqueues. This moves
> input_inject_event execution to process context, where it's safe to
> acquire locks that may be held by code using interrupt-unsafe locks.
So if we ignore the input code and instead look at the send_sigio()
(which input core ends up calling) and do_wait() we see that
send_sigio() disables interrupts and takes the owner's spinlock
before taking the tasklist_lock, while do_wait() takes the tasklist_lock
first, without disabling interrupts. This is root of the issue as far as
I can tell and no amount of changes to the keyboard handler (which is
just happens to be in the middle) will not solve for all potential cases
and code paths.
I believe either do_exit() or send_sigio() have to be changed to fix
this properly.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] tty/vt: Fix possible deadlock in input_inject_event
2025-09-28 13:08 [PATCH] tty/vt: Fix possible deadlock in input_inject_event pengyu
2025-09-29 4:54 ` Dmitry Torokhov
@ 2025-09-30 6:31 ` kernel test robot
2025-10-01 10:23 ` [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default pengyu
1 sibling, 1 reply; 10+ messages in thread
From: kernel test robot @ 2025-09-30 6:31 UTC (permalink / raw)
To: pengyu
Cc: oe-lkp, lkp, linux-kernel, linux-serial, gregkh, jirislaby,
legion, mingo, myrrhperiwinkle, tglx, dmitry.torokhov,
changlianzhi, pengyu, syzbot+79c403850e6816dc39cf, oliver.sang
Hello,
kernel test robot noticed "WARNING:at_kernel/workqueue.c:#enable_work" on:
commit: 81d246ffd402b46338d8a3e0cd23f474195dd236 ("[PATCH] tty/vt: Fix possible deadlock in input_inject_event")
url: https://github.com/intel-lab-lkp/linux/commits/pengyu/tty-vt-Fix-possible-deadlock-in-input_inject_event/20250928-211353
base: https://git.kernel.org/cgit/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/all/20250928130819.383808-1-pengyu@kylinos.cn/
patch subject: [PATCH] tty/vt: Fix possible deadlock in input_inject_event
in testcase: boot
config: x86_64-kexec
compiler: clang-20
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
+--------------------------------------------+------------+------------+
| | f4abab3508 | 81d246ffd4 |
+--------------------------------------------+------------+------------+
| WARNING:at_kernel/workqueue.c:#enable_work | 0 | 12 |
| RIP:enable_work | 0 | 12 |
+--------------------------------------------+------------+------------+
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202509301323.34d956e1-lkp@intel.com
[ 0.877354][ T1] ------------[ cut here ]------------
[ 0.878411][ T1] workqueue: work disable count underflowed
[ 0.879704][ T1] WARNING: CPU: 0 PID: 1 at kernel/workqueue.c:4326 enable_work (kernel/workqueue.c:4326)
[ 0.881213][ T1] Modules linked in:
[ 0.881232][ T1] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.17.0-rc6-00045-g81d246ffd402 #1 PREEMPT(voluntary)
[ 0.881232][ T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 0.881232][ T1] RIP: 0010:enable_work (kernel/workqueue.c:4326)
[ 0.881232][ T1] Code: 5e 5d e9 0c c1 ec 00 cc 0f 0b eb 9b 31 ed 80 3d 1e e8 c6 01 00 75 a0 c6 05 15 e8 c6 01 01 48 c7 c7 cd bf 6d 82 e8 89 26 f3 ff <0f> 0b eb 89 0f 0b eb 93 e8 cc c7 eb 00 66 66 66 2e 0f 1f 84 00 00
All code
========
0: 5e pop %rsi
1: 5d pop %rbp
2: e9 0c c1 ec 00 jmp 0xecc113
7: cc int3
8: 0f 0b ud2
a: eb 9b jmp 0xffffffffffffffa7
c: 31 ed xor %ebp,%ebp
e: 80 3d 1e e8 c6 01 00 cmpb $0x0,0x1c6e81e(%rip) # 0x1c6e833
15: 75 a0 jne 0xffffffffffffffb7
17: c6 05 15 e8 c6 01 01 movb $0x1,0x1c6e815(%rip) # 0x1c6e833
1e: 48 c7 c7 cd bf 6d 82 mov $0xffffffff826dbfcd,%rdi
25: e8 89 26 f3 ff call 0xfffffffffff326b3
2a:* 0f 0b ud2 <-- trapping instruction
2c: eb 89 jmp 0xffffffffffffffb7
2e: 0f 0b ud2
30: eb 93 jmp 0xffffffffffffffc5
32: e8 cc c7 eb 00 call 0xebc803
37: 66 data16
38: 66 data16
39: 66 data16
3a: 2e cs
3b: 0f .byte 0xf
3c: 1f (bad)
3d: 84 00 test %al,(%rax)
...
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: eb 89 jmp 0xffffffffffffff8d
4: 0f 0b ud2
6: eb 93 jmp 0xffffffffffffff9b
8: e8 cc c7 eb 00 call 0xebc7d9
d: 66 data16
e: 66 data16
f: 66 data16
10: 2e cs
11: 0f .byte 0xf
12: 1f (bad)
13: 84 00 test %al,(%rax)
...
[ 0.881232][ T1] RSP: 0000:ffffc90000013b28 EFLAGS: 00010046
[ 0.881232][ T1] RAX: 63593ad70bfd2600 RBX: ffffffff82ead988 RCX: 63593ad70bfd2601
[ 0.881232][ T1] RDX: ffffc90000013918 RSI: 00000000ffff7fff RDI: 0000000000000001
[ 0.881232][ T1] RBP: 0000000000000000 R08: 0000000000000000 R09: ffffc90000013910
[ 0.881232][ T1] R10: 00000000ffff7fff R11: 0000000000000001 R12: 0000000000000000
[ 0.881232][ T1] R13: 0000000000000000 R14: 000fffffffe00001 R15: 0000000000000000
[ 0.881232][ T1] FS: 0000000000000000(0000) GS:ffff8884ac411000(0000) knlGS:0000000000000000
[ 0.881232][ T1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 0.881232][ T1] CR2: ffff88843ffff000 CR3: 0000000002a30000 CR4: 00000000000406f0
[ 0.881232][ T1] Call Trace:
[ 0.881232][ T1] <TASK>
[ 0.881232][ T1] kbd_init (include/linux/workqueue.h:730 drivers/tty/vt/keyboard.c:1666)
[ 0.881232][ T1] vty_init (drivers/tty/vt/vt.c:3884)
[ 0.881232][ T1] tty_init (drivers/tty/tty_io.c:3651)
[ 0.881232][ T1] ? __pfx_chr_dev_init (drivers/char/mem.c:734)
[ 0.881232][ T1] do_one_initcall (init/main.c:1269)
[ 0.881232][ T1] ? __alloc_frozen_pages_noprof (mm/page_alloc.c:5148)
[ 0.881232][ T1] ? alloc_pages_mpol (mm/mempolicy.c:2416)
[ 0.881232][ T1] ? new_slab (mm/slub.c:560 mm/slub.c:2698 mm/slub.c:2714)
[ 0.881232][ T1] ? __proc_create (fs/proc/generic.c:453)
[ 0.881232][ T1] ? ida_alloc_range (include/linux/xarray.h:1437 lib/idr.c:461)
[ 0.881232][ T1] ? parameq (kernel/params.c:90 kernel/params.c:99)
[ 0.881232][ T1] ? __pfx_ignore_unknown_bootoption (init/main.c:1315)
[ 0.881232][ T1] ? parse_args (kernel/params.c:153 kernel/params.c:186)
[ 0.881232][ T1] do_initcall_level (init/main.c:1330)
[ 0.881232][ T1] do_initcalls (init/main.c:1344)
[ 0.881232][ T1] kernel_init_freeable (init/main.c:1583)
[ 0.881232][ T1] ? __pfx_kernel_init (init/main.c:1461)
[ 0.881232][ T1] kernel_init (init/main.c:1471)
[ 0.881232][ T1] ret_from_fork (arch/x86/kernel/process.c:154)
[ 0.881232][ T1] ? __pfx_kernel_init (init/main.c:1461)
[ 0.881232][ T1] ret_from_fork_asm (arch/x86/entry/entry_64.S:258)
[ 0.881232][ T1] </TASK>
[ 0.881232][ T1] ---[ end trace 0000000000000000 ]---
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250930/202509301323.34d956e1-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default
2025-09-30 6:31 ` kernel test robot
@ 2025-10-01 10:23 ` pengyu
2025-10-01 10:23 ` [PATCH v2 2/2] tty/vt: Fix possible deadlock in input_inject_event pengyu
2025-10-07 19:18 ` [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default Tejun Heo
0 siblings, 2 replies; 10+ messages in thread
From: pengyu @ 2025-10-01 10:23 UTC (permalink / raw)
To: tj, jiangshanlai, oliver.sang
Cc: changlianzhi, dmitry.torokhov, gregkh, jirislaby, legion,
linux-kernel, linux-serial, lkp, mingo, myrrhperiwinkle, oe-lkp,
pengyu, syzbot+79c403850e6816dc39cf, tglx
In certain scenarios, workqueue tasks that are disabled by default are
required. Similar to DECLARE_TASKLET_DISABLED, the DECLARE_WORK_DISABLED
macro is added to achieve this functionality.
Signed-off-by: pengyu <pengyu@kylinos.cn>
---
include/linux/workqueue.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 45d5dd470ff6..b6c72d59351b 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -102,6 +102,7 @@ enum wq_misc_consts {
/* Convenience constants - of type 'unsigned long', not 'enum'! */
#define WORK_OFFQ_BH (1ul << WORK_OFFQ_BH_BIT)
#define WORK_OFFQ_FLAG_MASK (((1ul << WORK_OFFQ_FLAG_BITS) - 1) << WORK_OFFQ_FLAG_SHIFT)
+#define WORK_OFFQ_DISABLED (1ul << WORK_OFFQ_DISABLE_SHIFT)
#define WORK_OFFQ_DISABLE_MASK (((1ul << WORK_OFFQ_DISABLE_BITS) - 1) << WORK_OFFQ_DISABLE_SHIFT)
#define WORK_OFFQ_POOL_NONE ((1ul << WORK_OFFQ_POOL_BITS) - 1)
#define WORK_STRUCT_NO_POOL (WORK_OFFQ_POOL_NONE << WORK_OFFQ_POOL_SHIFT)
@@ -110,6 +111,8 @@ enum wq_misc_consts {
#define WORK_DATA_INIT() ATOMIC_LONG_INIT((unsigned long)WORK_STRUCT_NO_POOL)
#define WORK_DATA_STATIC_INIT() \
ATOMIC_LONG_INIT((unsigned long)(WORK_STRUCT_NO_POOL | WORK_STRUCT_STATIC))
+#define WORK_DATA_DISABLED_INIT() \
+ ATOMIC_LONG_INIT((unsigned long)(WORK_STRUCT_NO_POOL | WORK_STRUCT_STATIC | WORK_OFFQ_DISABLED))
struct delayed_work {
struct work_struct work;
@@ -242,6 +245,13 @@ struct execute_work {
__WORK_INIT_LOCKDEP_MAP(#n, &(n)) \
}
+#define __WORK_DISABLED_INITIALIZER(n, f) { \
+ .data = WORK_DATA_DISABLED_INIT(), \
+ .entry = { &(n).entry, &(n).entry }, \
+ .func = (f), \
+ __WORK_INIT_LOCKDEP_MAP(#n, &(n)) \
+ }
+
#define __DELAYED_WORK_INITIALIZER(n, f, tflags) { \
.work = __WORK_INITIALIZER((n).work, (f)), \
.timer = __TIMER_INITIALIZER(delayed_work_timer_fn,\
@@ -251,6 +261,9 @@ struct execute_work {
#define DECLARE_WORK(n, f) \
struct work_struct n = __WORK_INITIALIZER(n, f)
+#define DECLARE_WORK_DISABLED(n, f) \
+ struct work_struct n = __WORK_DISABLED_INITIALIZER(n, f)
+
#define DECLARE_DELAYED_WORK(n, f) \
struct delayed_work n = __DELAYED_WORK_INITIALIZER(n, f, 0)
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] tty/vt: Fix possible deadlock in input_inject_event
2025-10-01 10:23 ` [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default pengyu
@ 2025-10-01 10:23 ` pengyu
2025-10-07 19:18 ` [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default Tejun Heo
1 sibling, 0 replies; 10+ messages in thread
From: pengyu @ 2025-10-01 10:23 UTC (permalink / raw)
To: tj, jiangshanlai, oliver.sang
Cc: changlianzhi, dmitry.torokhov, gregkh, jirislaby, legion,
linux-kernel, linux-serial, lkp, mingo, myrrhperiwinkle, oe-lkp,
pengyu, syzbot+79c403850e6816dc39cf, tglx
syzkaller testing revealed a potential deadlock involving keyboard
handling:
CPU0 CPU1 CPU2
---- ---- ----
read_lock(tasklist_lock); evdev_write
input_inject_event write_lock(tasklist_lock);
lock(&dev->event_lock);
read_lock(tasklist_lock);
<Interrupt>
kbd_bh() / kd_sound_helper()
input_inject_event
lock(&dev->event_lock); // Deadlock risk
The deadlock occurs because:
1. Both kbd_bh and kd_sound_helper run in interrupt context
2. tasklist_lock is interrupt-unsafe
3. When evdev_write holds both dev->event_lock and tasklist_lock,
interrupt context attempts to acquire dev->event_lock create deadlock
risks
Convert both kbd_bh and kd_sound_helper to use workqueues. This moves
input_inject_event execution to process context, where it's safe to
acquire locks that may be held by code using interrupt-unsafe locks.
Reported-by: syzbot+79c403850e6816dc39cf@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/66f6c8ce.050a0220.46d20.001c.GAE@google.com/T/#u
Fixes: fb09d0ac0772 ("tty: Fix the keyboard led light display problem")
Signed-off-by: pengyu <pengyu@kylinos.cn>
---
Changes in v2:
- enable_work needs to be used in pairs with disable_work_sync,
not with cancel_work_sync.
- use work items that diabled by default.
---
drivers/tty/vt/keyboard.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index ee1d9c448c7e..d3d9c2fda467 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -131,8 +131,8 @@ static const unsigned char max_vals[] = {
static const int NR_TYPES = ARRAY_SIZE(max_vals);
-static void kbd_bh(struct tasklet_struct *unused);
-static DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh);
+static void kbd_bh(struct work_struct *unused);
+static DECLARE_WORK_DISABLED(keyboard_work, kbd_bh);
static struct input_handler kbd_handler;
static DEFINE_SPINLOCK(kbd_event_lock);
@@ -264,23 +264,23 @@ static int kd_sound_helper(struct input_handle *handle, void *data)
return 0;
}
-static void kd_nosound(struct timer_list *unused)
+static void kd_nosound(struct work_struct *unused)
{
static unsigned int zero;
input_handler_for_each_handle(&kbd_handler, &zero, kd_sound_helper);
}
-static DEFINE_TIMER(kd_mksound_timer, kd_nosound);
+static DECLARE_DELAYED_WORK(kd_mksound_worker, kd_nosound);
void kd_mksound(unsigned int hz, unsigned int ticks)
{
- timer_delete_sync(&kd_mksound_timer);
+ cancel_delayed_work_sync(&kd_mksound_worker);
input_handler_for_each_handle(&kbd_handler, &hz, kd_sound_helper);
if (hz && ticks)
- mod_timer(&kd_mksound_timer, jiffies + ticks);
+ schedule_delayed_work(&kd_mksound_worker, ticks);
}
EXPORT_SYMBOL(kd_mksound);
@@ -390,7 +390,7 @@ static void put_queue_utf8(struct vc_data *vc, u32 value)
/* FIXME: review locking for vt.c callers */
static void set_leds(void)
{
- tasklet_schedule(&keyboard_tasklet);
+ schedule_work(&keyboard_work);
}
/*
@@ -1024,10 +1024,10 @@ static int kbd_led_trigger_activate(struct led_classdev *cdev)
struct kbd_led_trigger *trigger =
container_of(cdev->trigger, struct kbd_led_trigger, trigger);
- tasklet_disable(&keyboard_tasklet);
+ disable_work_sync(&keyboard_work);
if (ledstate != -1U)
led_set_brightness(cdev, ledstate & trigger->mask ? LED_FULL : LED_OFF);
- tasklet_enable(&keyboard_tasklet);
+ enable_work(&keyboard_work);
return 0;
}
@@ -1243,7 +1243,7 @@ void vt_kbd_con_stop(unsigned int console)
* handle the scenario when keyboard handler is not registered yet
* but we already getting updates from the VT to update led state.
*/
-static void kbd_bh(struct tasklet_struct *unused)
+static void kbd_bh(struct work_struct *unused)
{
unsigned int leds;
unsigned long flags;
@@ -1535,7 +1535,7 @@ static void kbd_event(struct input_handle *handle, unsigned int event_type,
spin_unlock(&kbd_event_lock);
- tasklet_schedule(&keyboard_tasklet);
+ schedule_work(&keyboard_work);
do_poke_blanked_console = 1;
schedule_console_callback();
}
@@ -1607,12 +1607,12 @@ static void kbd_disconnect(struct input_handle *handle)
*/
static void kbd_start(struct input_handle *handle)
{
- tasklet_disable(&keyboard_tasklet);
+ disable_work_sync(&keyboard_work);
if (ledstate != -1U)
kbd_update_leds_helper(handle, &ledstate);
- tasklet_enable(&keyboard_tasklet);
+ enable_work(&keyboard_work);
}
static const struct input_device_id kbd_ids[] = {
@@ -1662,8 +1662,8 @@ int __init kbd_init(void)
if (error)
return error;
- tasklet_enable(&keyboard_tasklet);
- tasklet_schedule(&keyboard_tasklet);
+ enable_work(&keyboard_work);
+ schedule_work(&keyboard_work);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] tty/vt: Fix possible deadlock in input_inject_event
2025-09-29 4:54 ` Dmitry Torokhov
@ 2025-10-01 10:42 ` pengyu
0 siblings, 0 replies; 10+ messages in thread
From: pengyu @ 2025-10-01 10:42 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: gregkh, jirislaby, legion, mingo, myrrhperiwinkle, tglx,
changlianzhi, linux-kernel, linux-serial,
syzbot+79c403850e6816dc39cf, jiangshanlai, tj
在 2025/9/29 12:54, Dmitry Torokhov 写道:
> Hi,
>
> On Sun, Sep 28, 2025 at 09:08:19PM +0800, pengyu wrote:
>> syzkaller testing revealed a potential deadlock involving keyboard
>> handling:
>>
>> CPU0 CPU1 CPU2
>> ---- ---- ----
>> read_lock(tasklist_lock); evdev_write
>> input_inject_event write_lock(tasklist_lock);
>> lock(&dev->event_lock);
>> read_lock(tasklist_lock);
>> <Interrupt>
>> kbd_bh() / kd_sound_helper()
>> input_inject_event
>> lock(&dev->event_lock); // Deadlock risk
>>
>> The deadlock occurs because:
>> 1. Both kbd_bh and kd_sound_helper run in interrupt context
>> 2. tasklist_lock is interrupt-unsafe
>> 3. When evdev_write holds both dev->event_lock and tasklist_lock,
>> interrupt context attempts to acquire dev->event_lock create deadlock
>> risks
>>
>> Convert both kbd_bh and kd_sound_helper to use workqueues. This moves
>> input_inject_event execution to process context, where it's safe to
>> acquire locks that may be held by code using interrupt-unsafe locks.
>
> So if we ignore the input code and instead look at the send_sigio()
> (which input core ends up calling) and do_wait() we see that
> send_sigio() disables interrupts and takes the owner's spinlock
> before taking the tasklist_lock, while do_wait() takes the tasklist_lock
> first, without disabling interrupts. This is root of the issue as far as
> I can tell and no amount of changes to the keyboard handler (which is
> just happens to be in the middle) will not solve for all potential cases
> and code paths.
>
> I believe either do_exit() or send_sigio() have to be changed to fix
> this properly.
>
> Thanks.
>
Hi,
I noticed that besides do_wait, there are many places in the kernel
where read_lock(tasklist_lock) is used without disabling interrupts.
Addressing this solely through tasklist_lock may not fully resolve the
issue.
This involves tasklist_lock, evdev_write, and various input device
drivers. The only approach I can think of is to move functions like
input_[inject]_event in the input drivers out of the interrupt context.
This could affect many code paths, so I plan to start by modifying the
keyboard code first.
--
Thanks,
Yu Peng
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default
2025-10-01 10:23 ` [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default pengyu
2025-10-01 10:23 ` [PATCH v2 2/2] tty/vt: Fix possible deadlock in input_inject_event pengyu
@ 2025-10-07 19:18 ` Tejun Heo
2025-10-17 7:56 ` [PATCH v3 " Yu Peng
1 sibling, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2025-10-07 19:18 UTC (permalink / raw)
To: pengyu
Cc: jiangshanlai, oliver.sang, changlianzhi, dmitry.torokhov, gregkh,
jirislaby, legion, linux-kernel, linux-serial, lkp, mingo,
myrrhperiwinkle, oe-lkp, syzbot+79c403850e6816dc39cf, tglx
On Wed, Oct 01, 2025 at 06:23:40PM +0800, pengyu wrote:
> In certain scenarios, workqueue tasks that are disabled by default are
> required. Similar to DECLARE_TASKLET_DISABLED, the DECLARE_WORK_DISABLED
> macro is added to achieve this functionality.
>
> Signed-off-by: pengyu <pengyu@kylinos.cn>
Can you please use FIRST_NAME LAST_NAME <EMAIL> for the From and SOB tags?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/2] workqueue: Add initialization macro for work items that disabled by default
2025-10-07 19:18 ` [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default Tejun Heo
@ 2025-10-17 7:56 ` Yu Peng
2025-10-17 7:56 ` [PATCH v3 2/2] tty/vt: Fix possible deadlock in input_inject_event Yu Peng
2025-10-17 15:35 ` [PATCH v3 1/2] workqueue: Add initialization macro for work items that disabled by default Tejun Heo
0 siblings, 2 replies; 10+ messages in thread
From: Yu Peng @ 2025-10-17 7:56 UTC (permalink / raw)
To: tj
Cc: changlianzhi, dmitry.torokhov, gregkh, jiangshanlai, jirislaby,
legion, linux-kernel, linux-serial, lkp, mingo, myrrhperiwinkle,
oe-lkp, oliver.sang, pengyu, syzbot+79c403850e6816dc39cf, tglx
In certain scenarios, workqueue tasks that are disabled by default are
required. Similar to DECLARE_TASKLET_DISABLED, the DECLARE_WORK_DISABLED
macro is added to achieve this functionality.
Signed-off-by: Yu Peng <pengyu@kylinos.cn>
---
include/linux/workqueue.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 45d5dd470ff6..b6c72d59351b 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -102,6 +102,7 @@ enum wq_misc_consts {
/* Convenience constants - of type 'unsigned long', not 'enum'! */
#define WORK_OFFQ_BH (1ul << WORK_OFFQ_BH_BIT)
#define WORK_OFFQ_FLAG_MASK (((1ul << WORK_OFFQ_FLAG_BITS) - 1) << WORK_OFFQ_FLAG_SHIFT)
+#define WORK_OFFQ_DISABLED (1ul << WORK_OFFQ_DISABLE_SHIFT)
#define WORK_OFFQ_DISABLE_MASK (((1ul << WORK_OFFQ_DISABLE_BITS) - 1) << WORK_OFFQ_DISABLE_SHIFT)
#define WORK_OFFQ_POOL_NONE ((1ul << WORK_OFFQ_POOL_BITS) - 1)
#define WORK_STRUCT_NO_POOL (WORK_OFFQ_POOL_NONE << WORK_OFFQ_POOL_SHIFT)
@@ -110,6 +111,8 @@ enum wq_misc_consts {
#define WORK_DATA_INIT() ATOMIC_LONG_INIT((unsigned long)WORK_STRUCT_NO_POOL)
#define WORK_DATA_STATIC_INIT() \
ATOMIC_LONG_INIT((unsigned long)(WORK_STRUCT_NO_POOL | WORK_STRUCT_STATIC))
+#define WORK_DATA_DISABLED_INIT() \
+ ATOMIC_LONG_INIT((unsigned long)(WORK_STRUCT_NO_POOL | WORK_STRUCT_STATIC | WORK_OFFQ_DISABLED))
struct delayed_work {
struct work_struct work;
@@ -242,6 +245,13 @@ struct execute_work {
__WORK_INIT_LOCKDEP_MAP(#n, &(n)) \
}
+#define __WORK_DISABLED_INITIALIZER(n, f) { \
+ .data = WORK_DATA_DISABLED_INIT(), \
+ .entry = { &(n).entry, &(n).entry }, \
+ .func = (f), \
+ __WORK_INIT_LOCKDEP_MAP(#n, &(n)) \
+ }
+
#define __DELAYED_WORK_INITIALIZER(n, f, tflags) { \
.work = __WORK_INITIALIZER((n).work, (f)), \
.timer = __TIMER_INITIALIZER(delayed_work_timer_fn,\
@@ -251,6 +261,9 @@ struct execute_work {
#define DECLARE_WORK(n, f) \
struct work_struct n = __WORK_INITIALIZER(n, f)
+#define DECLARE_WORK_DISABLED(n, f) \
+ struct work_struct n = __WORK_DISABLED_INITIALIZER(n, f)
+
#define DECLARE_DELAYED_WORK(n, f) \
struct delayed_work n = __DELAYED_WORK_INITIALIZER(n, f, 0)
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] tty/vt: Fix possible deadlock in input_inject_event
2025-10-17 7:56 ` [PATCH v3 " Yu Peng
@ 2025-10-17 7:56 ` Yu Peng
2025-10-17 15:35 ` [PATCH v3 1/2] workqueue: Add initialization macro for work items that disabled by default Tejun Heo
1 sibling, 0 replies; 10+ messages in thread
From: Yu Peng @ 2025-10-17 7:56 UTC (permalink / raw)
To: tj
Cc: changlianzhi, dmitry.torokhov, gregkh, jiangshanlai, jirislaby,
legion, linux-kernel, linux-serial, lkp, mingo, myrrhperiwinkle,
oe-lkp, oliver.sang, pengyu, syzbot+79c403850e6816dc39cf, tglx
syzkaller testing revealed a potential deadlock involving keyboard
handling:
CPU0 CPU1 CPU2
---- ---- ----
read_lock(tasklist_lock); evdev_write
input_inject_event write_lock(tasklist_lock);
lock(&dev->event_lock);
read_lock(tasklist_lock);
<Interrupt>
kbd_bh() / kd_sound_helper()
input_inject_event
lock(&dev->event_lock); // Deadlock risk
The deadlock occurs because:
1. Both kbd_bh and kd_sound_helper run in interrupt context
2. tasklist_lock is interrupt-unsafe
3. When evdev_write holds both dev->event_lock and tasklist_lock,
interrupt context attempts to acquire dev->event_lock create deadlock
risks
Convert both kbd_bh and kd_sound_helper to use workqueues. This moves
input_inject_event execution to process context, where it's safe to
acquire locks that may be held by code using interrupt-unsafe locks.
Reported-by: syzbot+79c403850e6816dc39cf@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/66f6c8ce.050a0220.46d20.001c.GAE@google.com/T/#u
Fixes: fb09d0ac0772 ("tty: Fix the keyboard led light display problem")
Signed-off-by: Yu Peng <pengyu@kylinos.cn>
---
Hi, tejun
Thank you for pointing that out. I'll make sure to use the proper format in
future submissions.
---
Changes in v3:
- use the correct signature format.
Changes in v2:
- enable_work needs to be used in pairs with disable_work_sync,
not with cancel_work_sync.
---
drivers/tty/vt/keyboard.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index ee1d9c448c7e..d3d9c2fda467 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -131,8 +131,8 @@ static const unsigned char max_vals[] = {
static const int NR_TYPES = ARRAY_SIZE(max_vals);
-static void kbd_bh(struct tasklet_struct *unused);
-static DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh);
+static void kbd_bh(struct work_struct *unused);
+static DECLARE_WORK_DISABLED(keyboard_work, kbd_bh);
static struct input_handler kbd_handler;
static DEFINE_SPINLOCK(kbd_event_lock);
@@ -264,23 +264,23 @@ static int kd_sound_helper(struct input_handle *handle, void *data)
return 0;
}
-static void kd_nosound(struct timer_list *unused)
+static void kd_nosound(struct work_struct *unused)
{
static unsigned int zero;
input_handler_for_each_handle(&kbd_handler, &zero, kd_sound_helper);
}
-static DEFINE_TIMER(kd_mksound_timer, kd_nosound);
+static DECLARE_DELAYED_WORK(kd_mksound_worker, kd_nosound);
void kd_mksound(unsigned int hz, unsigned int ticks)
{
- timer_delete_sync(&kd_mksound_timer);
+ cancel_delayed_work_sync(&kd_mksound_worker);
input_handler_for_each_handle(&kbd_handler, &hz, kd_sound_helper);
if (hz && ticks)
- mod_timer(&kd_mksound_timer, jiffies + ticks);
+ schedule_delayed_work(&kd_mksound_worker, ticks);
}
EXPORT_SYMBOL(kd_mksound);
@@ -390,7 +390,7 @@ static void put_queue_utf8(struct vc_data *vc, u32 value)
/* FIXME: review locking for vt.c callers */
static void set_leds(void)
{
- tasklet_schedule(&keyboard_tasklet);
+ schedule_work(&keyboard_work);
}
/*
@@ -1024,10 +1024,10 @@ static int kbd_led_trigger_activate(struct led_classdev *cdev)
struct kbd_led_trigger *trigger =
container_of(cdev->trigger, struct kbd_led_trigger, trigger);
- tasklet_disable(&keyboard_tasklet);
+ disable_work_sync(&keyboard_work);
if (ledstate != -1U)
led_set_brightness(cdev, ledstate & trigger->mask ? LED_FULL : LED_OFF);
- tasklet_enable(&keyboard_tasklet);
+ enable_work(&keyboard_work);
return 0;
}
@@ -1243,7 +1243,7 @@ void vt_kbd_con_stop(unsigned int console)
* handle the scenario when keyboard handler is not registered yet
* but we already getting updates from the VT to update led state.
*/
-static void kbd_bh(struct tasklet_struct *unused)
+static void kbd_bh(struct work_struct *unused)
{
unsigned int leds;
unsigned long flags;
@@ -1535,7 +1535,7 @@ static void kbd_event(struct input_handle *handle, unsigned int event_type,
spin_unlock(&kbd_event_lock);
- tasklet_schedule(&keyboard_tasklet);
+ schedule_work(&keyboard_work);
do_poke_blanked_console = 1;
schedule_console_callback();
}
@@ -1607,12 +1607,12 @@ static void kbd_disconnect(struct input_handle *handle)
*/
static void kbd_start(struct input_handle *handle)
{
- tasklet_disable(&keyboard_tasklet);
+ disable_work_sync(&keyboard_work);
if (ledstate != -1U)
kbd_update_leds_helper(handle, &ledstate);
- tasklet_enable(&keyboard_tasklet);
+ enable_work(&keyboard_work);
}
static const struct input_device_id kbd_ids[] = {
@@ -1662,8 +1662,8 @@ int __init kbd_init(void)
if (error)
return error;
- tasklet_enable(&keyboard_tasklet);
- tasklet_schedule(&keyboard_tasklet);
+ enable_work(&keyboard_work);
+ schedule_work(&keyboard_work);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] workqueue: Add initialization macro for work items that disabled by default
2025-10-17 7:56 ` [PATCH v3 " Yu Peng
2025-10-17 7:56 ` [PATCH v3 2/2] tty/vt: Fix possible deadlock in input_inject_event Yu Peng
@ 2025-10-17 15:35 ` Tejun Heo
1 sibling, 0 replies; 10+ messages in thread
From: Tejun Heo @ 2025-10-17 15:35 UTC (permalink / raw)
To: Yu Peng
Cc: changlianzhi, dmitry.torokhov, gregkh, jiangshanlai, jirislaby,
legion, linux-kernel, linux-serial, lkp, mingo, myrrhperiwinkle,
oe-lkp, oliver.sang, syzbot+79c403850e6816dc39cf, tglx
On Fri, Oct 17, 2025 at 03:56:54PM +0800, Yu Peng wrote:
> In certain scenarios, workqueue tasks that are disabled by default are
> required. Similar to DECLARE_TASKLET_DISABLED, the DECLARE_WORK_DISABLED
> macro is added to achieve this functionality.
>
> Signed-off-by: Yu Peng <pengyu@kylinos.cn>
Applied to wq/for-6.19.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-10-17 15:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-28 13:08 [PATCH] tty/vt: Fix possible deadlock in input_inject_event pengyu
2025-09-29 4:54 ` Dmitry Torokhov
2025-10-01 10:42 ` pengyu
2025-09-30 6:31 ` kernel test robot
2025-10-01 10:23 ` [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default pengyu
2025-10-01 10:23 ` [PATCH v2 2/2] tty/vt: Fix possible deadlock in input_inject_event pengyu
2025-10-07 19:18 ` [PATCH v2 1/2] workqueue: Add initialization macro for work items that disabled by default Tejun Heo
2025-10-17 7:56 ` [PATCH v3 " Yu Peng
2025-10-17 7:56 ` [PATCH v3 2/2] tty/vt: Fix possible deadlock in input_inject_event Yu Peng
2025-10-17 15:35 ` [PATCH v3 1/2] workqueue: Add initialization macro for work items that disabled by default Tejun Heo
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).