[syzbot] [kernel?] WARNING in posixtimer_send_sigqueue (2)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 78d4f34e2115 Linux 6.13-rc3
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10010b44580000
kernel config: https://syzkaller.appspot.com/x/.config?x=b86955f5c0c7be27
dashboard link: https://syzkaller.appspot.com/bug?extid=3c2e3cc60665d71de2f7
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10d4b4f8580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=132b9730580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7feb34a89c2a/non_bootable_disk-78d4f34e.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/e3d545fe5a74/vmlinux-78d4f34e.xz
kernel image: https://storage.googleapis.com/syzbot-assets/3c244cffe535/bzImage-78d4f34e.xz

The issue was bisected to:

commit df7a996b4dab03c889fa86d849447b716f07b069
Author: Thomas Gleixner <tg...@linutronix.de>
Date: Tue Nov 5 08:14:54 2024 +0000

signal: Queue ignored posixtimers on ignore list

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=110727e8580000
final oops: https://syzkaller.appspot.com/x/report.txt?x=130727e8580000
console output: https://syzkaller.appspot.com/x/log.txt?x=150727e8580000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+3c2e3c...@syzkaller.appspotmail.com
Fixes: df7a996b4dab ("signal: Queue ignored posixtimers on ignore list")

------------[ cut here ]------------
WARNING: CPU: 1 PID: 5933 at kernel/signal.c:2050 posixtimer_send_sigqueue+0xba8/0x1020 kernel/signal.c:2050
Modules linked in:
CPU: 1 UID: 0 PID: 5933 Comm: syz-executor261 Not tainted 6.13.0-rc3-syzkaller #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:posixtimer_send_sigqueue+0xba8/0x1020 kernel/signal.c:2050
Code: ff ff 4c 89 e7 e8 98 ff 9d 00 e9 7e f8 ff ff 41 bf 02 00 00 00 e9 87 f8 ff ff 48 89 54 24 10 48 89 44 24 08 e8 59 51 3b 00 90 <0f> 0b 90 48 8d 7b 10 48 8b 44 24 08 48 b9 00 00 00 00 00 fc ff df
RSP: 0018:ffffc900006b0d50 EFLAGS: 00010046
RAX: 0000000080010003 RBX: ffff888031d62000 RCX: 1ffff110063ac403
RDX: ffff888022920000 RSI: ffffffff815ec2e7 RDI: 0000000000000001
RBP: ffff888029fb2440 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000003 R12: ffff888031d620a4
R13: 1ffff920000d61af R14: ffff888031d620d8 R15: ffff888031d620c0
FS: 00007fb777ee36c0(0000) GS:ffff88806a700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020040fe0 CR3: 0000000023c34000 CR4: 0000000000352ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<IRQ>
posix_timer_fn+0x31/0x60 kernel/time/posix-timers.c:323
__run_hrtimer kernel/time/hrtimer.c:1739 [inline]
__hrtimer_run_queues+0x20a/0xae0 kernel/time/hrtimer.c:1803
hrtimer_interrupt+0x392/0x8e0 kernel/time/hrtimer.c:1865
local_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1038 [inline]
__sysvec_apic_timer_interrupt+0x10f/0x400 arch/x86/kernel/apic/apic.c:1055
instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1049 [inline]
sysvec_apic_timer_interrupt+0x9f/0xc0 arch/x86/kernel/apic/apic.c:1049
</IRQ>
<TASK>
asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:702
RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:152 [inline]
RIP: 0010:_raw_spin_unlock_irqrestore+0x31/0x80 kernel/locking/spinlock.c:194
Code: f5 53 48 8b 74 24 10 48 89 fb 48 83 c7 18 e8 86 04 4b f6 48 89 df e8 ce 83 4b f6 f7 c5 00 02 00 00 75 23 9c 58 f6 c4 02 75 37 <bf> 01 00 00 00 e8 c5 68 3c f6 65 8b 05 96 23 d8 74 85 c0 74 16 5b
RSP: 0018:ffffc900028f7d58 EFLAGS: 00000246
RAX: 0000000000000002 RBX: ffff888031d62030 RCX: 1ffffffff20bdd41
RDX: 0000000000000000 RSI: ffffffff8b6cd840 RDI: ffffffff8bd1e7e0
RBP: 0000000000000293 R08: 0000000000000001 R09: 0000000000000001
R10: ffffffff905f2c57 R11: 0000000000000002 R12: ffffc900028f7e50
R13: 0000000000000000 R14: 1ffff9200051efb1 R15: dffffc0000000000
spin_unlock_irqrestore include/linux/spinlock.h:406 [inline]
unlock_timer kernel/time/posix-timers.c:128 [inline]
do_timer_settime+0x315/0x400 kernel/time/posix-timers.c:908
__do_sys_timer_settime kernel/time/posix-timers.c:928 [inline]
__se_sys_timer_settime kernel/time/posix-timers.c:914 [inline]
__x64_sys_timer_settime+0x26a/0x2c0 kernel/time/posix-timers.c:914
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fb777f283a9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 51 18 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fb777ee3228 EFLAGS: 00000246 ORIG_RAX: 00000000000000df
RAX: ffffffffffffffda RBX: 000000000000001e RCX: 00007fb777f283a9
RDX: 0000000020040fe0 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 00007fb777fb2308 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fb777fb2300
R13: 00007fb777fb230c R14: 00007fffae0f2ba0 R15: 00007fffae0f2c88
</TASK>
----------------
Code disassembly (best guess):
0: f5 cmc
1: 53 push %rbx
2: 48 8b 74 24 10 mov 0x10(%rsp),%rsi
7: 48 89 fb mov %rdi,%rbx
a: 48 83 c7 18 add $0x18,%rdi
e: e8 86 04 4b f6 call 0xf64b0499
13: 48 89 df mov %rbx,%rdi
16: e8 ce 83 4b f6 call 0xf64b83e9
1b: f7 c5 00 02 00 00 test $0x200,%ebp
21: 75 23 jne 0x46
23: 9c pushf
24: 58 pop %rax
25: f6 c4 02 test $0x2,%ah
28: 75 37 jne 0x61
* 2a: bf 01 00 00 00 mov $0x1,%edi <-- trapping instruction
2f: e8 c5 68 3c f6 call 0xf63c68f9
34: 65 8b 05 96 23 d8 74 mov %gs:0x74d82396(%rip),%eax # 0x74d823d1
3b: 85 c0 test %eax,%eax
3d: 74 16 je 0x55
3f: 5b pop %rbx


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

