linux-kernel - Re: usb: use-after-free write in usb_hcd_link_urb_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.1703241024080.1861-100000@iolanthe.rowland.org>
Date:   Fri, 24 Mar 2017 10:27:43 -0400 (EDT)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Dmitry Vyukov <dvyukov@...gle.com>
cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        <mathias.nyman@...ux.intel.com>, <baoyou.xie@...aro.org>,
        <peter.chen@....com>, william wu <wulf@...k-chips.com>,
        <wsa-dev@...g-engineering.com>, <javier@....samsung.com>,
        Chris Bainbridge <chris.bainbridge@...il.com>,
        USB list <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        syzkaller <syzkaller@...glegroups.com>
Subject: Re: usb: use-after-free write in usb_hcd_link_urb_to_ep

On Fri, 24 Mar 2017, Dmitry Vyukov wrote:

> On Thu, Mar 23, 2017 at 4:22 PM, Dmitry Vyukov <dvyukov@...gle.com> wrote:
> >> On Thu, 23 Mar 2017, Dmitry Vyukov wrote:
> >>
> >>> > Putting these together:
> >>> >
> >>> >         The memory was allocated in usb_internal_control_msg() line 93.
> >>> >         The later events occurred within the call in line 100 to
> >>> >         usb_start_wait_urb().
> >>> >
> >>> >         The invalid access occurred within usb_start_wait_urb() line 56.
> >>> >
> >>> >         The memory was deallocated within usb_start_wait_urb() line 78.
> >>> >
> >>> > Since these routines don't involve any loops or backward jumps, this
> >>> > says that the invalid access occurred before the memory was
> >>> > deallocated!  So why is it reported as a problem?
> >>>
> >>>
> >>> My first guess would be that pid 3348 did 2 calls to open and the urb
> >>> was somehow referenced across these calls. Is it possible?
> >>
> >> I don't think so.  The URB gets allocated and deallocated separately
> >> for each call.  You can see this very plainly by reading the source
> >> code for usb_internal_control_msg() and usb_start_wait_urb().
> >>
> >> It's possible that the same memory location was allocated and
> >> deallocated for two different calls at different times.  That wouldn't
> >> fool syzkaller, would it?
> >
> >
> > Generally it does not fool KASAN because of heap memory quarantine.
> > I will take a closer look tomorrow.
> > Thanks for looking into this.
> 
> 
> The bug looks real to me and it can be easily reproduced by executing:
> 
> mmap(&(0x7f0000000000/0xfff000)=nil, (0xfff000), 0x3, 0x32,
> 0xffffffffffffffff, 0x0)
> syz_open_dev$usb(&(0x7f0000001000-0x15)="2f6465762f6275732f7573622f3030232f30302300",
> 0x1ff, 0x200)
> syz_open_dev$usb(&(0x7f0000002000-0x15)="2f6465762f6275732f7573622f3030232f30302300",
> 0x3f, 0x0)

(Incidentally, those ASCII strings say "/dev/bus/usb/00#/00#", which is 
a rather unusual name for a USB device node.  Where did it come from?)

> 
> and failing 7-th malloc in the first one, this one:
> 
>  kzalloc include/linux/slab.h:663 [inline]
>  rh_call_control drivers/usb/core/hcd.c:522 [inline]
> 
> From the report:
> 
> (from the failure stack)
> kmalloc in rh_call_control fails, and it straight returns -ENOMEM
> without calling  usb_hcd_unlink_urb_from_ep(hcd, urb) leaving urb
> linked into hcd (and that's probably the root problem)

Ah, very good detective work!  I agree.

> consequently:
> rh_urb_enqueue return -ENOMEM
> usb_hcd_submit_urb does INIT_LIST_HEAD(&urb->urb_list), but the urb is
> still linked, so we got corrupted list
> (from the free stack)
> eventually usb_start_wait_urb frees the still linked urb by calling usb_free_urb
> (form the use-after-free stack)
> the subsequent open links a new urb into the corrupted list in
> usb_hcd_link_urb_to_ep
> bang!

Here's a patch to fix the problem; let me know how it works.

Alan



Index: usb-4.x/drivers/usb/core/hcd.c
===================================================================
--- usb-4.x.orig/drivers/usb/core/hcd.c
+++ usb-4.x/drivers/usb/core/hcd.c
@@ -520,8 +520,10 @@ static int rh_call_control (struct usb_h
 	 */
 	tbuf_size =  max_t(u16, sizeof(struct usb_hub_descriptor), wLength);
 	tbuf = kzalloc(tbuf_size, GFP_KERNEL);
-	if (!tbuf)
-		return -ENOMEM;
+	if (!tbuf) {
+		status = -ENOMEM;
+		goto err_alloc;
+	}
 
 	bufp = tbuf;
 
@@ -734,6 +736,7 @@ error:
 	}
 
 	kfree(tbuf);
+ err_alloc:
 
 	/* any errors get returned through the urb completion */
 	spin_lock_irq(&hcd_root_hub_lock);