[PATCH] fs/fcntl: fix SOFTIRQ-unsafe lock order in send_sigio()

[PATCH] fs/fcntl: fix SOFTIRQ-unsafe lock order in send_sigio()

[w15303746062]

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

A SOFTIRQ-safe to SOFTIRQ-unsafe lock order deadlock can occur in
send_sigio() when a process group receives a SIGIO.

When FASYNC is configured for a process group (PIDTYPE_PGID),
send_sigio() uses read_lock(&tasklist_lock) to traverse the task
list. However, send_sigio() is often called from softirq context
(e.g., input_inject_event -> kill_fasync), where it already holds
SOFTIRQ-safe locks like &dev->event_lock and &f_owner->lock.

The deadlock is caused by the rwlock writer fairness mechanism:
1. CPU 0 (process context) holds read_lock(&tasklist_lock) in do_wait().
2. CPU 1 (process context) attempts write_lock(&tasklist_lock) in
   fork() or exit() and spins, which blocks all new readers.
3. CPU 0 is interrupted by a softirq (e.g., keyboard input event).
4. The softirq calls send_sigio() and attempts to acquire
   read_lock(&tasklist_lock), deadlocking because CPU 1 is waiting.

Since PID hashing and do_each_pid_task() traversals are already
RCU-protected, the read_lock on tasklist_lock is no longer strictly
required for safe traversal. Fix this by replacing tasklist_lock with
rcu_read_lock(), aligning the process group signaling path with the
single-PID path.

Lockdep splat:
=====================================================
WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
[...]
Chain exists of:
  &dev->event_lock --> &f_owner->lock --> tasklist_lock

Possible interrupt unsafe locking scenario:
       CPU0                    CPU1
       ----                    ----
  lock(tasklist_lock);
                           local_irq_disable();
                           lock(&dev->event_lock);
                           lock(&f_owner->lock);
  <Interrupt>
    lock(&dev->event_lock);

*** DEADLOCK ***

Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 fs/fcntl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index beab8080badf..a6c764ede282 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -929,11 +929,11 @@ void send_sigio(struct fown_struct *fown, int fd, int band)
 			send_sigio_to_task(p, fown, fd, band, type);
 		rcu_read_unlock();
 	} else {
-		read_lock(&tasklist_lock);
+		rcu_read_lock();
 		do_each_pid_task(pid, type, p) {
 			send_sigio_to_task(p, fown, fd, band, type);
 		} while_each_pid_task(pid, type, p);
-		read_unlock(&tasklist_lock);
+		rcu_read_unlock();
 	}
  out_unlock_fown:
 	read_unlock_irqrestore(&fown->lock, flags);
-- 
2.34.1

Re: [PATCH] fs/fcntl: fix SOFTIRQ-unsafe lock order in send_sigio()

[Jeff Layton]

