KASAN: slab-out-of-bounds Read in handle_vmptrld

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit: 1e3778cb Merge tag 'scsi-fixes' of git://git.kernel.org/pu..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15bdfc5e600000
kernel config: https://syzkaller.appspot.com/x/.config?x=b89bb446a3faaba4
dashboard link: https://syzkaller.appspot.com/bug?extid=46f1dd7dbbe2bfb98b10
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1709421a600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=168fc4b2600000

The bug was bisected to:

commit a87f854ddcf7ff7e044d72db0aa6da82f26d69a6
Author: Neil Armstrong <narms...@baylibre.com>
Date: Wed Oct 11 15:39:40 2017 +0000

ARM64: dts: meson-gx: remove unnecessary uart compatible

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=17e78a6e600000
final crash: https://syzkaller.appspot.com/x/report.txt?x=14178a6e600000
console output: https://syzkaller.appspot.com/x/log.txt?x=10178a6e600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+46f1dd...@syzkaller.appspotmail.com
Fixes: a87f854ddcf7 ("ARM64: dts: meson-gx: remove unnecessary uart
compatible")

L1TF CPU bug present and SMT on, data leak possible. See CVE-2018-3646 and
https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/l1tf.html for
details.
==================================================================
BUG: KASAN: slab-out-of-bounds in handle_vmptrld
arch/x86/kvm/vmx/nested.c:4789 [inline]
BUG: KASAN: slab-out-of-bounds in handle_vmptrld+0x777/0x800
arch/x86/kvm/vmx/nested.c:4749
Read of size 4 at addr ffff888091e10000 by task syz-executor758/10006

CPU: 1 PID: 10006 Comm: syz-executor758 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
print_address_description.cold+0xd4/0x306 mm/kasan/report.c:351
__kasan_report.cold+0x1b/0x36 mm/kasan/report.c:482
kasan_report+0x12/0x17 mm/kasan/common.c:618
__asan_report_load_n_noabort+0xf/0x20 mm/kasan/generic_report.c:142
handle_vmptrld arch/x86/kvm/vmx/nested.c:4789 [inline]
handle_vmptrld+0x777/0x800 arch/x86/kvm/vmx/nested.c:4749
vmx_handle_exit+0x299/0x15e0 arch/x86/kvm/vmx/vmx.c:5886
vcpu_enter_guest+0x1087/0x5e90 arch/x86/kvm/x86.c:8088
vcpu_run arch/x86/kvm/x86.c:8152 [inline]
kvm_arch_vcpu_ioctl_run+0x464/0x1750 arch/x86/kvm/x86.c:8360
kvm_vcpu_ioctl+0x4dc/0xfd0 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2765
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:509 [inline]
do_vfs_ioctl+0xdb6/0x13e0 fs/ioctl.c:696
ksys_ioctl+0xab/0xd0 fs/ioctl.c:713
__do_sys_ioctl fs/ioctl.c:720 [inline]
__se_sys_ioctl fs/ioctl.c:718 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x447269
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 3b d0 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffd58df6ad8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007ffd58df6ae0 RCX: 0000000000447269
RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 0000000000000005
RBP: 0000000000000000 R08: 0000000020003800 R09: 0000000000400e80
R10: 00007ffd58df4f20 R11: 0000000000000246 R12: 0000000000404730
R13: 00000000004047c0 R14: 0000000000000000 R15: 0000000000000000

Allocated by task 10006:
save_stack+0x23/0x90 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_kmalloc mm/kasan/common.c:493 [inline]
__kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:466
kasan_kmalloc+0x9/0x10 mm/kasan/common.c:507
__do_kmalloc mm/slab.c:3655 [inline]
__kmalloc+0x163/0x770 mm/slab.c:3664
kmalloc include/linux/slab.h:557 [inline]
hcd_buffer_alloc+0x1c6/0x260 drivers/usb/core/buffer.c:132
usb_alloc_coherent+0x62/0x90 drivers/usb/core/usb.c:910
usbdev_mmap+0x1ce/0x790 drivers/usb/core/devio.c:224
call_mmap include/linux/fs.h:1875 [inline]
mmap_region+0xc35/0x1760 mm/mmap.c:1788
do_mmap+0x82e/0x1090 mm/mmap.c:1561
do_mmap_pgoff include/linux/mm.h:2374 [inline]
vm_mmap_pgoff+0x1c5/0x230 mm/util.c:391
ksys_mmap_pgoff+0x4aa/0x630 mm/mmap.c:1611
__do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
__se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
__x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 9516:
save_stack+0x23/0x90 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/common.c:455
kasan_slab_free+0xe/0x10 mm/kasan/common.c:463
__cache_free mm/slab.c:3425 [inline]
kfree+0x10a/0x2c0 mm/slab.c:3756
tomoyo_init_log+0x15ba/0x2070 security/tomoyo/audit.c:293
tomoyo_supervisor+0x33f/0xef0 security/tomoyo/common.c:2095
tomoyo_audit_env_log security/tomoyo/environ.c:36 [inline]
tomoyo_env_perm+0x18e/0x210 security/tomoyo/environ.c:63
tomoyo_environ security/tomoyo/domain.c:670 [inline]
tomoyo_find_next_domain+0x1354/0x1f6c security/tomoyo/domain.c:876
tomoyo_bprm_check_security security/tomoyo/tomoyo.c:107 [inline]
tomoyo_bprm_check_security+0x124/0x1b0 security/tomoyo/tomoyo.c:97
security_bprm_check+0x63/0xb0 security/security.c:750
search_binary_handler+0x71/0x570 fs/exec.c:1645
exec_binprm fs/exec.c:1701 [inline]
__do_execve_file.isra.0+0x1333/0x2340 fs/exec.c:1821
do_execveat_common fs/exec.c:1868 [inline]
do_execve fs/exec.c:1885 [inline]
__do_sys_execve fs/exec.c:1961 [inline]
__se_sys_execve fs/exec.c:1956 [inline]
__x64_sys_execve+0x8f/0xc0 fs/exec.c:1956
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff888091e109c0
which belongs to the cache kmalloc-8k of size 8192
The buggy address is located 2496 bytes to the left of
8192-byte region [ffff888091e109c0, ffff888091e129c0)
The buggy address belongs to the page:
page:ffffea0002478400 refcount:2 mapcount:0 mapping:ffff8880aa4021c0
index:0x0 compound_mapcount: 0
flags: 0x1fffc0000010200(slab|head)
raw: 01fffc0000010200 ffffea000242e608 ffffea0002436708 ffff8880aa4021c0
raw: 0000000000000000 ffff888091e109c0 0000000200000001 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff888091e0ff00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888091e0ff80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff888091e10000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff888091e10080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888091e10100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================


---
This bug 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 syzk...@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

[Will Deacon]

On Wed, Sep 11, 2019 at 01:38:08PM -0700, syzbot wrote:
> syzbot found the following crash on:
>
> HEAD commit: 1e3778cb Merge tag 'scsi-fixes' of git://git.kernel.org/pu..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=15bdfc5e600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=b89bb446a3faaba4
> dashboard link: https://syzkaller.appspot.com/bug?extid=46f1dd7dbbe2bfb98b10
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1709421a600000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=168fc4b2600000
>
> The bug was bisected to:
>
> commit a87f854ddcf7ff7e044d72db0aa6da82f26d69a6
> Author: Neil Armstrong <narms...@baylibre.com>
> Date: Wed Oct 11 15:39:40 2017 +0000
>
> ARM64: dts: meson-gx: remove unnecessary uart compatible
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=17e78a6e600000
> final crash: https://syzkaller.appspot.com/x/report.txt?x=14178a6e600000
> console output: https://syzkaller.appspot.com/x/log.txt?x=10178a6e600000

Unfortunately, I think the bisect must be bogus, since I can't see how a
devicetree change for an arm64 file can affect the x86 KVM instruction
emulation.

Maybe somebody from the x86 KVM side could have a look at the KASAN splat?

Will

> _______________________________________________
> linux-arm-kernel mailing list
> linux-ar...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[Vitaly Kuznetsov]

Hm, the bisection seems bogus but the stack points us to the following
piece of code:

4776) if (kvm_vcpu_map(vcpu, gpa_to_gfn(vmptr), &map)) {
<skip>
4783) return nested_vmx_failValid(vcpu,
4784) VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID);
4785) }
4786)
4787) new_vmcs12 = map.hva;
4788)
*4789) if (new_vmcs12->hdr.revision_id != VMCS12_REVISION ||
4790) (new_vmcs12->hdr.shadow_vmcs &&
4791) !nested_cpu_has_vmx_shadow_vmcs(vcpu))) {

the reported problem seems to be on VMCS12 region access but it's part
of guest memory and we successfuly managed to map it. We're definitely
within 1-page range. Maybe KASAN is just wrong here?

--
Vitaly

[Paolo Bonzini]

[tl;dr: there could be a /dev/usb bug only affecting KASAN
configurations, jump to the end to skip the analysis and get to the bug
details]

On 12/09/19 15:54, Vitaly Kuznetsov wrote:
> Hm, the bisection seems bogus but the stack points us to the following
> piece of code:
>
> 4776) if (kvm_vcpu_map(vcpu, gpa_to_gfn(vmptr), &map)) {
> <skip>
> 4783) return nested_vmx_failValid(vcpu,
> 4784) VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID);
> 4785) }
> 4786)
> 4787) new_vmcs12 = map.hva;
> 4788)
> *4789) if (new_vmcs12->hdr.revision_id != VMCS12_REVISION ||
> 4790) (new_vmcs12->hdr.shadow_vmcs &&
> 4791) !nested_cpu_has_vmx_shadow_vmcs(vcpu))) {
>
> the reported problem seems to be on VMCS12 region access but it's part
> of guest memory and we successfuly managed to map it. We're definitely
> within 1-page range. Maybe KASAN is just wrong here?

Here is the relevant part of the syzkaller repro:

syz_kvm_setup_cpu$x86(r1, 0xffffffffffffffff,
&(0x7f0000000000/0x18000)=nil, 0x0, 0x133, 0x0, 0x0, 0xff7d)
r3 = syz_open_dev$usb(&(0x7f0000000080)='/dev/bus/usb/00#/00#\x00',
0x40000fffffd, 0x200800000000042)
mmap$IORING_OFF_SQES(&(0x7f0000007000/0x2000)=nil, 0x2000, 0x4, 0x13,
r3, 0x10000000)
syz_kvm_setup_cpu$x86(0xffffffffffffffff, r2,
&(0x7f0000000000/0x18000)=nil, 0x0, 0xfefd, 0x40, 0x0, 0xfffffffffffffdd4)
ioctl$KVM_RUN(r2, 0xae80, 0x0)

The mmap$IORING_OFF_SQES is just a normal mmap from a device, which
replaces the previous mapping for guest memory and in particular
0x7f0000007000 which is the VMCS (from the C reproducer: "#define
ADDR_VAR_VMCS 0x7000").

The previous mapping is freed with do_munmap and then repopulated in
usbdev_mmap with remap_pfn_range. In KVM this means that kvm_vcpu_map
goes through hva_to_pfn_remapped, which correctly calls get_page via
kvm_get_pfn. (Note that although drivers/usb/core/devio.c's usbdev_mmap
sets VM_IO *after* calling remap_pfn_range, remap_pfn_range itself
helpfully sets it before calling remap_p4d_range. And anyway KVM is
looking at vma->vm_flags under mmap_sem, which is held during mmap).

So, KVM should be doing the right thing. Now, the error is:

> Read of size 4 at addr ffff888091e10000 by task syz-executor758/10006

> The buggy address belongs to the object at ffff888091e109c0

> The buggy address is located 2496 bytes to the left of
> 8192-byte region [ffff888091e109c0, ffff888091e129c0)

And given the use of remap_pfn_range in devusb_mmap, the simplest
explanation could be that USB expects kmalloc-8k to return 8k-aligned
values, but this is not true anymore with KASAN. CCing Dmitry, Greg and
linux-usb.

Paolo

[Greg Kroah-Hartman]

USB drivers expect kmalloc to return DMA-able memory. I don't know
about specific alignment issues, that should only an issue for the host
controller being used here, which you do not say in the above list.

We have had some reports that usbdev_mmap() does not do the "correct
thing" for all host controllers, but a lot of the DMA work that is in
linux-next for 5.4-rc1 should have helped resolve those issues. What
tree are you seeing these bug reports happening from?

thanks,

greg k-h

[Paolo Bonzini]

On 13/09/19 06:46, Greg Kroah-Hartman wrote:
> USB drivers expect kmalloc to return DMA-able memory. I don't know
> about specific alignment issues, that should only an issue for the host
> controller being used here, which you do not say in the above list.

I have no idea, this is just the analysis of a syzkaller report. From
the backtrace, it's one that ends up calling kmalloc; all of them should
have the same issue with KASAN.

The specific alignment requirement for this bug comes from this call in
usbdev_mmap:

if (remap_pfn_range(vma, vma->vm_start,
virt_to_phys(usbm->mem) >> PAGE_SHIFT,
size, vma->vm_page_prot) < 0) {

> We have had some reports that usbdev_mmap() does not do the "correct
> thing" for all host controllers, but a lot of the DMA work that is in
> linux-next for 5.4-rc1 should have helped resolve those issues. What
> tree are you seeing these bug reports happening from?

It's in master, but the relevant code is the same in linux-next; in fact
in this case there is no DMA involved at all. hcd_buffer_alloc hits
the case "some USB hosts just use PIO".

On those host controllers, it should be reproducible with just this:

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 7fcb9f782931..cc0460730bce 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -905,9 +905,12 @@ EXPORT_SYMBOL_GPL(__usb_get_extra_descriptor);
void *usb_alloc_coherent(struct usb_device *dev, size_t size, gfp_t mem_flags,
dma_addr_t *dma)
{
+ void *buf;
if (!dev || !dev->bus)
return NULL;
- return hcd_buffer_alloc(dev->bus, size, mem_flags, dma);
+ buf = hcd_buffer_alloc(dev->bus, size, mem_flags, dma);
+ WARN_ON_ONCE(virt_to_phys(buf) & ~PAGE_MASK);
+ return buf;
}
EXPORT_SYMBOL_GPL(usb_alloc_coherent);


and CONFIG_KASAN=y or possibly just CONFIG_DEBUG_SLAB=y. mmap-ing /dev/usb
should warn if my analysis is correct.

If you think the above patch makes sense, I can test it and submit it formally.

Paolo

[Greg Kroah-Hartman]

Look at linux-next, we "should" have fixed up hcd_buffer_alloc() now to
not need this type of thing. If we got it wrong, please let us know and
then yes, a fix like this would be most appreciated :)

thanks,

greg k-h

[Paolo Bonzini]

On 13/09/19 15:02, Greg Kroah-Hartman wrote:
> Look at linux-next, we "should" have fixed up hcd_buffer_alloc() now to
> not need this type of thing. If we got it wrong, please let us know and
> then yes, a fix like this would be most appreciated :)

I still see

/* some USB hosts just use PIO */
if (!hcd_uses_dma(hcd)) {
*dma = ~(dma_addr_t) 0;
return kmalloc(size, mem_flags);
}

in linux-next's hcd_buffer_alloc and also in usb.git's usb-next branch.
I also see the same

if (remap_pfn_range(vma, vma->vm_start,
virt_to_phys(usbm->mem) >> PAGE_SHIFT,
size, vma->vm_page_prot) < 0) {

...
}

in usbdev_mmap. Of course it's possible that I'm looking at the wrong
branch, or just being dense.

Paolo

[Robin Murphy]

Oh, that bit of usbdev_mmap() is already known to be pretty much totally
bogus for various reasons - there have been a few threads about it, of
which I think [1] is both the most recent and the most informative.
There was another patch[2], but that might have stalled (and might need
reworking with additional hcd_uses_dma() checks anyway).

Robin.

[1]
https://lore.kernel.org/linux-arm-kernel/20190808084...@priv-mua.localdomain/
[2]
https://lore.kernel.org/linux-usb/20190801220134...@thegavinli.com/

[Alan Stern]

Have you seen

https://marc.info/?l=linux-usb&m=156758511218419&w=2

? It certainly is relevant, although Greg hasn't replied to it.

There have been other messages on the mailing list about this issue,
but I haven't tried to keep track of them.

Also, just warning about a non-page-aligned allocation doesn't really
help. It would be better to fix the misbehaving allocator.

Alan Stern

[Paolo Bonzini]

On 13/09/19 17:36, Alan Stern wrote:
> On Fri, 13 Sep 2019, Paolo Bonzini wrote:
>
>> On 13/09/19 15:02, Greg Kroah-Hartman wrote:
>>> Look at linux-next, we "should" have fixed up hcd_buffer_alloc() now to
>>> not need this type of thing. If we got it wrong, please let us know and
>>> then yes, a fix like this would be most appreciated :)
>>
>> I still see
>>
>> /* some USB hosts just use PIO */
>> if (!hcd_uses_dma(hcd)) {
>> *dma = ~(dma_addr_t) 0;
>> return kmalloc(size, mem_flags);
>> }
>>
>> in linux-next's hcd_buffer_alloc and also in usb.git's usb-next branch.
>> I also see the same
>>
>> if (remap_pfn_range(vma, vma->vm_start,
>> virt_to_phys(usbm->mem) >> PAGE_SHIFT,
>> size, vma->vm_page_prot) < 0) {
>> ...
>> }
>>
>> in usbdev_mmap. Of course it's possible that I'm looking at the wrong
>> branch, or just being dense.
>
> Have you seen
>
> https://marc.info/?l=linux-usb&m=156758511218419&w=2
>
> ? It certainly is relevant, although Greg hasn't replied to it.

