Re: sctp: kernel memory overwrite attempt detected in sctp_getsockopt_assoc_stats

2017-01-17 Thread Kees Cook
On Mon, Jan 16, 2017 at 6:56 AM, Dmitry Vyukov  wrote:
> On Mon, Jan 16, 2017 at 3:50 PM, David Laight  wrote:
>> From: Dmitry Vyukov
>>> Sent: 16 January 2017 14:04
>>> >> >> I've enabled CONFIG_HARDENED_USERCOPY_PAGESPAN on syzkaller fuzzer and
>> ...
>>> >> The code also takes into account compound pages. As far as I
>>> >> understand the intention of the check is to effectively find
>>> >> out-of-bounds copies (e.g. goes beyond the current heap allocation). I
>>> >> would expect that stacks are allocated as compound pages and don't
>>> >> trigger this check. I don't see it is firing in other similar places.
>>> >>
>>> > Honestly, I'm not overly familiar with stack page allocation, at least 
>>> > not so
>>> > far as compound vs. single page allocation is concerned.  I suppose the 
>>> > question
>>> > your really asking here is: Have you found a case in which the syscall 
>>> > fuzzer
>>> > has forced the allocation of an insecure non-compound page on the stack, 
>>> > or is
>>> > this a false positive warning.  I can't provide the answer to that.
>>>
>>> Yes. I added Kees, author of CONFIG_HARDENED_USERCOPY_PAGESPAN, to To line.
>>> Kees, is this a false positive?
>>
>> I'd guess that the kernel stack is (somehow) allocated page by page
>> rather than by a single multi-page allocate.
>> Or maybe vmalloc() isn't setting the required flag??
>
>
> Just in case, I don't have CONFIG_VMAP_STACK selected.
> If it is a generic issue, then CONFIG_HARDENED_USERCOPY_PAGESPAN looks
> considerably broken as there are tons of copies onto stack. I don't
> see what's special in this particular case.

There have been so many false positives on this option, even though it
is known not to be quite right, that I'll probably just remove it
entirely. It clearly needs much more work before it'll be useful, so
there's no reason to leave it in the kernel to confuse people. :)

-Kees

-- 
Kees Cook
Nexus Security


Re: sctp: kernel memory overwrite attempt detected in sctp_getsockopt_assoc_stats

2017-01-16 Thread Dmitry Vyukov
On Mon, Jan 16, 2017 at 3:50 PM, David Laight  wrote:
> From: Dmitry Vyukov
>> Sent: 16 January 2017 14:04
>> >> >> I've enabled CONFIG_HARDENED_USERCOPY_PAGESPAN on syzkaller fuzzer and
> ...
>> >> The code also takes into account compound pages. As far as I
>> >> understand the intention of the check is to effectively find
>> >> out-of-bounds copies (e.g. goes beyond the current heap allocation). I
>> >> would expect that stacks are allocated as compound pages and don't
>> >> trigger this check. I don't see it is firing in other similar places.
>> >>
>> > Honestly, I'm not overly familiar with stack page allocation, at least not 
>> > so
>> > far as compound vs. single page allocation is concerned.  I suppose the 
>> > question
>> > your really asking here is: Have you found a case in which the syscall 
>> > fuzzer
>> > has forced the allocation of an insecure non-compound page on the stack, 
>> > or is
>> > this a false positive warning.  I can't provide the answer to that.
>>
>> Yes. I added Kees, author of CONFIG_HARDENED_USERCOPY_PAGESPAN, to To line.
>> Kees, is this a false positive?
>
> I'd guess that the kernel stack is (somehow) allocated page by page
> rather than by a single multi-page allocate.
> Or maybe vmalloc() isn't setting the required flag??


Just in case, I don't have CONFIG_VMAP_STACK selected.
If it is a generic issue, then CONFIG_HARDENED_USERCOPY_PAGESPAN looks
considerably broken as there are tons of copies onto stack. I don't
see what's special in this particular case.