On Sat, 2026-05-23 at 16:02 +0800, w15303746062@163.com wrote:
> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> 
> A SOFTIRQ-safe to SOFTIRQ-unsafe lock order deadlock can occur in
> send_sigio() when a process group receives a SIGIO.
> 
> When FASYNC is configured for a process group (PIDTYPE_PGID),
> send_sigio() uses read_lock(&tasklist_lock) to traverse the task
> list. However, send_sigio() is often called from softirq context
> (e.g., input_inject_event -> kill_fasync), where it already holds
> SOFTIRQ-safe locks like &dev->event_lock and &f_owner->lock.
> 
> The deadlock is caused by the rwlock writer fairness mechanism:
> 1. CPU 0 (process context) holds read_lock(&tasklist_lock) in do_wait().
> 2. CPU 1 (process context) attempts write_lock(&tasklist_lock) in
>    fork() or exit() and spins, which blocks all new readers.
> 3. CPU 0 is interrupted by a softirq (e.g., keyboard input event).
> 4. The softirq calls send_sigio() and attempts to acquire
>    read_lock(&tasklist_lock), deadlocking because CPU 1 is waiting.
> 
> Since PID hashing and do_each_pid_task() traversals are already
> RCU-protected, the read_lock on tasklist_lock is no longer strictly
> required for safe traversal. Fix this by replacing tasklist_lock with
> rcu_read_lock(), aligning the process group signaling path with the
> single-PID path.
> 
> Lockdep splat:
> =====================================================
> WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
> [...]
> Chain exists of:
>   &dev->event_lock --> &f_owner->lock --> tasklist_lock
> 
> Possible interrupt unsafe locking scenario:
>        CPU0                    CPU1
>        ----                    ----
>   lock(tasklist_lock);
>                            local_irq_disable();
>                            lock(&dev->event_lock);
>                            lock(&f_owner->lock);
>   <Interrupt>
>     lock(&dev->event_lock);
> 
> *** DEADLOCK ***
> 
> Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> ---
>  fs/fcntl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/fcntl.c b/fs/fcntl.c
> index beab8080badf..a6c764ede282 100644
> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -929,11 +929,11 @@ void send_sigio(struct fown_struct *fown, int fd, int band)
>  			send_sigio_to_task(p, fown, fd, band, type);
>  		rcu_read_unlock();
>  	} else {
> -		read_lock(&tasklist_lock);
> +		rcu_read_lock();
>  		do_each_pid_task(pid, type, p) {
>  			send_sigio_to_task(p, fown, fd, band, type);
>  		} while_each_pid_task(pid, type, p);
> -		read_unlock(&tasklist_lock);
> +		rcu_read_unlock();
>  	}
>   out_unlock_fown:
>  	read_unlock_irqrestore(&fown->lock, flags);

This looks good to me. Sashiko seems to think that send_sigurg has the
same problem though. Care to look into fixing that one too?

https://sashiko.dev/#/patchset/20260523080255.585201-1-w15303746062%40163.com

You can add:

Reviewed-by: Jeff Layton <jlayton@kernel.org>

[PATCH v2] fs/fcntl: fix SOFTIRQ-unsafe lock order in fasync signaling

[w15303746062]

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

A SOFTIRQ-safe to SOFTIRQ-unsafe lock order deadlock can occur in
send_sigio() and send_sigurg() when a process group receives a signal.

When FASYNC is configured for a process group (PIDTYPE_PGID), both
functions use read_lock(&tasklist_lock) to traverse the task list.
However, they are frequently called from softirq context:
- send_sigio() via input_inject_event -> kill_fasync
- send_sigurg() via tcp_check_urg -> sk_send_sigurg (NET_RX_SOFTIRQ)

The deadlock is caused by the rwlock writer fairness mechanism:
1. CPU 0 (process context) holds read_lock(&tasklist_lock) in do_wait().
2. CPU 1 (process context) attempts write_lock(&tasklist_lock) in
   fork() or exit() and spins, which blocks all new readers.
3. CPU 0 is interrupted by a softirq (e.g., TCP URG packet reception).
4. The softirq calls send_sigurg() and attempts to acquire
   read_lock(&tasklist_lock), deadlocking because CPU 1 is waiting.

Since PID hashing and do_each_pid_task() traversals are already
RCU-protected, the read_lock on tasklist_lock is no longer strictly
required for safe traversal. Fix this by replacing tasklist_lock with
rcu_read_lock(), aligning the process group signaling path with the
single-PID path. This also mitigates a potential remote denial of
service vector via TCP URG packets.

Lockdep splat:
=====================================================
WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
[...]
Chain exists of:
  &dev->event_lock --> &f_owner->lock --> tasklist_lock

Possible interrupt unsafe locking scenario:
       CPU0                    CPU1
       ----                    ----
  lock(tasklist_lock);
                           local_irq_disable();
                           lock(&dev->event_lock);
                           lock(&f_owner->lock);
  <Interrupt>
    lock(&dev->event_lock);

