WARNING in ex_handler_uaccess

WARNING in ex_handler_uaccess

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    10b82d51 Merge branch 'for-5.9-fixes' of git://git.kernel...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13fb6b07900000
kernel config:  https://syzkaller.appspot.com/x/.config?x=773ee8ece5f19a24
dashboard link: https://syzkaller.appspot.com/bug?extid=ea3a78a71705faf41d77
compiler:       gcc (GCC) 10.1.0-syz 20200507
userspace arch: i386

Unfortunately, I don't have any reproducer for this issue yet.

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+ea3a78a71705faf41d77@syzkaller.appspotmail.com

WARNING: CPU: 2 PID: 6875 at arch/x86/mm/extable.c:77 ex_handler_uaccess+0xba/0xe0 arch/x86/mm/extable.c:77
Kernel panic - not syncing: panic_on_warn set ...
CPU: 2 PID: 6875 Comm: io_uring-sq Not tainted 5.9.0-rc5-syzkaller #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x198/0x1fd lib/dump_stack.c:118
 panic+0x382/0x7fb kernel/panic.c:231
 __warn.cold+0x20/0x4b kernel/panic.c:600
 report_bug+0x1bd/0x210 lib/bug.c:198
 handle_bug+0x38/0x90 arch/x86/kernel/traps.c:234
 exc_invalid_op+0x14/0x40 arch/x86/kernel/traps.c:254
 asm_exc_invalid_op+0x12/0x20 arch/x86/include/asm/idtentry.h:536
RIP: 0010:ex_handler_uaccess+0xba/0xe0 arch/x86/mm/extable.c:77
Code: 1d 5c 30 fa 09 31 ff 89 de e8 e2 ae 40 00 84 db 75 80 e8 b9 b2 40 00 48 c7 c7 e0 1a 69 88 c6 05 3c 30 fa 09 01 e8 a8 ee 10 00 <0f> 0b e9 61 ff ff ff 48 89 df e8 87 7a 81 00 eb 87 e8 10 7b 81 00
RSP: 0018:ffffc9000e03f6c8 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff88801b420400 RSI: ffffffff815f5a85 RDI: fffff52001c07ecb
RBP: ffffc9000e03f7b8 R08: 0000000000000001 R09: ffffffff8ce2daef
R10: 000000000000143b R11: 0000000000000000 R12: ffffffff89b3f410
R13: 000000000000000d R14: 0000000000000000 R15: 0000000000000000
 fixup_exception+0x9a/0xca arch/x86/mm/extable.c:166
 __exc_general_protection arch/x86/kernel/traps.c:557 [inline]
 exc_general_protection+0xeb/0x2e0 arch/x86/kernel/traps.c:524
 asm_exc_general_protection+0x1e/0x30 arch/x86/include/asm/idtentry.h:532
RIP: 0010:check_zeroed_user+0xd7/0x260 lib/usercopy.c:65
Code: ab 00 00 00 e8 6a 61 d4 fd 48 89 ee 48 89 df e8 5f 5d d4 fd 48 39 eb 0f 87 92 00 00 00 e8 51 61 d4 fd 0f 01 cb 0f ae e8 31 ed <49> 8b 1e 31 ff 89 ee e8 9d 5d d4 fd 85 ed 75 6d e8 34 61 d4 fd 31
RSP: 0018:ffffc9000e03f860 EFLAGS: 00050246
RAX: 0000000000000000 RBX: 2000024020012545 RCX: ffffffff83a1de61
RDX: ffff88801b420400 RSI: ffffffff83a1de6f RDI: 0000000000000006
RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffff8b34a68f
R10: 2000024020012545 R11: 0000000000000000 R12: 0000000000000000
R13: 000000000001232d R14: 2000024020000218 R15: 0000000000000000
 copy_struct_from_user include/linux/uaccess.h:310 [inline]
 io_openat2_prep fs/io_uring.c:3560 [inline]
 io_openat2_prep+0x142/0x1a0 fs/io_uring.c:3547
 io_issue_sqe+0x1932/0x61e0 fs/io_uring.c:5850
 __io_queue_sqe+0x280/0x1160 fs/io_uring.c:6150
 io_queue_sqe+0x692/0xfa0 fs/io_uring.c:6229
 io_submit_sqe fs/io_uring.c:6299 [inline]
 io_submit_sqes+0x1761/0x2400 fs/io_uring.c:6496
 io_sq_thread+0x3ac/0xe00 fs/io_uring.c:6633
 kthread+0x3b5/0x4a0 kernel/kthread.c:292
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

