Skip to content

Commit f6e2aa9

Browse files
committed
signal/ptrace: Don't leak unitialized kernel memory with PTRACE_PEEK_SIGINFO
Recently syzbot in conjunction with KMSAN reported that ptrace_peek_siginfo can copy an uninitialized siginfo to userspace. Inspecting ptrace_peek_siginfo confirms this. The problem is that off when initialized from args.off can be initialized to a negaive value. At which point the "if (off >= 0)" test to see if off became negative fails because off started off negative. Prevent the core problem by adding a variable found that is only true if a siginfo is found and copied to a temporary in preparation for being copied to userspace. Prevent args.off from being truncated when being assigned to off by testing that off is <= the maximum possible value of off. Convert off to an unsigned long so that we should not have to truncate args.off, we have well defined overflow behavior so if we add another check we won't risk fighting undefined compiler behavior, and so that we have a type whose maximum value is easy to test for. Cc: Andrei Vagin <[email protected]> Cc: [email protected] Reported-by: [email protected] Fixes: 84c751b ("ptrace: add ability to retrieve signals without removing from a queue (v4)") Signed-off-by: "Eric W. Biederman" <[email protected]>
1 parent a188339 commit f6e2aa9

File tree

1 file changed

+8
-2
lines changed

1 file changed

+8
-2
lines changed

kernel/ptrace.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -704,25 +704,31 @@ static int ptrace_peek_siginfo(struct task_struct *child,
704704
if (arg.nr < 0)
705705
return -EINVAL;
706706

707+
/* Ensure arg.off fits in an unsigned long */
708+
if (arg.off > ULONG_MAX)
709+
return 0;
710+
707711
if (arg.flags & PTRACE_PEEKSIGINFO_SHARED)
708712
pending = &child->signal->shared_pending;
709713
else
710714
pending = &child->pending;
711715

712716
for (i = 0; i < arg.nr; ) {
713717
kernel_siginfo_t info;
714-
s32 off = arg.off + i;
718+
unsigned long off = arg.off + i;
719+
bool found = false;
715720

716721
spin_lock_irq(&child->sighand->siglock);
717722
list_for_each_entry(q, &pending->list, list) {
718723
if (!off--) {
724+
found = true;
719725
copy_siginfo(&info, &q->info);
720726
break;
721727
}
722728
}
723729
spin_unlock_irq(&child->sighand->siglock);
724730

725-
if (off >= 0) /* beyond the end of the list */
731+
if (!found) /* beyond the end of the list */
726732
break;
727733

728734
#ifdef CONFIG_COMPAT

0 commit comments

Comments
 (0)