[Thomas Gleixner]

syzbot triggered the warning in posixtimer_send_sigqueue(), which warns
about a non-ignored signal being already queued on the ignored list:

WARNING: ... at kernel/signal.c:2050 posixtimer_send_sigqueue

The warning is actually bogus, as the following valid sequence can
trigger it:

signal($SIG, SIGIGN);
timer_settime(...); // arm periodic timer

timer fires, signal is ignored and queued on ignored list

sigprocmask(SIG_BLOCK, ...); // block the signal
timer_settime(...); // re-arm periodic timer

timer fires, signal is not ignored because it is blocked
---> Warning triggers as signal is on the ignored list

Ideally timer_settime() should remove the signal, but that's racy and
incomplete vs. other scenarios and requires a full re-evaluation of the
pending signal list.

Instead of adding more complexity, handle it gracefully by removing the
warning and requeueing the signal to the pending list. If the signal gets
unblocked and is still ignored, it's going back to the ignore list. If a
handler was installed before unblocking, it's going to be delivered.

There is a related scenario to trigger the complementary warning in the
signal ignored path, which does not expect the signal to be on the pending
list when it is ignored.

WARNING: ... at kernel/signal.c:2014 posixtimer_send_sigqueue

That can be triggered even before the above change via:

task1 task2

signal($SIG, SIGIGN);
sigprocmask(SIG_BLOCK, ...);

timer_create(); // Signal target is task2
timer_settime(...); // arm periodic timer

timer fires, signal is not ignored because it is blocked
and queued on the pending list of task2

syscall()
// Sets the pending flag
sigprocmask(SIG_UNBLOCK, ...);

-> preemption, task2 does not make it
back to exit to userspace and therefore
cannot dequeue the signal before:

timer_settime(...); // re-arm periodic timer

timer fires, signal is ignored
---> Warning triggers as signal is on task2's pending list
and the thread group is not exiting

Consequently, remove that warning too and just keep the signal on the
pending list. If the signal is dequeued by task2 and still ignored, it will
be moved to the ignored list. If a handler gets installed before the
dequeue, then it will be delivered in the same way as a signal, which is on
the ignored list when SIGIGN is lifted and therefore moved back to the
pending list.

Fixes: df7a996b4dab ("signal: Queue ignored posixtimers on ignore list")

Reported-by: syzbot+3c2e3c...@syzkaller.appspotmail.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Closes: https://lore.kernel.org/all/6761b16e.050a022...@google.com
---
kernel/signal.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)