Re: WARNING in ex_handler_uaccess

[Andy Lutomirski]

On Fri, Sep 18, 2020 at 2:01 PM syzbot
<syzbot+ea3a78a71705faf41d77@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:    10b82d51 Merge branch 'for-5.9-fixes' of git://git.kernel...
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=13fb6b07900000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=773ee8ece5f19a24
> dashboard link: https://syzkaller.appspot.com/bug?extid=ea3a78a71705faf41d77
> compiler:       gcc (GCC) 10.1.0-syz 20200507
> userspace arch: i386
>
> Unfortunately, I don't have any reproducer for this issue yet.
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+ea3a78a71705faf41d77@syzkaller.appspotmail.com
>
> WARNING: CPU: 2 PID: 6875 at arch/x86/mm/extable.c:77 ex_handler_uaccess+0xba/0xe0 arch/x86/mm/extable.c:77
> Kernel panic - not syncing: panic_on_warn set ...
> CPU: 2 PID: 6875 Comm: io_uring-sq Not tainted 5.9.0-rc5-syzkaller #0
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x198/0x1fd lib/dump_stack.c:118
>  panic+0x382/0x7fb kernel/panic.c:231
>  __warn.cold+0x20/0x4b kernel/panic.c:600
>  report_bug+0x1bd/0x210 lib/bug.c:198
>  handle_bug+0x38/0x90 arch/x86/kernel/traps.c:234
>  exc_invalid_op+0x14/0x40 arch/x86/kernel/traps.c:254
>  asm_exc_invalid_op+0x12/0x20 arch/x86/include/asm/idtentry.h:536
> RIP: 0010:ex_handler_uaccess+0xba/0xe0 arch/x86/mm/extable.c:77
> Code: 1d 5c 30 fa 09 31 ff 89 de e8 e2 ae 40 00 84 db 75 80 e8 b9 b2 40 00 48 c7 c7 e0 1a 69 88 c6 05 3c 30 fa 09 01 e8 a8 ee 10 00 <0f> 0b e9 61 ff ff ff 48 89 df e8 87 7a 81 00 eb 87 e8 10 7b 81 00
> RSP: 0018:ffffc9000e03f6c8 EFLAGS: 00010282
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: ffff88801b420400 RSI: ffffffff815f5a85 RDI: fffff52001c07ecb
> RBP: ffffc9000e03f7b8 R08: 0000000000000001 R09: ffffffff8ce2daef
> R10: 000000000000143b R11: 0000000000000000 R12: ffffffff89b3f410
> R13: 000000000000000d R14: 0000000000000000 R15: 0000000000000000
>  fixup_exception+0x9a/0xca arch/x86/mm/extable.c:166
>  __exc_general_protection arch/x86/kernel/traps.c:557 [inline]
>  exc_general_protection+0xeb/0x2e0 arch/x86/kernel/traps.c:524
>  asm_exc_general_protection+0x1e/0x30 arch/x86/include/asm/idtentry.h:532
> RIP: 0010:check_zeroed_user+0xd7/0x260 lib/usercopy.c:65
> Code: ab 00 00 00 e8 6a 61 d4 fd 48 89 ee 48 89 df e8 5f 5d d4 fd 48 39 eb 0f 87 92 00 00 00 e8 51 61 d4 fd 0f 01 cb 0f ae e8 31 ed <49> 8b 1e 31 ff 89 ee e8 9d 5d d4 fd 85 ed 75 6d e8 34 61 d4 fd 31
> RSP: 0018:ffffc9000e03f860 EFLAGS: 00050246
> RAX: 0000000000000000 RBX: 2000024020012545 RCX: ffffffff83a1de61
> RDX: ffff88801b420400 RSI: ffffffff83a1de6f RDI: 0000000000000006
> RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffff8b34a68f
> R10: 2000024020012545 R11: 0000000000000000 R12: 0000000000000000
> R13: 000000000001232d R14: 2000024020000218 R15: 0000000000000000
>  copy_struct_from_user include/linux/uaccess.h:310 [inline]
>  io_openat2_prep fs/io_uring.c:3560 [inline]

Hi Aleksa-

check_zeroed_user() looks buggy.  It does:

       if (!user_access_begin(from, size))
               return -EFAULT;

       unsafe_get_user(val, (unsigned long __user *) from, err_fault);

