linux-kernel - Re: KASAN: slab-out-of-bounds Read in usb_reset_and_verify

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]

Date:   Wed, 4 Sep 2019 17:01:17 +0200
From:   Andrey Konovalov <andreyknvl@...gle.com>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     syzbot <syzbot+35f4d916c623118d576e@...kaller.appspotmail.com>,
        Thinh.Nguyen@...opsys.com, dianders@...omium.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        jflat@...omium.org, Kai Heng Feng <kai.heng.feng@...onical.com>,
        LKML <linux-kernel@...r.kernel.org>,
        USB list <linux-usb@...r.kernel.org>, malat@...ian.org,
        mathias.nyman@...ux.intel.com, nsaenzjulienne@...e.de,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>
Subject: Re: KASAN: slab-out-of-bounds Read in usb_reset_and_verify_device

On Wed, Sep 4, 2019 at 4:41 PM Alan Stern <stern@...land.harvard.edu> wrote:
>
> On Tue, 3 Sep 2019, syzbot wrote:
>
> > Hello,
> >
> > syzbot has tested the proposed patch but the reproducer still triggered
> > crash:
> > KASAN: slab-out-of-bounds Read in usb_reset_and_verify_device
> >
> > usb 6-1: Using ep0 maxpacket: 16
> > usb 6-1: BOS total length 54, descriptor 168
> > usb 6-1: Old BOS ffff8881cd814f60  Len 0xa8
> > usb 6-1: New BOS ffff8881cd257ae0  Len 0xa8
> > ==================================================================
> > BUG: KASAN: slab-out-of-bounds in memcmp+0xa6/0xb0 lib/string.c:904
> > Read of size 1 at addr ffff8881cd257c36 by task kworker/1:0/17
>
> Very sneaky!  A BOS descriptor whose wTotalLength field varies
> depending on how many bytes you read.
>
> This should fix it.  It's the same approach we use for the Config
> descriptor.

Nice, core USB bug :)

Can this potentially lead to something worse than a out-of-bounds memcmp?

>
> Alan Stern
>
> #syz test: https://github.com/google/kasan.git eea39f24
>
>  drivers/usb/core/config.c |   12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> Index: usb-devel/drivers/usb/core/config.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/config.c
> +++ usb-devel/drivers/usb/core/config.c
> @@ -921,7 +921,7 @@ int usb_get_bos_descriptor(struct usb_de
>         struct usb_bos_descriptor *bos;
>         struct usb_dev_cap_header *cap;
>         struct usb_ssp_cap_descriptor *ssp_cap;
> -       unsigned char *buffer;
> +       unsigned char *buffer, *buffer0;
>         int length, total_len, num, i, ssac;
>         __u8 cap_type;
>         int ret;
> @@ -966,10 +966,12 @@ int usb_get_bos_descriptor(struct usb_de
>                         ret = -ENOMSG;
>                 goto err;
>         }
> +
> +       buffer0 = buffer;
>         total_len -= length;
> +       buffer += length;
>
>         for (i = 0; i < num; i++) {
> -               buffer += length;
>                 cap = (struct usb_dev_cap_header *)buffer;
>
>                 if (total_len < sizeof(*cap) || total_len < cap->bLength) {
> @@ -983,8 +985,6 @@ int usb_get_bos_descriptor(struct usb_de
>                         break;
>                 }
>
> -               total_len -= length;
> -
>                 if (cap->bDescriptorType != USB_DT_DEVICE_CAPABILITY) {
>                         dev_warn(ddev, "descriptor type invalid, skip\n");
>                         continue;
> @@ -1019,7 +1019,11 @@ int usb_get_bos_descriptor(struct usb_de
>                 default:
>                         break;
>                 }
> +
> +               total_len -= length;
> +               buffer += length;
>         }
> +       dev->bos->desc->wTotalLength = cpu_to_le16(buffer - buffer0);
>
>         return 0;
>
>