--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2007,11 +2007,23 @@ void posixtimer_send_sigqueue(struct k_i

if (!list_empty(&q->list)) {
/*
- * If task group is exiting with the signal already pending,
- * wait for __exit_signal() to do its job. Otherwise if
- * ignored, it's not supposed to be queued. Try to survive.
+ * The signal was ignored and blocked. The timer
+ * expiry queued it because blocked signals are
+ * queued independent of the ignored state.
+ *
+ * The unblocking set SIGPENDING, but the signal
+ * was not yet dequeued from the pending list,
+ * which would have put it back on the ignore list.
+ * So prepare_signal() sees unblocked and ignored,
+ * which ends up here. Leave it queued like a
+ * regular signal.
+ *
+ * The same happens when the task group is exiting
+ * and the signal is already queued.
+ * prepare_signal() treats SIGNAL_GROUP_EXIT as
+ * ignored independent of its queued state. This
+ * gets cleaned up in __exit_signal().
*/
- WARN_ON_ONCE(!(t->signal->flags & SIGNAL_GROUP_EXIT));
goto out;
}

@@ -2046,17 +2058,25 @@ void posixtimer_send_sigqueue(struct k_i
goto out;
}

- /* This should never happen and leaks a reference count */
- if (WARN_ON_ONCE(!hlist_unhashed(&tmr->ignored_list)))
- hlist_del_init(&tmr->ignored_list);
-
if (unlikely(!list_empty(&q->list))) {
/* This holds a reference count already */
result = TRACE_SIGNAL_ALREADY_PENDING;
goto out;
}

- posixtimer_sigqueue_getref(q);
+ /*
+ * If the signal is on the ignore list, it got blocked after it was
+ * ignored earlier. But nothing lifted the ignore. Move it back to
+ * the pending list to be consistent with the regular signal
+ * handling. If it gets unblocked, it will be ignored again unless
+ * a handler has been installed before unblocking. If it's not on
+ * the ignore list acquire a reference count.
+ */
+ if (likely(hlist_unhashed(&tmr->ignored_list)))
+ posixtimer_sigqueue_getref(q);
+ else
+ hlist_del_init(&tmr->ignored_list);
+
posixtimer_queue_sigqueue(q, t, tmr->it_pid_type);
result = TRACE_SIGNAL_DELIVERED;
out:

[Frederic Weisbecker]

Le Thu, Dec 19, 2024 at 08:46:25PM +0100, Thomas Gleixner a écrit :
> kernel/signal.c | 38 +++++++++++++++++++++++++++++---------
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -2007,11 +2007,23 @@ void posixtimer_send_sigqueue(struct k_i
>
> if (!list_empty(&q->list)) {
> /*
> - * If task group is exiting with the signal already pending,
> - * wait for __exit_signal() to do its job. Otherwise if
> - * ignored, it's not supposed to be queued. Try to survive.
> + * The signal was ignored and blocked. The timer
> + * expiry queued it because blocked signals are
> + * queued independent of the ignored state.
> + *
> + * The unblocking set SIGPENDING, but the signal
> + * was not yet dequeued from the pending list,
> + * which would have put it back on the ignore list.

I must be missing something. I don't see dequeue_signal() checking if a signal
is ignored upon delivery.

I'm not sure about that. If I follow the sigprocmask() path, set_task_blocked()
doesn't take care about that. And later on, dequeue_signal() doesn't seem to
check either...

Thanks.

[Thomas Gleixner]

On Fri, Dec 20 2024 at 14:06, Frederic Weisbecker wrote:
> Le Thu, Dec 19, 2024 at 08:46:25PM +0100, Thomas Gleixner a écrit :
>> if (!list_empty(&q->list)) {
>> /*
>> - * If task group is exiting with the signal already pending,
>> - * wait for __exit_signal() to do its job. Otherwise if
>> - * ignored, it's not supposed to be queued. Try to survive.
>> + * The signal was ignored and blocked. The timer
>> + * expiry queued it because blocked signals are
>> + * queued independent of the ignored state.
>> + *
>> + * The unblocking set SIGPENDING, but the signal
>> + * was not yet dequeued from the pending list,
>> + * which would have put it back on the ignore list.
>
> I must be missing something. I don't see dequeue_signal() checking if a signal
> is ignored upon delivery.

Sorry, I meant get_signal() which is what the actual signal delivery
path on exit to user space invokes. dequeue itself does not care.

[Frederic Weisbecker]

Hmm, ok it eventually ignores the signal delivery to the user but:

_ Dequeue signal has delivered it to posix timers
_ The signal isn't moved back to the ignored list (or I'm missing something else?)

Thanks.

[Thomas Gleixner]

Hmm. Right it is not, but that's correct for blocked signals because
they can be also retrieved by sig[timed]wait().

For the case where the signal is ignored, the dequeue and posix timer
delivery is not harmful. If it gets rearmed, then the next timer expiry
will see ignored and move it to the ignored list.

Let me stare at it some more and rewrite the comments to match reality.

Thanks for pointing it out!

tglx

[Thomas Gleixner]

syzbot triggered the warning in posixtimer_send_sigqueue(), which warns

about a non-ignored signal being already queued on the ignored list.

The warning is actually bogus, as the following sequence causes this:

signal($SIG, SIGIGN);
timer_settime(...); // arm periodic timer

timer fires, signal is ignored and queued on ignored list

sigprocmask(SIG_BLOCK, ...); // block the signal
timer_settime(...); // re-arm periodic timer

timer fires, signal is not ignored because it is blocked
---> Warning triggers as signal is on the ignored list

Ideally timer_settime() could remove the signal, but that's racy and
incomplete vs. other scenarios and requires a full reevaluation of the

pending signal list.

Instead of adding more complexity, handle it gracefully by removing the

warning and requeueing the signal to the pending list. That's correct
versus:

1) sig[timed]wait() as that does not check for SIGIGN and only relies on
dequeue_signal() -> posixtimers_deliver_signal() to check whether the
pending signal is still valid.

2) Unblocking of the signal.

- If the unblocking happens before SIGIGN is replaced by a signal
handler, then the timer is rearmed in dequeue_signal(), but
get_signal() will ignore it. The next timer expiry will move it back
to the ignored list.

- If SIGIGN was replaced before unblocking, then the signal will be
delivered and a subsequent expiry will queue a signal on the pending
list again.

There is a related scenario to trigger the complementary warning in the
signal ignored path, which does not expect the signal to be on the pending

list when it is ignored. That can be triggered even before the above change

via:

task1 task2

signal($SIG, SIGIGN);
sigprocmask(SIG_BLOCK, ...);

timer_create(); // Signal target is task2
timer_settime(...); // arm periodic timer

timer fires, signal is not ignored because it is blocked
and queued on the pending list of task2

syscall()
// Sets the pending flag
sigprocmask(SIG_UNBLOCK, ...);

-> preemption, task2 cannot dequeue the signal

timer_settime(...); // re-arm periodic timer

timer fires, signal is ignored
---> Warning triggers as signal is on task2's pending list
and the thread group is not exiting

Consequently, remove that warning too and just keep the signal on the
pending list.

The following attempt to deliver the signal on return to user space of
task2 will ignore the signal and a subsequent expiry will bring it back to
the ignored list, if it did not get blocked or un-ignored before that.

Fixes: df7a996b4dab ("signal: Queue ignored posixtimers on ignore list")
Reported-by: syzbot+3c2e3c...@syzkaller.appspotmail.com
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>

---
V2: Improve change log and comments - Frederic
---
kernel/signal.c | 37 ++++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)

--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2007,11 +2007,22 @@ void posixtimer_send_sigqueue(struct k_i

if (!list_empty(&q->list)) {
/*
- * If task group is exiting with the signal already pending,
- * wait for __exit_signal() to do its job. Otherwise if
- * ignored, it's not supposed to be queued. Try to survive.
+ * The signal was ignored and blocked. The timer
+ * expiry queued it because blocked signals are
+ * queued independent of the ignored state.
+ *
+ * The unblocking set SIGPENDING, but the signal

+ * was not yet dequeued from the pending list.

+ * So prepare_signal() sees unblocked and ignored,
+ * which ends up here. Leave it queued like a
+ * regular signal.

+ *

+ * The same happens when the task group is exiting
+ * and the signal is already queued.
+ * prepare_signal() treats SIGNAL_GROUP_EXIT as
+ * ignored independent of its queued state. This
+ * gets cleaned up in __exit_signal().
*/
- WARN_ON_ONCE(!(t->signal->flags & SIGNAL_GROUP_EXIT));
goto out;
}

@@ -2046,17 +2057,25 @@ void posixtimer_send_sigqueue(struct k_i

goto out;
}

- /* This should never happen and leaks a reference count */
- if (WARN_ON_ONCE(!hlist_unhashed(&tmr->ignored_list)))
- hlist_del_init(&tmr->ignored_list);
-
if (unlikely(!list_empty(&q->list))) {
/* This holds a reference count already */
result = TRACE_SIGNAL_ALREADY_PENDING;
goto out;
}

- posixtimer_sigqueue_getref(q);
+ /*
+ * If the signal is on the ignore list, it got blocked after it was
+ * ignored earlier. But nothing lifted the ignore. Move it back to
+ * the pending list to be consistent with the regular signal

+ * handling. This already holds a reference count.
+ *
+ * If it's not on the ignore list acquire a reference count.

[Frederic Weisbecker]

Reviewed-by: Frederic Weisbecker <fred...@kernel.org>