Messages in this thread Patch in this message | | | Date | Wed, 10 Oct 2018 00:16:26 -0700 | From | Andrei Vagin <> | Subject | Re: [PATCH] ptrace: zero out siginfo_t in ptrace_peek_siginfo() |
| |
On Wed, Sep 26, 2018 at 05:17:25PM +0200, Alexander Potapenko wrote: > KMSAN reported the following infoleak: > > ================================================================== > BUG: KMSAN: kernel-infoleak in _copy_to_user+0x15d/0x1f0 > ... > Call Trace: > __dump_stack lib/dump_stack.c:77 > dump_stack+0x2f5/0x430 lib/dump_stack.c:113 > kmsan_report+0x183/0x2b0 mm/kmsan/kmsan.c:917 > kmsan_internal_check_memory+0x17e/0x1f0 mm/kmsan/kmsan.c:981 > kmsan_copy_to_user+0x79/0xc0 mm/kmsan/kmsan_hooks.c:482 > _copy_to_user+0x15d/0x1f0 lib/usercopy.c:31 > copy_to_user ./include/linux/uaccess.h:183 > copy_siginfo_to_user+0x81/0x130 kernel/signal.c:2897 > ptrace_peek_siginfo kernel/ptrace.c:741 > ptrace_request+0x2278/0x2680 kernel/ptrace.c:912 > arch_ptrace+0xbdd/0x11a0 arch/x86/kernel/ptrace.c:877 > __do_sys_ptrace kernel/ptrace.c:1145 > __se_sys_ptrace+0x422/0x920 kernel/ptrace.c:1110 > __x64_sys_ptrace+0x56/0x70 kernel/ptrace.c:1110 > do_syscall_64+0xb8/0x100 arch/x86/entry/common.c:291 > entry_SYSCALL_64_after_hwframe+0x63/0xe7 arch/x86/entry/entry_64.S:240 > ... > Local variable description: ----info.i@ptrace_request > Variable was created at: > ptrace_peek_siginfo kernel/ptrace.c:712 > ptrace_request+0xdf/0x2680 kernel/ptrace.c:912 > arch_ptrace+0xbdd/0x11a0 arch/x86/kernel/ptrace.c:877 > > Bytes 16-127 of 128 are uninitialized > Memory access starts at ffff88007af6fc90 > ================================================================== > > when calling ptrace(PTRACE_PEEKSIGINFO) for a traceable child process > with args = {-1, 0, 1}. > > Initialize the |info| structure to avoid leaking stack data.
"info" is filled up by copy_siginfo(), which overwrites everything.
static inline void copy_siginfo(struct siginfo *to, const struct siginfo *from) { memcpy(to, from, sizeof(*to)); }
so here is another problem. We handle arg.off incorrectly. The right fix should look something like this:
diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 21fec73d45d4..e336434a6f71 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -710,7 +710,7 @@ static int ptrace_peek_siginfo(struct task_struct *child, for (i = 0; i < arg.nr; ) { siginfo_t info; - s32 off = arg.off + i; + u64 off = arg.off + i; spin_lock_irq(&child->sighand->siglock); list_for_each_entry(q, &pending->list, list) { @@ -721,7 +721,7 @@ static int ptrace_peek_siginfo(struct task_struct *child, } spin_unlock_irq(&child->sighand->siglock); - if (off >= 0) /* beyond the end of the list */ + if (off + 1 != 0) /* beyond the end of the list */ break; #ifdef CONFIG_COMPAT
> > Signed-off-by: Alexander Potapenko <glider@google.com> > Reported-by: syzbot+69c3bd9869b32e394c48@syzkaller.appspotmail.com > Fixes: 84c751bd4aebb ("ptrace: add ability to retrieve signals without > removing from a queue (v4)") > Cc: Andrey Vagin <avagin@openvz.org> > Cc: Oleg Nesterov <oleg@redhat.com> > Cc: Willy Tarreau <w@1wt.eu> > --- > kernel/ptrace.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/ptrace.c b/kernel/ptrace.c > index 21fec73d45d4..92c3855c2b9c 100644 > --- a/kernel/ptrace.c > +++ b/kernel/ptrace.c > @@ -712,6 +712,7 @@ static int ptrace_peek_siginfo(struct task_struct *child, > siginfo_t info; > s32 off = arg.off + i; > > + memset(&info, 0, sizeof(info)); > spin_lock_irq(&child->sighand->siglock); > list_for_each_entry(q, &pending->list, list) { > if (!off--) { > -- > 2.19.0.605.g01d371f741-goog >
| |