| From: "Eric W. Biederman" <ebiederm@xmission.com> |
| Date: Tue, 28 May 2019 18:46:37 -0500 |
| Subject: signal/ptrace: Don't leak unitialized kernel memory with |
| PTRACE_PEEK_SIGINFO |
| |
| commit f6e2aa91a46d2bc79fce9b93a988dbe7655c90c0 upstream. |
| |
| 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 <avagin@gmail.com> |
| Reported-by: syzbot+0d602a1b0d8c95bdf299@syzkaller.appspotmail.com |
| Fixes: 84c751bd4aeb ("ptrace: add ability to retrieve signals without removing from a queue (v4)") |
| Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> |
| [bwh: Backported to 3.16: adjust context] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| kernel/ptrace.c | 10 ++++++++-- |
| 1 file changed, 8 insertions(+), 2 deletions(-) |
| |
| --- a/kernel/ptrace.c |
| +++ b/kernel/ptrace.c |
| @@ -711,6 +711,10 @@ static int ptrace_peek_siginfo(struct ta |
| 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 |
| @@ -718,18 +722,20 @@ static int ptrace_peek_siginfo(struct ta |
| |
| for (i = 0; i < arg.nr; ) { |
| 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 |