RE: sctp: kernel memory overwrite attempt detected in sctp_getsockopt_assoc_stats

2017-01-16 Thread David Laight
From: Dmitry Vyukov
> Sent: 16 January 2017 14:04
> >> >> I've enabled CONFIG_HARDENED_USERCOPY_PAGESPAN on syzkaller fuzzer and
...
> >> The code also takes into account compound pages. As far as I
> >> understand the intention of the check is to effectively find
> >> out-of-bounds copies (e.g. goes beyond the current heap allocation). I
> >> would expect that stacks are allocated as compound pages and don't
> >> trigger this check. I don't see it is firing in other similar places.
> >>
> > Honestly, I'm not overly familiar with stack page allocation, at least not 
> > so
> > far as compound vs. single page allocation is concerned.  I suppose the 
> > question
> > your really asking here is: Have you found a case in which the syscall 
> > fuzzer
> > has forced the allocation of an insecure non-compound page on the stack, or 
> > is
> > this a false positive warning.  I can't provide the answer to that.
> 
> Yes. I added Kees, author of CONFIG_HARDENED_USERCOPY_PAGESPAN, to To line.
> Kees, is this a false positive?

I'd guess that the kernel stack is (somehow) allocated page by page
rather than by a single multi-page allocate.
Or maybe vmalloc() isn't setting the required flag??

David



Re: sctp: kernel memory overwrite attempt detected in sctp_getsockopt_assoc_stats

2017-01-16 Thread Dmitry Vyukov
On Mon, Jan 16, 2017 at 2:57 PM, Neil Horman  wrote:
> On Mon, Jan 16, 2017 at 08:11:40AM +0100, Dmitry Vyukov wrote:
>> On Sun, Jan 15, 2017 at 9:35 PM, Neil Horman  wrote:
>> > On Sun, Jan 15, 2017 at 06:29:59PM +0100, Dmitry Vyukov wrote:
>> >> Hello,
>> >>
>> >> I've enabled CONFIG_HARDENED_USERCOPY_PAGESPAN on syzkaller fuzzer and
>> >> now I am seeing lots of:
>> >>
>> > If I'm not mistaken, its because thats specifically what that option does. 
>> >  From
>> > the Kconfig:
>> > onfig HARDENED_USERCOPY_PAGESPAN
>> > bool "Refuse to copy allocations that span multiple pages"
>> > depends on HARDENED_USERCOPY
>> > depends on EXPERT
>> > help
>> >   When a multi-page allocation is done without __GFP_COMP,
>> >   hardened usercopy will reject attempts to copy it. There are,
>> >   however, several cases of this in the kernel that have not all
>> >   been removed. This config is intended to be used only while
>> >   trying to find such users.
>> >
>> > So, if the fuzzer does a setsockopt and the data it passes crosses a page
>> > boundary, it seems like this will get triggered.  Based on the fact that 
>> > its
>> > only used to find users that do this, it seems like not the sort of thing 
>> > that
>> > you want enabled while running a fuzzer that might do it arbitrarily.
>>
>>
>> The code also takes into account compound pages. As far as I
>> understand the intention of the check is to effectively find
>> out-of-bounds copies (e.g. goes beyond the current heap allocation). I
>> would expect that stacks are allocated as compound pages and don't
>> trigger this check. I don't see it is firing in other similar places.
>>
> Honestly, I'm not overly familiar with stack page allocation, at least not so
> far as compound vs. single page allocation is concerned.  I suppose the 
> question
> your really asking here is: Have you found a case in which the syscall fuzzer
> has forced the allocation of an insecure non-compound page on the stack, or is
> this a false positive warning.  I can't provide the answer to that.


Yes. I added Kees, author of CONFIG_HARDENED_USERCOPY_PAGESPAN, to To line.
Kees, is this a false positive?


Re: sctp: kernel memory overwrite attempt detected in sctp_getsockopt_assoc_stats

