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