*** DEADLOCK ***

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
Changes in v2:
- Also apply the RCU replacement to send_sigurg() to fix an identical
  deadlock vector triggered by TCP URG packets.
- Add Reviewed-by tag from Jeff Layton.

 fs/fcntl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index beab8080badf..92d643a14196 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -929,11 +929,11 @@ void send_sigio(struct fown_struct *fown, int fd, int band)
 			send_sigio_to_task(p, fown, fd, band, type);
 		rcu_read_unlock();
 	} else {
-		read_lock(&tasklist_lock);
+		rcu_read_lock();
 		do_each_pid_task(pid, type, p) {
 			send_sigio_to_task(p, fown, fd, band, type);
 		} while_each_pid_task(pid, type, p);
-		read_unlock(&tasklist_lock);
+		rcu_read_unlock();
 	}
  out_unlock_fown:
 	read_unlock_irqrestore(&fown->lock, flags);
@@ -975,11 +975,11 @@ int send_sigurg(struct file *file)
 			send_sigurg_to_task(p, fown, type);
 		rcu_read_unlock();
 	} else {
-		read_lock(&tasklist_lock);
+		rcu_read_lock();
 		do_each_pid_task(pid, type, p) {
 			send_sigurg_to_task(p, fown, type);
 		} while_each_pid_task(pid, type, p);
-		read_unlock(&tasklist_lock);
+		rcu_read_unlock();
 	}
  out_unlock_fown:
 	read_unlock_irqrestore(&fown->lock, flags);
-- 
2.34.1

Re: [PATCH v2] fs/fcntl: fix SOFTIRQ-unsafe lock order in fasync signaling

[Christian Brauner]

On Sat, 23 May 2026 21:52:10 +0800, w15303746062@163.com wrote:
> A SOFTIRQ-safe to SOFTIRQ-unsafe lock order deadlock can occur in
> send_sigio() and send_sigurg() when a process group receives a signal.
> 
> When FASYNC is configured for a process group (PIDTYPE_PGID), both
> functions use read_lock(&tasklist_lock) to traverse the task list.
> However, they are frequently called from softirq context:
> - send_sigio() via input_inject_event -> kill_fasync
> - send_sigurg() via tcp_check_urg -> sk_send_sigurg (NET_RX_SOFTIRQ)
> 
> [...]

Applied to the vfs-7.2.misc branch of the vfs/vfs.git tree.
Patches in the vfs-7.2.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.2.misc

[1/1] fs/fcntl: fix SOFTIRQ-unsafe lock order in fasync signaling
      https://git.kernel.org/vfs/vfs/c/00633c468382

Re: [PATCH v2] fs/fcntl: fix SOFTIRQ-unsafe lock order in fasync signaling

[Oleg Nesterov]

Hi,

On 05/23, w15303746062@163.com wrote:
>
> Since PID hashing and do_each_pid_task() traversals are already
> RCU-protected, the read_lock on tasklist_lock is no longer strictly
> required for safe traversal. Fix this by replacing tasklist_lock with
> rcu_read_lock(),

I don't think we can simply avoid tasklist_lock. Let me quote my old email,

	do_each_pid_task(PIDTYPE_PGID) can race with change_pid(PIDTYPE_PGID)
	which moves the task from one hlist to another. Yes, it is safe in
	that task_struct can't go away. But still this is not right because
	do_each_pid_task() can scan the wrong (2nd) hlist.

And... I need to recheck, but perhaps signal_struct->multiprocess logic
is another reason why the lockless do_each_pid_task() is wrong...

See also the recent discussion
https://lore.kernel.org/all/20261003182224.2171b574@pumpkin/

Oleg.

> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -929,11 +929,11 @@ void send_sigio(struct fown_struct *fown, int fd, int band)
>  			send_sigio_to_task(p, fown, fd, band, type);
>  		rcu_read_unlock();
>  	} else {
> -		read_lock(&tasklist_lock);
> +		rcu_read_lock();
>  		do_each_pid_task(pid, type, p) {
>  			send_sigio_to_task(p, fown, fd, band, type);
>  		} while_each_pid_task(pid, type, p);
> -		read_unlock(&tasklist_lock);
> +		rcu_read_unlock();
>  	}
>   out_unlock_fown:
>  	read_unlock_irqrestore(&fown->lock, flags);
> @@ -975,11 +975,11 @@ int send_sigurg(struct file *file)
>  			send_sigurg_to_task(p, fown, type);
>  		rcu_read_unlock();
>  	} else {
> -		read_lock(&tasklist_lock);
> +		rcu_read_lock();
>  		do_each_pid_task(pid, type, p) {
>  			send_sigurg_to_task(p, fown, type);
>  		} while_each_pid_task(pid, type, p);
> -		read_unlock(&tasklist_lock);
> +		rcu_read_unlock();
>  	}
>   out_unlock_fown:
>  	read_unlock_irqrestore(&fown->lock, flags);
> -- 
> 2.34.1
> 

Re: [PATCH v2] fs/fcntl: fix SOFTIRQ-unsafe lock order in fasync signaling

[Oleg Nesterov]

Nobody replied ;)

In case it was not clear, I believe the commit 00633c4683828
("fs/fcntl: fix SOFTIRQ-unsafe lock order in fasync signaling")
backported to stable trees is wrong.

Right now the fix is not clear to me, I don't even fully understand
the changelog:

    The deadlock is caused by the rwlock writer fairness mechanism:
    1. CPU 0 (process context) holds read_lock(&tasklist_lock) in do_wait().
    2. CPU 1 (process context) attempts write_lock(&tasklist_lock) in
       fork() or exit() and spins, which blocks all new readers.

see below ...

    3. CPU 0 is interrupted by a softirq (e.g., TCP URG packet reception).
    4. The softirq calls send_sigurg() and attempts to acquire
       read_lock(&tasklist_lock), deadlocking because CPU 1 is waiting.

but queued_read_lock_slowpath() does not block the reader if
in_interrupt() == T ?

Mingyu, could you provide more details? How was this problem reported?

So far I only sent the doc patch to explain that we can't simply avoid tasklist:

	[PATCH] pid: document that do_each_pid_task() needs tasklist_lock
	https://lore.kernel.org/all/asuaU8H_5MbO3-cW@redhat.com/

Oleg.

On 10/04, Oleg Nesterov wrote:
>
> Hi,
>
> On 05/23, w15303746062@163.com wrote:
> >
> > Since PID hashing and do_each_pid_task() traversals are already
> > RCU-protected, the read_lock on tasklist_lock is no longer strictly
> > required for safe traversal. Fix this by replacing tasklist_lock with
> > rcu_read_lock(),
>
> I don't think we can simply avoid tasklist_lock. Let me quote my old email,
>
> 	do_each_pid_task(PIDTYPE_PGID) can race with change_pid(PIDTYPE_PGID)
> 	which moves the task from one hlist to another. Yes, it is safe in
> 	that task_struct can't go away. But still this is not right because
> 	do_each_pid_task() can scan the wrong (2nd) hlist.
>
> And... I need to recheck, but perhaps signal_struct->multiprocess logic
> is another reason why the lockless do_each_pid_task() is wrong...
>
> See also the recent discussion
> https://lore.kernel.org/all/20261003182224.2171b574@pumpkin/
>
> Oleg.
>
> > --- a/fs/fcntl.c
> > +++ b/fs/fcntl.c
> > @@ -929,11 +929,11 @@ void send_sigio(struct fown_struct *fown, int fd, int band)
> >  			send_sigio_to_task(p, fown, fd, band, type);
> >  		rcu_read_unlock();
> >  	} else {
> > -		read_lock(&tasklist_lock);
> > +		rcu_read_lock();
> >  		do_each_pid_task(pid, type, p) {
> >  			send_sigio_to_task(p, fown, fd, band, type);
> >  		} while_each_pid_task(pid, type, p);
> > -		read_unlock(&tasklist_lock);
> > +		rcu_read_unlock();
> >  	}
> >   out_unlock_fown:
> >  	read_unlock_irqrestore(&fown->lock, flags);
> > @@ -975,11 +975,11 @@ int send_sigurg(struct file *file)
> >  			send_sigurg_to_task(p, fown, type);
> >  		rcu_read_unlock();
> >  	} else {
> > -		read_lock(&tasklist_lock);
> > +		rcu_read_lock();
> >  		do_each_pid_task(pid, type, p) {
> >  			send_sigurg_to_task(p, fown, type);
> >  		} while_each_pid_task(pid, type, p);
> > -		read_unlock(&tasklist_lock);
> > +		rcu_read_unlock();
> >  	}
> >   out_unlock_fown:
> >  	read_unlock_irqrestore(&fown->lock, flags);
> > --
> > 2.34.1
> >