2017-01-16 Thread Neil Horman
On Mon, Jan 16, 2017 at 08:11:40AM +0100, Dmitry Vyukov wrote:
> On Sun, Jan 15, 2017 at 9:35 PM, Neil Horman  wrote:
> > On Sun, Jan 15, 2017 at 06:29:59PM +0100, Dmitry Vyukov wrote:
> >> Hello,
> >>
> >> I've enabled CONFIG_HARDENED_USERCOPY_PAGESPAN on syzkaller fuzzer and
> >> now I am seeing lots of:
> >>
> > If I'm not mistaken, its because thats specifically what that option does.  
> > From
> > the Kconfig:
> > onfig HARDENED_USERCOPY_PAGESPAN
> > bool "Refuse to copy allocations that span multiple pages"
> > depends on HARDENED_USERCOPY
> > depends on EXPERT
> > help
> >   When a multi-page allocation is done without __GFP_COMP,
> >   hardened usercopy will reject attempts to copy it. There are,
> >   however, several cases of this in the kernel that have not all
> >   been removed. This config is intended to be used only while
> >   trying to find such users.
> >
> > So, if the fuzzer does a setsockopt and the data it passes crosses a page
> > boundary, it seems like this will get triggered.  Based on the fact that its
> > only used to find users that do this, it seems like not the sort of thing 
> > that
> > you want enabled while running a fuzzer that might do it arbitrarily.
> 
> 
> The code also takes into account compound pages. As far as I
> understand the intention of the check is to effectively find
> out-of-bounds copies (e.g. goes beyond the current heap allocation). I
> would expect that stacks are allocated as compound pages and don't
> trigger this check. I don't see it is firing in other similar places.
> 
Honestly, I'm not overly familiar with stack page allocation, at least not so
far as compound vs. single page allocation is concerned.  I suppose the question
your really asking here is: Have you found a case in which the syscall fuzzer
has forced the allocation of an insecure non-compound page on the stack, or is
this a false positive warning.  I can't provide the answer to that.

Neil

