linux-kernel - Re: linux-next boot error: KASAN: slab-out-of-bounds Read in post_usb

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] [day] [month] [year] [list]

Message-ID: <20200120133758.GK19765@kadam>
Date:   Mon, 20 Jan 2020 16:37:58 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     David Howells <dhowells@...hat.com>
Cc:     syzbot <syzbot+afeecc39f502a8681560@...kaller.appspotmail.com>,
        arnd@...db.de, dmitry.torokhov@...il.com, ebiederm@...ssion.com,
        gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org, stern@...land.harvard.edu,
        syzkaller-bugs@...glegroups.com
Subject: Re: linux-next boot error: KASAN: slab-out-of-bounds Read in
 post_usb_notification

On Mon, Jan 20, 2020 at 01:15:41PM +0000, David Howells wrote:
> Dan Carpenter <dan.carpenter@...cle.com> wrote:
> 
> >   2759          struct {
> >   2760                  struct usb_notification n;
> >   2761                  char more_name[USB_NOTIFICATION_MAX_NAME_LEN -
> >   2762                                 (sizeof(struct usb_notification) -
> >   2763                                  offsetof(struct usb_notification, name))];
> >   2764          } n;
> >   2765  
> >   2766          name_len = strlen(devname);
> >   2767          name_len = min_t(size_t, name_len, USB_NOTIFICATION_MAX_NAME_LEN);
> >                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > This limit is too high.  It should be USB_NOTIFICATION_MAX_NAME_LEN -
> > sizeof(struct usb_notification). or just
> > "min_t(size_t, name_len, sizeof(n.more_name));".  n.n.name[] is a
> > zero size array.
> 
> No.  It's not that simple.  If you look at the struct:
> 
> 	struct usb_notification {
> 		struct watch_notification watch;
> 		__u32	error;
> 		__u32	reserved;
> 		__u8	name_len;
> 		__u8	name[0];
> 	};
> 
> There are at least 3, if not 7, bytes of padding after name[] as the struct is
> not packed - and isn't necessarily rounded up to a multiple of 8 bytes either.
> If you look at the definition of more_name[] above, you'll see:
> 
> 	USB_NOTIFICATION_MAX_NAME_LEN -
> 	(sizeof(struct usb_notification) -
> 	 offsetof(struct usb_notification, name))
> 
> That calculates the amount of padding and then subtracts it from the amount of
> name bufferage required.
> 
> USB_NOTIFICATION_MAX_NAME_LEN is 63, which is 64 minus one for the length.

Ah yes...  You're right.  I didn't think about padding.  And even if I
had, I would have thought the hole would have gone before name[] but
it comes after as you say.

regards,
dan carpenter