This is wrong if size < sizeof(unsigned long) -- you read outside the
area you verified using user_access_begin().  The code down below in
the loop appears to have the same bug.

I'm not sure what the right fix is.  Even changing the
user_access_begin() isn't going to fix it, because you have a second
bug: this over-read can also get -EFAULT and fail a perfectly valid
syscall if the zeroed region is unaligned and butts up to the end of a
vma.  For example, if you do a syscall that has a checked zeroed
region that's exactly one byte long at the last byte of a page, you'll
-EFAULT.  Whoops.

I don't *think* this is a major security bug, but it does give
malicious userspace a trivial way to generate WARN messages on x86_64.
(Although I admit I'm a wee bit confused as to how this actually
triggered the #GP warning -- maybe the address check is a bit looser
than it could be.  We don't actually allow tasks the allocate the
topmost lower-half canonical address.)

Re: WARNING in ex_handler_uaccess

[Al Viro]

On Fri, Sep 18, 2020 at 04:31:33PM -0700, Andy Lutomirski wrote:

> check_zeroed_user() looks buggy.  It does:
> 
>        if (!user_access_begin(from, size))
>                return -EFAULT;
> 
>        unsafe_get_user(val, (unsigned long __user *) from, err_fault);
> 
> This is wrong if size < sizeof(unsigned long) -- you read outside the
> area you verified using user_access_begin().

Read the code immediately prior to that.  from will be word-aligned,
and size will be extended accordingly.  If the area acceptable for
user_access_begin() ends *NOT* on a word boundary, you have a problem
and I would strongly recommend to seek professional help.

All reads in that thing are word-aligned and word-sized.  So I very
much doubt that your analysis is correct.

Re: WARNING in ex_handler_uaccess

[Andy Lutomirski]

On Fri, Sep 18, 2020 at 4:55 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Fri, Sep 18, 2020 at 04:31:33PM -0700, Andy Lutomirski wrote:
>
> > check_zeroed_user() looks buggy.  It does:
> >
> >        if (!user_access_begin(from, size))
> >                return -EFAULT;
> >
> >        unsafe_get_user(val, (unsigned long __user *) from, err_fault);
> >
> > This is wrong if size < sizeof(unsigned long) -- you read outside the
> > area you verified using user_access_begin().
>
> Read the code immediately prior to that.  from will be word-aligned,
> and size will be extended accordingly.  If the area acceptable for
> user_access_begin() ends *NOT* on a word boundary, you have a problem
> and I would strongly recommend to seek professional help.
>
> All reads in that thing are word-aligned and word-sized.  So I very
> much doubt that your analysis is correct.

Maybe -ETOOTIRED, but I seriously question the math in here.  Suppose
from == (unsigned long *)1 and size == 1.  Then align is 1, and we do:

from -= align;
size += align;

So now from = 0 and size = 2.  Now we do user_access_begin(0, 2) and
then immediately read 4 or 8 bytes.  No good.


--Andy

Re: WARNING in ex_handler_uaccess

[Al Viro]

On Fri, Sep 18, 2020 at 05:07:43PM -0700, Andy Lutomirski wrote:
> On Fri, Sep 18, 2020 at 4:55 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Fri, Sep 18, 2020 at 04:31:33PM -0700, Andy Lutomirski wrote:
> >
> > > check_zeroed_user() looks buggy.  It does:
> > >
> > >        if (!user_access_begin(from, size))
> > >                return -EFAULT;
> > >
> > >        unsafe_get_user(val, (unsigned long __user *) from, err_fault);
> > >
> > > This is wrong if size < sizeof(unsigned long) -- you read outside the
> > > area you verified using user_access_begin().
> >
> > Read the code immediately prior to that.  from will be word-aligned,
> > and size will be extended accordingly.  If the area acceptable for
> > user_access_begin() ends *NOT* on a word boundary, you have a problem
> > and I would strongly recommend to seek professional help.
> >
> > All reads in that thing are word-aligned and word-sized.  So I very
> > much doubt that your analysis is correct.
> 
> Maybe -ETOOTIRED, but I seriously question the math in here.  Suppose
> from == (unsigned long *)1 and size == 1.  Then align is 1, and we do:
> 
> from -= align;
> size += align;
> 
> So now from = 0 and size = 2.  Now we do user_access_begin(0, 2) and
> then immediately read 4 or 8 bytes.  No good.