It helps but it's not a full fix, since the address would fail
is_vmalloc_addr. On top of that, hcd_buffer_alloc and hcd_buffer_free
need to switch from kmalloc to vmalloc.

> Also, just warning about a non-page-aligned allocation doesn't really
> help. It would be better to fix the misbehaving allocator.

Of course. The above patch does not fix the issue, it should just allow
for an easier reproduction not involving KVM. More long term, it points
out where the contracts mismatch (i.e. between hcd_buffer_alloc and
usb_alloc_coherent), and more selfishly whose bug it is when syzkaller
complains. :)

Paolo

[Paolo Bonzini]

On 13/09/19 17:32, Robin Murphy wrote:
> Oh, that bit of usbdev_mmap() is already known to be pretty much totally
> bogus for various reasons - there have been a few threads about it, of
> which I think [1] is both the most recent and the most informative.
> There was another patch[2], but that might have stalled (and might need
> reworking with additional hcd_uses_dma() checks anyway).

Neither is enough, see my reply to Alan. Memory from kmalloc just
*cannot* be passed down to remap_pfn_range, dma_mmap_coherent or
anything like that. It's a simple alignment issue.

Paolo

[Paolo Bonzini]

Fixed now by commit 59bb47985c1d ("mm, sl[aou]b: guarantee natural
alignment for kmalloc(power-of-two)").

Paolo

[syzbot]

Auto-closing this bug as obsolete.
No recent activity, existing reproducers are no longer triggering the issue.