]>
Commit | Line | Data |
---|---|---|
96f81b03 GKH |
1 | From f6e2aa91a46d2bc79fce9b93a988dbe7655c90c0 Mon Sep 17 00:00:00 2001 |
2 | From: "Eric W. Biederman" <ebiederm@xmission.com> | |
3 | Date: Tue, 28 May 2019 18:46:37 -0500 | |
4 | Subject: signal/ptrace: Don't leak unitialized kernel memory with PTRACE_PEEK_SIGINFO | |
5 | ||
6 | From: Eric W. Biederman <ebiederm@xmission.com> | |
7 | ||
8 | commit f6e2aa91a46d2bc79fce9b93a988dbe7655c90c0 upstream. | |
9 | ||
10 | Recently syzbot in conjunction with KMSAN reported that | |
11 | ptrace_peek_siginfo can copy an uninitialized siginfo to userspace. | |
12 | Inspecting ptrace_peek_siginfo confirms this. | |
13 | ||
14 | The problem is that off when initialized from args.off can be | |
15 | initialized to a negaive value. At which point the "if (off >= 0)" | |
16 | test to see if off became negative fails because off started off | |
17 | negative. | |
18 | ||
19 | Prevent the core problem by adding a variable found that is only true | |
20 | if a siginfo is found and copied to a temporary in preparation for | |
21 | being copied to userspace. | |
22 | ||
23 | Prevent args.off from being truncated when being assigned to off by | |
24 | testing that off is <= the maximum possible value of off. Convert off | |
25 | to an unsigned long so that we should not have to truncate args.off, | |
26 | we have well defined overflow behavior so if we add another check we | |
27 | won't risk fighting undefined compiler behavior, and so that we have a | |
28 | type whose maximum value is easy to test for. | |
29 | ||
30 | Cc: Andrei Vagin <avagin@gmail.com> | |
31 | Cc: stable@vger.kernel.org | |
32 | Reported-by: syzbot+0d602a1b0d8c95bdf299@syzkaller.appspotmail.com | |
33 | Fixes: 84c751bd4aeb ("ptrace: add ability to retrieve signals without removing from a queue (v4)") | |
34 | Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> | |
35 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
36 | ||
37 | --- | |
38 | kernel/ptrace.c | 9 +++++++-- | |
39 | 1 file changed, 7 insertions(+), 2 deletions(-) | |
40 | ||
41 | --- a/kernel/ptrace.c | |
42 | +++ b/kernel/ptrace.c | |
43 | @@ -710,6 +710,10 @@ static int ptrace_peek_siginfo(struct ta | |
44 | if (arg.nr < 0) | |
45 | return -EINVAL; | |
46 | ||
47 | + /* Ensure arg.off fits in an unsigned long */ | |
48 | + if (arg.off > ULONG_MAX) | |
49 | + return 0; | |
50 | + | |
51 | if (arg.flags & PTRACE_PEEKSIGINFO_SHARED) | |
52 | pending = &child->signal->shared_pending; | |
53 | else | |
54 | @@ -717,7 +721,8 @@ static int ptrace_peek_siginfo(struct ta | |
55 | ||
56 | for (i = 0; i < arg.nr; ) { | |
57 | siginfo_t info; | |
58 | - s32 off = arg.off + i; | |
59 | + unsigned long off = arg.off + i; | |
60 | + bool found = false; | |
61 | ||
62 | spin_lock_irq(&child->sighand->siglock); | |
63 | list_for_each_entry(q, &pending->list, list) { | |
64 | @@ -728,7 +733,7 @@ static int ptrace_peek_siginfo(struct ta | |
65 | } | |
66 | spin_unlock_irq(&child->sighand->siglock); | |
67 | ||
68 | - if (off >= 0) /* beyond the end of the list */ | |
69 | + if (!found) /* beyond the end of the list */ | |
70 | break; | |
71 | ||
72 | #ifdef CONFIG_COMPAT |