Re: [PATCH v2] fs/fcntl: fix SOFTIRQ-unsafe lock order in fasync signaling

[Mingyu Wang]

Hi Oleg,

Sorry for the delayed response due to the recent holiday break.

> Mingyu, could you provide more details? How was this problem reported?

The warning was found during kernel fuzzing with DevGen, our own testing
tool, in a QEMU x86_64 guest. The report identifies the kernel as
7.0.0 #6, PREEMPT(full).

This was a lockdep report. It does not establish the actual deadlock
interleaving described in my changelog, nor does it substantiate the
claim about a remote denial of service through TCP URG packets.

> but queued_read_lock_slowpath() does not block the reader if
> in_interrupt() == T ?

You are right. My two CPU scenario overlooked that an interrupt reader
can acquire the read lock while the writer is only waiting.

I also agree with your concern about change_pid(PIDTYPE_PGID). RCU
protects the task lifetime, but it does not prevent the traversal from
following a node into another hlist after that node is moved. My
justification for replacing tasklist_lock with RCU was incorrect.

In the original report, the task triggering the warning was in process
context, through a TTY ioctl. Here are the relevant excerpts; omissions
are marked with [...].

WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
7.0.0 #6 Tainted: G W N

syz.2.658/3606 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
ffffffff8760c098 (tasklist_lock){.+.+}-{3:3}, at: send_sigio+0xc0/0x380 
fs/fcntl.c:932

and this task is already holding:
ffff888105547220 (&f_owner->lock){....}-{3:3}, at: send_sigio+0x36/0x380 
fs/fcntl.c:918

which would create a new lock dependency:
(&f_owner->lock){....}-{3:3} -> (tasklist_lock){.+.+}-{3:3}

[...]

Chain exists of:
&dev->event_lock --> &f_owner->lock --> tasklist_lock

[...]

CPU: 3 UID: 0 PID: 3606 Comm: syz.2.658 Tainted: G W N 7.0.0 #6 
PREEMPT(full)

Call Trace:
<TASK>
[...]
send_sigio+0xc0/0x380 fs/fcntl.c:932
kill_fasync_rcu fs/fcntl.c:1144 [inline]
kill_fasync fs/fcntl.c:1159 [inline]
kill_fasync+0x21d/0x520 fs/fcntl.c:1152
__receive_buf drivers/tty/n_tty.c:1632 [inline]
n_tty_receive_buf_common+0x670/0x1770 drivers/tty/n_tty.c:1715
tty_ldisc_receive_buf+0xa8/0x1c0 drivers/tty/tty_buffer.c:387
paste_selection+0x605/0x730 drivers/tty/vt/selection.c:439
tioclinux+0x3d3/0x630 drivers/tty/vt/vt.c:3505
vt_ioctl+0x23ec/0x2ee0 drivers/tty/vt/vt_ioctl.c:753
tty_ioctl+0x717/0x1670 drivers/tty/tty_io.c:2792
[...]
entry_SYSCALL_64_after_hwframe+0x77/0x7f
[...]
</TASK>

