Skip to content

Commit

Permalink
signal/ptrace: Don't leak unitialized kernel memory with PTRACE_PEEK_…
Browse files Browse the repository at this point in the history
…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]>
  • Loading branch information
ebiederm committed May 30, 2019
1 parent a188339 commit f6e2aa9
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions kernel/ptrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -704,25 +704,31 @@ static int ptrace_peek_siginfo(struct task_struct *child,
if (arg.nr < 0)
return -EINVAL;

/* Ensure arg.off fits in an unsigned long */
if (arg.off > ULONG_MAX)
return 0;

if (arg.flags & PTRACE_PEEKSIGINFO_SHARED)
pending = &child->signal->shared_pending;
else
pending = &child->pending;

for (i = 0; i < arg.nr; ) {
kernel_siginfo_t info;
s32 off = arg.off + i;
unsigned long off = arg.off + i;
bool found = false;

spin_lock_irq(&child->sighand->siglock);
list_for_each_entry(q, &pending->list, list) {
if (!off--) {
found = true;
copy_siginfo(&info, &q->info);
break;
}
}
spin_unlock_irq(&child->sighand->siglock);

if (off >= 0) /* beyond the end of the list */
if (!found) /* beyond the end of the list */
break;

#ifdef CONFIG_COMPAT
Expand Down

0 comments on commit f6e2aa9

Please sign in to comment.