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 for Android: free password hash cracker in your pocket

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

Message-ID: <929068.1579526141@warthog.procyon.org.uk>
Date:   Mon, 20 Jan 2020 13:15:41 +0000
From:   David Howells <dhowells@...hat.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     dhowells@...hat.com,
        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

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.

>   2771          memcpy(n.n.name, devname, n_len);
>                                           ^^^^^
> name_len was intended here.

Yeah.  I think that's actually the bug.  n_len is the length of the entire
notification record.

David