Could you explain what kind of insane hardware manages to do #PF-related
checks (including SMAP, whatever) with *sub*WORD* granularity?

If it's OK with 16bit read from word-aligned address, but barfs on 64bit
one...  I want to know what the hell had its authors been smoking.

RE: WARNING in ex_handler_uaccess

[David Laight]

From: Al Viro
> Sent: 19 September 2020 01:17
> 
> On Fri, Sep 18, 2020 at 05:07:43PM -0700, Andy Lutomirski wrote:
> > On Fri, Sep 18, 2020 at 4:55 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > >
> > > On Fri, Sep 18, 2020 at 04:31:33PM -0700, Andy Lutomirski wrote:
> > >
> > > > check_zeroed_user() looks buggy.  It does:
> > > >
> > > >        if (!user_access_begin(from, size))
> > > >                return -EFAULT;
> > > >
> > > >        unsafe_get_user(val, (unsigned long __user *) from, err_fault);
> > > >
> > > > This is wrong if size < sizeof(unsigned long) -- you read outside the
> > > > area you verified using user_access_begin().
> > >
> > > Read the code immediately prior to that.  from will be word-aligned,
> > > and size will be extended accordingly.  If the area acceptable for
> > > user_access_begin() ends *NOT* on a word boundary, you have a problem
> > > and I would strongly recommend to seek professional help.
> > >
> > > All reads in that thing are word-aligned and word-sized.  So I very
> > > much doubt that your analysis is correct.
> >
> > Maybe -ETOOTIRED, but I seriously question the math in here.  Suppose
> > from == (unsigned long *)1 and size == 1.  Then align is 1, and we do:
> >
> > from -= align;
> > size += align;
> >
> > So now from = 0 and size = 2.  Now we do user_access_begin(0, 2) and
> > then immediately read 4 or 8 bytes.  No good.
> 
> Could you explain what kind of insane hardware manages to do #PF-related
> checks (including SMAP, whatever) with *sub*WORD* granularity?
> 
> If it's OK with 16bit read from word-aligned address, but barfs on 64bit
> one...  I want to know what the hell had its authors been smoking.

Not going to happen to anything in the data cache. But...

Byte parity errors on memory locations that have never been written
but power-up initialised to 'bad parity'?

I have seen that - but I've completely forgotten the hardware.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: WARNING in ex_handler_uaccess

[Rasmus Villemoes]

On 19/09/2020 02.17, Al Viro wrote:
> On Fri, Sep 18, 2020 at 05:07:43PM -0700, Andy Lutomirski wrote:
>> On Fri, Sep 18, 2020 at 4:55 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>>>
>>> On Fri, Sep 18, 2020 at 04:31:33PM -0700, Andy Lutomirski wrote:
>>>
>>>> check_zeroed_user() looks buggy.  It does:
>>>>
>>>>        if (!user_access_begin(from, size))
>>>>                return -EFAULT;
>>>>
>>>>        unsafe_get_user(val, (unsigned long __user *) from, err_fault);
>>>>
>>>> This is wrong if size < sizeof(unsigned long) -- you read outside the
>>>> area you verified using user_access_begin().
>>>
>>> Read the code immediately prior to that.  from will be word-aligned,
>>> and size will be extended accordingly.  If the area acceptable for
>>> user_access_begin() ends *NOT* on a word boundary, you have a problem
>>> and I would strongly recommend to seek professional help.
>>>
>>> All reads in that thing are word-aligned and word-sized.  So I very
>>> much doubt that your analysis is correct.
>>
>> Maybe -ETOOTIRED, but I seriously question the math in here.  Suppose
>> from == (unsigned long *)1 and size == 1.  Then align is 1, and we do:
>>
>> from -= align;
>> size += align;
>>
>> So now from = 0 and size = 2.  Now we do user_access_begin(0, 2) and
>> then immediately read 4 or 8 bytes.  No good.
> 
> Could you explain what kind of insane hardware manages to do #PF-related
> checks (including SMAP, whatever) with *sub*WORD* granularity?
> 
> If it's OK with 16bit read from word-aligned address, but barfs on 64bit
> one...  I want to know what the hell had its authors been smoking.
> 

So, not sure how the above got triggered, but I notice there might be an
edge case in check_zeroed_user():

	from -= align;
	size += align;

	if (!user_read_access_begin(from, size))
		return -EFAULT;

	unsafe_get_user(val, (unsigned long __user *) from, err_fault);