> 
> 
> >> usercopy: kernel memory overwrite attempt detected to 8801a74f6f10
> >> () (256 bytes)
> >>
> >> kernel BUG at mm/usercopy.c:75!
> >> invalid opcode:  [#1] SMP KASAN
> >> Dumping ftrace buffer:
> >>(ftrace buffer empty)
> >> Modules linked in:
> >> CPU: 1 PID: 15686 Comm: syz-executor3 Not tainted 4.9.0 #1
> >> Hardware name: Google Google Compute Engine/Google Compute Engine,
> >> BIOS Google 01/01/2011
> >> task: 8801c89b2500 task.stack: 8801a74f
> >> RIP: 0010:[]  [] report_usercopy
> >> mm/usercopy.c:67 [inline]
> >> RIP: 0010:[]  []
> >> __check_object_size+0x2d1/0x9ec mm/usercopy.c:278
> >> RSP: 0018:8801a74f6cd0  EFLAGS: 00010286
> >> RAX: 006b RBX: 84500120 RCX: 
> >> RDX: 006b RSI: 815a7791 RDI: ed0034e9ed8c
> >> RBP: 8801a74f6e48 R08: 0001 R09: 
> >> R10:  R11: 0001 R12: 8801a74f6f10
> >> R13: 0100 R14: 845000e0 R15: 8801a74f700f
> >> FS:  7f80918de700() GS:8801dc10() 
> >> knlGS:
> >> CS:  0010 DS:  ES:  CR0: 80050033
> >> CR2: 20058ffc CR3: 0001cc1cc000 CR4: 001406e0
> >> Stack:
> >>  8598fcc8  77ff8000 ea0005c99608
> >>  844fff40 844fff40 41b58ab3 84ae0fa0
> >>  81a1ad70 8801c89b2500 dead0100 814d4425
> >> Call Trace:
> >>  [] check_object_size include/linux/thread_info.h:129 
> >> [inline]
> >>  [] copy_from_user arch/x86/include/asm/uaccess.h:692 
> >> [inline]
> >>  [] sctp_getsockopt_assoc_stats+0x169/0xa10
> >> net/sctp/socket.c:6182
> >>  [] sctp_getsockopt+0x1af2/0x66a0 net/sctp/socket.c:6556
> >>  [] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2649
> >>  [] SYSC_getsockopt net/socket.c:1788 [inline]
> >>  [] SyS_getsockopt+0x240/0x380 net/socket.c:1770
> >>  [] do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:280
> >>  [] entry_SYSCALL64_slow_path+0x25/0x25
> >> Code: b0 fe ff ff e8 e1 25 ce ff 48 8b 85 b0 fe ff ff 4d 89 e9 4c 89
> >> e1 4c 89 f2 48 89 de 48 c7 c7 a0 01 50 84 49 89 c0 e8 51 d9 e0 ff <0f>
> >> 0b e8 b8 25 ce ff 4c 89 f2 4c 89 ee 4c 89 e7 e8 6a 1b fc ff
> >> RIP  [] report_usercopy mm/usercopy.c:67 [inline]
> >> RIP  [] __check_object_size+0x2d1/0x9ec mm/usercopy.c:278
> >>  RSP 
> >> ---[ end trace 5e438996b2c0b35d ]---
> >>
> >>
> >> I am not sure why check_object_size flags this an a bug,
> >> copy_from_user copies into a stack object:
> >>
> >> static int sctp_getsockopt_assoc_stats(struct sock *sk, int len,
> >>char __user *optval,
> >>int __user *optlen)
> >> {
> >> struct sctp_assoc_stats sas;
> >> len = min_t(size_t, len, sizeof(sas));
> >> if (copy_from_user(&sas, optval, len))
> >> return -EFAULT;
> >>
> >> K

Re: sctp: kernel memory overwrite attempt detected in sctp_getsockopt_assoc_stats

2017-01-15 Thread Dmitry Vyukov
On Sun, Jan 15, 2017 at 9:35 PM, Neil Horman  wrote:
> On Sun, Jan 15, 2017 at 06:29:59PM +0100, Dmitry Vyukov wrote:
>> Hello,
>>
>> I've enabled CONFIG_HARDENED_USERCOPY_PAGESPAN on syzkaller fuzzer and
>> now I am seeing lots of:
>>
> If I'm not mistaken, its because thats specifically what that option does.  
> From
> the Kconfig:
> onfig HARDENED_USERCOPY_PAGESPAN
> bool "Refuse to copy allocations that span multiple pages"
> depends on HARDENED_USERCOPY
> depends on EXPERT
> help
>   When a multi-page allocation is done without __GFP_COMP,
>   hardened usercopy will reject attempts to copy it. There are,
>   however, several cases of this in the kernel that have not all
>   been removed. This config is intended to be used only while
>   trying to find such users.
>
> So, if the fuzzer does a setsockopt and the data it passes crosses a page
> boundary, it seems like this will get triggered.  Based on the fact that its
> only used to find users that do this, it seems like not the sort of thing that
> you want enabled while running a fuzzer that might do it arbitrarily.


The code also takes into account compound pages. As far as I
understand the intention of the check is to effectively find
out-of-bounds copies (e.g. goes beyond the current heap allocation). I
would expect that stacks are allocated as compound pages and don't
trigger this check. I don't see it is firing in other similar places.



>> usercopy: kernel memory overwrite attempt detected to 8801a74f6f10
>> () (256 bytes)
>>
>> kernel BUG at mm/usercopy.c:75!
>> invalid opcode:  [#1] SMP KASAN
>> Dumping ftrace buffer:
>>(ftrace buffer empty)
>> Modules linked in:
>> CPU: 1 PID: 15686 Comm: syz-executor3 Not tainted 4.9.0 #1
>> Hardware name: Google Google Compute Engine/Google Compute Engine,
>> BIOS Google 01/01/2011
>> task: 8801c89b2500 task.stack: 8801a74f
>> RIP: 0010:[]  [] report_usercopy
>> mm/usercopy.c:67 [inline]
>> RIP: 0010:[]  []
>> __check_object_size+0x2d1/0x9ec mm/usercopy.c:278
>> RSP: 0018:8801a74f6cd0  EFLAGS: 00010286
>> RAX: 006b RBX: 84500120 RCX: 
>> RDX: 006b RSI: 815a7791 RDI: ed0034e9ed8c
>> RBP: 8801a74f6e48 R08: 0001 R09: 
>> R10:  R11: 0001 R12: 8801a74f6f10
>> R13: 0100 R14: 845000e0 R15: 8801a74f700f
>> FS:  7f80918de700() GS:8801dc10() knlGS:
>> CS:  0010 DS:  ES:  CR0: 80050033
>> CR2: 20058ffc CR3: 0001cc1cc000 CR4: 001406e0
>> Stack:
>>  8598fcc8  77ff8000 ea0005c99608
>>  844fff40 844fff40 41b58ab3 84ae0fa0
>>  81a1ad70 8801c89b2500 dead0100 814d4425
>> Call Trace:
>>  [] check_object_size include/linux/thread_info.h:129 
>> [inline]
>>  [] copy_from_user arch/x86/include/asm/uaccess.h:692 
>> [inline]
>>  [] sctp_getsockopt_assoc_stats+0x169/0xa10
>> net/sctp/socket.c:6182
>>  [] sctp_getsockopt+0x1af2/0x66a0 net/sctp/socket.c:6556
>>  [] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2649
>>  [] SYSC_getsockopt net/socket.c:1788 [inline]
>>  [] SyS_getsockopt+0x240/0x380 net/socket.c:1770
>>  [] do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:280
>>  [] entry_SYSCALL64_slow_path+0x25/0x25
>> Code: b0 fe ff ff e8 e1 25 ce ff 48 8b 85 b0 fe ff ff 4d 89 e9 4c 89
>> e1 4c 89 f2 48 89 de 48 c7 c7 a0 01 50 84 49 89 c0 e8 51 d9 e0 ff <0f>
>> 0b e8 b8 25 ce ff 4c 89 f2 4c 89 ee 4c 89 e7 e8 6a 1b fc ff
>> RIP  [] report_usercopy mm/usercopy.c:67 [inline]
>> RIP  [] __check_object_size+0x2d1/0x9ec mm/usercopy.c:278
>>  RSP 
>> ---[ end trace 5e438996b2c0b35d ]---
>>
>>
>> I am not sure why check_object_size flags this an a bug,
>> copy_from_user copies into a stack object:
>>
>> static int sctp_getsockopt_assoc_stats(struct sock *sk, int len,
>>char __user *optval,
>>int __user *optlen)
>> {
>> struct sctp_assoc_stats sas;
>> len = min_t(size_t, len, sizeof(sas));
>> if (copy_from_user(&sas, optval, len))
>> return -EFAULT;
>>
>> Kees, can this be a false positive?
>>
>> On commit f4d3935e4f4884ba80561db5549394afb8eef8f7.
>>


Re: sctp: kernel memory overwrite attempt detected in sctp_getsockopt_assoc_stats

2017-01-15 Thread Neil Horman
On Sun, Jan 15, 2017 at 06:29:59PM +0100, Dmitry Vyukov wrote:
> Hello,
> 
> I've enabled CONFIG_HARDENED_USERCOPY_PAGESPAN on syzkaller fuzzer and
> now I am seeing lots of:
> 
If I'm not mistaken, its because thats specifically what that option does.  From
the Kconfig:
onfig HARDENED_USERCOPY_PAGESPAN
bool "Refuse to copy allocations that span multiple pages"
depends on HARDENED_USERCOPY
depends on EXPERT
help
  When a multi-page allocation is done without __GFP_COMP,
  hardened usercopy will reject attempts to copy it. There are,
  however, several cases of this in the kernel that have not all
  been removed. This config is intended to be used only while
  trying to find such users.

So, if the fuzzer does a setsockopt and the data it passes crosses a page
boundary, it seems like this will get triggered.  Based on the fact that its
only used to find users that do this, it seems like not the sort of thing that
you want enabled while running a fuzzer that might do it arbitrarily.

Regards
Neil


> usercopy: kernel memory overwrite attempt detected to 8801a74f6f10
> () (256 bytes)
> 
> kernel BUG at mm/usercopy.c:75!
> invalid opcode:  [#1] SMP KASAN
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Modules linked in:
> CPU: 1 PID: 15686 Comm: syz-executor3 Not tainted 4.9.0 #1
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS Google 01/01/2011
> task: 8801c89b2500 task.stack: 8801a74f
> RIP: 0010:[]  [] report_usercopy
> mm/usercopy.c:67 [inline]
> RIP: 0010:[]  []
> __check_object_size+0x2d1/0x9ec mm/usercopy.c:278
> RSP: 0018:8801a74f6cd0  EFLAGS: 00010286
> RAX: 006b RBX: 84500120 RCX: 
> RDX: 006b RSI: 815a7791 RDI: ed0034e9ed8c
> RBP: 8801a74f6e48 R08: 0001 R09: 
> R10:  R11: 0001 R12: 8801a74f6f10
> R13: 0100 R14: 845000e0 R15: 8801a74f700f
> FS:  7f80918de700() GS:8801dc10() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 20058ffc CR3: 0001cc1cc000 CR4: 001406e0
> Stack:
>  8598fcc8  77ff8000 ea0005c99608
>  844fff40 844fff40 41b58ab3 84ae0fa0
>  81a1ad70 8801c89b2500 dead0100 814d4425
> Call Trace:
>  [] check_object_size include/linux/thread_info.h:129 
> [inline]
>  [] copy_from_user arch/x86/include/asm/uaccess.h:692 
> [inline]
>  [] sctp_getsockopt_assoc_stats+0x169/0xa10
> net/sctp/socket.c:6182
>  [] sctp_getsockopt+0x1af2/0x66a0 net/sctp/socket.c:6556
>  [] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2649
>  [] SYSC_getsockopt net/socket.c:1788 [inline]
>  [] SyS_getsockopt+0x240/0x380 net/socket.c:1770
>  [] do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:280
>  [] entry_SYSCALL64_slow_path+0x25/0x25
> Code: b0 fe ff ff e8 e1 25 ce ff 48 8b 85 b0 fe ff ff 4d 89 e9 4c 89
> e1 4c 89 f2 48 89 de 48 c7 c7 a0 01 50 84 49 89 c0 e8 51 d9 e0 ff <0f>
> 0b e8 b8 25 ce ff 4c 89 f2 4c 89 ee 4c 89 e7 e8 6a 1b fc ff
> RIP  [] report_usercopy mm/usercopy.c:67 [inline]
> RIP  [] __check_object_size+0x2d1/0x9ec mm/usercopy.c:278
>  RSP 
> ---[ end trace 5e438996b2c0b35d ]---
> 
> 
> I am not sure why check_object_size flags this an a bug,
> copy_from_user copies into a stack object:
> 
> static int sctp_getsockopt_assoc_stats(struct sock *sk, int len,
>char __user *optval,
>int __user *optlen)
> {
> struct sctp_assoc_stats sas;
> len = min_t(size_t, len, sizeof(sas));
> if (copy_from_user(&sas, optval, len))
> return -EFAULT;
> 
> Kees, can this be a false positive?
> 
> On commit f4d3935e4f4884ba80561db5549394afb8eef8f7.
>