linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
       [not found] ` <87lfutei1j.fsf@vitty.brq.redhat.com>
@ 2019-09-12 16:49   ` Paolo Bonzini
  2019-09-13  4:46     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2019-09-12 16:49 UTC (permalink / raw)
  To: Vitaly Kuznetsov, kvm
  Cc: bp, carlo, catalin.marinas, devicetree, hpa, jmattson, joro,
	khilman, linux-amlogic, linux-arm-kernel, linux-kernel,
	mark.rutland, mingo, narmstrong, rkrcmar, robh+dt,
	sean.j.christopherson, syzkaller-bugs, tglx, wanpengli,
	will.deacon, x86, syzbot, Dmitry Vyukov, Greg Kroah-Hartman,
	USB list

[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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
  2019-09-12 16:49   ` KASAN: slab-out-of-bounds Read in handle_vmptrld Paolo Bonzini
@ 2019-09-13  4:46     ` Greg Kroah-Hartman
  2019-09-13  7:34       ` Paolo Bonzini
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-09-13  4:46 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Vitaly Kuznetsov, kvm, bp, carlo, catalin.marinas, devicetree,
	hpa, jmattson, joro, khilman, linux-amlogic, linux-arm-kernel,
	linux-kernel, mark.rutland, mingo, narmstrong, rkrcmar, robh+dt,
	sean.j.christopherson, syzkaller-bugs, tglx, wanpengli,
	will.deacon, x86, syzbot, Dmitry Vyukov, USB list

On Thu, Sep 12, 2019 at 06:49:26PM +0200, Paolo Bonzini wrote:
> [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.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
  2019-09-13  4:46     ` Greg Kroah-Hartman
@ 2019-09-13  7:34       ` Paolo Bonzini
  2019-09-13 13:02         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2019-09-13  7:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vitaly Kuznetsov, kvm, bp, carlo, catalin.marinas, devicetree,
	hpa, jmattson, joro, khilman, linux-amlogic, linux-arm-kernel,
	linux-kernel, mark.rutland, mingo, narmstrong, rkrcmar, robh+dt,
	sean.j.christopherson, syzkaller-bugs, tglx, wanpengli,
	will.deacon, x86, syzbot, Dmitry Vyukov, USB list

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
  2019-09-13  7:34       ` Paolo Bonzini
@ 2019-09-13 13:02         ` Greg Kroah-Hartman
  2019-09-13 15:01           ` Paolo Bonzini
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2019-09-13 13:02 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Vitaly Kuznetsov, kvm, bp, carlo, catalin.marinas, devicetree,
	hpa, jmattson, joro, khilman, linux-amlogic, linux-arm-kernel,
	linux-kernel, mark.rutland, mingo, narmstrong, rkrcmar, robh+dt,
	sean.j.christopherson, syzkaller-bugs, tglx, wanpengli,
	will.deacon, x86, syzbot, Dmitry Vyukov, USB list

On Fri, Sep 13, 2019 at 09:34:32AM +0200, Paolo Bonzini wrote:
> 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);

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

>  
> 
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
  2019-09-13 13:02         ` Greg Kroah-Hartman
@ 2019-09-13 15:01           ` Paolo Bonzini
  2019-09-13 15:32             ` Robin Murphy
  2019-09-13 15:36             ` Alan Stern
  0 siblings, 2 replies; 10+ messages in thread
From: Paolo Bonzini @ 2019-09-13 15:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vitaly Kuznetsov, kvm, bp, carlo, catalin.marinas, devicetree,
	hpa, jmattson, joro, khilman, linux-amlogic, linux-arm-kernel,
	linux-kernel, mark.rutland, mingo, narmstrong, rkrcmar, robh+dt,
	sean.j.christopherson, syzkaller-bugs, tglx, wanpengli,
	will.deacon, x86, syzbot, Dmitry Vyukov, USB list

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
  2019-09-13 15:01           ` Paolo Bonzini
@ 2019-09-13 15:32             ` Robin Murphy
  2019-09-13 21:39               ` Paolo Bonzini
  2019-09-13 15:36             ` Alan Stern
  1 sibling, 1 reply; 10+ messages in thread