Suppose size is (size_t)-3 and align is 3. What's the convention for
access_ok(whatever, 0)? Is that equivalent to access_ok(whatever, 1), or
is it always true (or $ARCH-dependent)?

But, AFAICT, no current caller of check_zeroed_user can end up passing
in a size that can overflow to 0. E.g. for the case at hand, size cannot
be more than SIZE_MAX-24.

Rasmus

RE: WARNING in ex_handler_uaccess

[David Laight]

From: Rasmus Villemoes
> Sent: 21 September 2020 11:22
 
> On 19/09/2020 02.17, Al Viro wrote:
> > On Fri, Sep 18, 2020 at 05:07:43PM -0700, Andy Lutomirski wrote:
> >> On Fri, Sep 18, 2020 at 4:55 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >>>
> >>> On Fri, Sep 18, 2020 at 04:31:33PM -0700, Andy Lutomirski wrote:
> >>>
> >>>> check_zeroed_user() looks buggy.  It does:
> >>>>
> >>>>        if (!user_access_begin(from, size))
> >>>>                return -EFAULT;
> >>>>
> >>>>        unsafe_get_user(val, (unsigned long __user *) from, err_fault);
> >>>>
> >>>> This is wrong if size < sizeof(unsigned long) -- you read outside the
> >>>> area you verified using user_access_begin().
> >>>
> >>> Read the code immediately prior to that.  from will be word-aligned,
> >>> and size will be extended accordingly.  If the area acceptable for
> >>> user_access_begin() ends *NOT* on a word boundary, you have a problem
> >>> and I would strongly recommend to seek professional help.
> >>>
> >>> All reads in that thing are word-aligned and word-sized.  So I very
> >>> much doubt that your analysis is correct.
> >>
> >> Maybe -ETOOTIRED, but I seriously question the math in here.  Suppose
> >> from == (unsigned long *)1 and size == 1.  Then align is 1, and we do:
> >>
> >> from -= align;
> >> size += align;
> >>
> >> So now from = 0 and size = 2.  Now we do user_access_begin(0, 2) and
> >> then immediately read 4 or 8 bytes.  No good.
> >
> > Could you explain what kind of insane hardware manages to do #PF-related
> > checks (including SMAP, whatever) with *sub*WORD* granularity?
> >
> > If it's OK with 16bit read from word-aligned address, but barfs on 64bit
> > one...  I want to know what the hell had its authors been smoking.
> >
> 
> So, not sure how the above got triggered, but I notice there might be an
> edge case in check_zeroed_user():
> 
> 	from -= align;
> 	size += align;
> 
> 	if (!user_read_access_begin(from, size))
> 		return -EFAULT;
> 
> 	unsafe_get_user(val, (unsigned long __user *) from, err_fault);
> 
> 
> Suppose size is (size_t)-3 and align is 3. What's the convention for
> access_ok(whatever, 0)? Is that equivalent to access_ok(whatever, 1), or
> is it always true (or $ARCH-dependent)?

Doesn't matter, it will be doing access_ok(xxx, 8) regardless of
the user-supplied transfer length.

> But, AFAICT, no current caller of check_zeroed_user can end up passing
> in a size that can overflow to 0. E.g. for the case at hand, size cannot
> be more than SIZE_MAX-24.

Basically KASAN doesn't like you reading full words and masking
off the unused bytes.

	David

	

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: WARNING in ex_handler_uaccess

[Al Viro]

On Mon, Sep 21, 2020 at 12:22:19PM +0200, Rasmus Villemoes wrote:

> So, not sure how the above got triggered, but I notice there might be an
> edge case in check_zeroed_user():
> 
> 	from -= align;
> 	size += align;
> 
> 	if (!user_read_access_begin(from, size))
> 		return -EFAULT;
> 
> 	unsafe_get_user(val, (unsigned long __user *) from, err_fault);
> 
> 
> Suppose size is (size_t)-3 and align is 3. What's the convention for
> access_ok(whatever, 0)? Is that equivalent to access_ok(whatever, 1), or
> is it always true (or $ARCH-dependent)?

It's usually true...

> But, AFAICT, no current caller of check_zeroed_user can end up passing
> in a size that can overflow to 0. E.g. for the case at hand, size cannot
> be more than SIZE_MAX-24.

Might be worth slapping if (unlikely(!size)) return -EFAULT; // overflow
just before user_read_access_begin() to be sure...