The report separately records the following lock usage history. These
are historical acquisition paths, not simultaneous CPU stacks showing
a confirmed deadlock.

... which became SOFTIRQ-irq-safe at:
[...]
input_inject_event+0xa4/0x3b0 drivers/input/input.c:419
__led_set_brightness drivers/leds/led-core.c:52 [inline]
led_set_brightness_nopm drivers/leds/led-core.c:335 [inline]
led_set_brightness_nosleep drivers/leds/led-core.c:369 [inline]
led_set_brightness+0x21c/0x2a0 drivers/leds/led-core.c:328
led_trigger_event drivers/leds/led-triggers.c:420 [inline]
led_trigger_event+0xdf/0x270 drivers/leds/led-triggers.c:408
kbd_propagate_led_state drivers/tty/vt/keyboard.c:1065 [inline]
kbd_bh+0x23c/0x310 drivers/tty/vt/keyboard.c:1244
tasklet_action_common+0x25c/0x3d0 kernel/softirq.c:925
handle_softirqs+0x1ba/0x890 kernel/softirq.c:622
[...]

... which became SOFTIRQ-irq-unsafe at:
[...]
__do_wait+0xfe/0x870 kernel/exit.c:1678
do_wait+0x1d0/0x5a0 kernel/exit.c:1722
kernel_wait+0xa4/0x160 kernel/exit.c:1898
[...]

The recent program history includes id=658, which contains:

ioctl$TIOCL_PASTESEL(r1, 0x541c, &(0x7f00000000c0))

This is consistent with the reporting stack, but the program history
should not be treated as a validated minimal reproducer.

I support reverting the RCU replacement. I am sharing these excerpts
to clarify how the warning was reported and to help assess the original
warning and determine the appropriate next steps.

Thank you for pointing out these issues.

Mingyu

Re: [PATCH v2] fs/fcntl: fix SOFTIRQ-unsafe lock order in fasync signaling

[Jeff Layton]

On Sun, 2026-10-11 at 17:57 +0200, Oleg Nesterov wrote:
> Nobody replied ;)
> 
> In case it was not clear, I believe the commit 00633c4683828
> ("fs/fcntl: fix SOFTIRQ-unsafe lock order in fasync signaling")
> backported to stable trees is wrong.
> 
> Right now the fix is not clear to me, I don't even fully understand
> the changelog:
> 
>     The deadlock is caused by the rwlock writer fairness mechanism:
>     1. CPU 0 (process context) holds read_lock(&tasklist_lock) in do_wait().
>     2. CPU 1 (process context) attempts write_lock(&tasklist_lock) in
>        fork() or exit() and spins, which blocks all new readers.
> 
> see below ...
> 
>     3. CPU 0 is interrupted by a softirq (e.g., TCP URG packet reception).
>     4. The softirq calls send_sigurg() and attempts to acquire
>        read_lock(&tasklist_lock), deadlocking because CPU 1 is waiting.
> 
> but queued_read_lock_slowpath() does not block the reader if
> in_interrupt() == T ?
> 
> Mingyu, could you provide more details? How was this problem reported?
> 
> So far I only sent the doc patch to explain that we can't simply avoid tasklist:
> 
> 	[PATCH] pid: document that do_each_pid_task() needs tasklist_lock
> 	https://lore.kernel.org/all/asuaU8H_5MbO3-cW@redhat.com/
> 
> Oleg.
> 

FYI, I took a closer look, and I think Oleg is correct.

When I reviewed this, I missed the fact that the rcu_read_lock() isn't
sufficient to stabilize the do_each_pid_task() walk. We probably should
revert this patch for now, until the problem and potential solution are
more clear.

-- 
Jeff Layton <jlayton@kernel.org>