From: Robin Murphy @ 2019-09-13 15:32 UTC (permalink / raw)
  To: Paolo Bonzini, Greg Kroah-Hartman
  Cc: mark.rutland, x86, wanpengli, kvm, narmstrong, catalin.marinas,
	will.deacon, hpa, khilman, joro, rkrcmar, mingo, Dmitry Vyukov,
	syzbot, devicetree, syzkaller-bugs, robh+dt, bp, linux-amlogic,
	tglx, linux-arm-kernel, jmattson, USB list, linux-kernel,
	sean.j.christopherson, carlo, Vitaly Kuznetsov

On 13/09/2019 16:01, 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.

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/20190808084636.GB15080@priv-mua.localdomain/
[2] 
https://lore.kernel.org/linux-usb/20190801220134.3295-1-gavinli@thegavinli.com/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
  2019-09-13 15:01           ` Paolo Bonzini
  2019-09-13 15:32             ` Robin Murphy
@ 2019-09-13 15:36             ` Alan Stern
  2019-09-13 16:14               ` Paolo Bonzini
  1 sibling, 1 reply; 10+ messages in thread
From: Alan Stern @ 2019-09-13 15:36 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Greg Kroah-Hartman, Vitaly Kuznetsov, kvm, bp, carlo,
	catalin.marinas, devicetree, hpa, jmattson, joro, khilman,
	linux-amlogic, linux-arm-kernel, linux-kernel, mark.rutland,
	mingo, narmstrong, rkrcmar, robh+dt, sean.j.christopherson,
	syzkaller-bugs, tglx, wanpengli, will.deacon, x86, syzbot,
	Dmitry Vyukov, USB list

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.

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
  2019-09-13 15:36             ` Alan Stern
@ 2019-09-13 16:14               ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2019-09-13 16:14 UTC (permalink / raw)
  To: Alan Stern
  Cc: Greg Kroah-Hartman, Vitaly Kuznetsov, kvm, bp, carlo,
	catalin.marinas, devicetree, hpa, jmattson, joro, khilman,
	linux-amlogic, linux-arm-kernel, linux-kernel, mark.rutland,
	mingo, narmstrong, rkrcmar, robh+dt, sean.j.christopherson,
	syzkaller-bugs, tglx, wanpengli, will.deacon, x86, syzbot,
	Dmitry Vyukov, USB list

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
  2019-09-13 15:32             ` Robin Murphy
@ 2019-09-13 21:39               ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2019-09-13 21:39 UTC (permalink / raw)
  To: Robin Murphy, Greg Kroah-Hartman
  Cc: mark.rutland, x86, wanpengli, kvm, narmstrong, catalin.marinas,
	will.deacon, hpa, khilman, joro, rkrcmar, mingo, Dmitry Vyukov,
	syzbot, devicetree, syzkaller-bugs, robh+dt, bp, linux-amlogic,
	tglx, linux-arm-kernel, jmattson, USB list, linux-kernel,
	sean.j.christopherson, carlo, Vitaly Kuznetsov

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: slab-out-of-bounds Read in handle_vmptrld
       [not found] <000000000000a9d4f705924cff7a@google.com>
       [not found] ` <87lfutei1j.fsf@vitty.brq.redhat.com>
@ 2019-10-21 15:47 ` Paolo Bonzini
  1 sibling, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2019-10-21 15:47 UTC (permalink / raw)
  To: syzbot, jmattson, joro, kvm, linux-kernel, mingo, rkrcmar,
	sean.j.christopherson, syzkaller-bugs, tglx, vkuznets, wanpengli,
	will.deacon, x86, USB list, Greg Kroah-Hartman, Alan Stern

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

Paolo

On 11/09/19 22:38, syzbot wrote:
> 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 <narmstrong@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+46f1dd7dbbe2bfb98b10@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 syzkaller@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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-10-21 15:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <000000000000a9d4f705924cff7a@google.com>
     [not found] ` <87lfutei1j.fsf@vitty.brq.redhat.com>
2019-09-12 16:49   ` KASAN: slab-out-of-bounds Read in handle_vmptrld Paolo Bonzini
2019-09-13  4:46     ` Greg Kroah-Hartman
2019-09-13  7:34       ` Paolo Bonzini
2019-09-13 13:02         ` Greg Kroah-Hartman
2019-09-13 15:01           ` Paolo Bonzini
2019-09-13 15:32             ` Robin Murphy
2019-09-13 21:39               ` Paolo Bonzini
2019-09-13 15:36             ` Alan Stern
2019-09-13 16:14               ` Paolo Bonzini
2019-10-21 15:47 ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).