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: <Ypor265BTdnmgwpM@rowland.harvard.edu>
Date:   Fri, 3 Jun 2022 11:42:19 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     syzbot <syzbot+dd3c97de244683533381@...kaller.appspotmail.com>,
        gregkh@...uxfoundation.org, hdanton@...a.com, lenb@...nel.org,
        linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
        rafael.j.wysocki@...el.com, rafael@...nel.org, rjw@...ysocki.net,
        syzkaller-bugs@...glegroups.com, linux-usb@...r.kernel.org
Subject: Re: [syzbot] general protection fault in __device_attach

On Fri, Jun 03, 2022 at 02:04:04PM +0300, Andy Shevchenko wrote:
> On Fri, Jun 03, 2022 at 03:02:07AM -0700, syzbot wrote:
> > syzbot has bisected this issue to:
> > 
> > commit a9c4cf299f5f79d5016c8a9646fa1fc49381a8c1
> > Author: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> > Date:   Fri Jun 18 13:41:27 2021 +0000
> > 
> >     ACPI: sysfs: Use __ATTR_RO() and __ATTR_RW() macros
> 
> Hmm... It's not obvious at all how this change can alter the behaviour so
> drastically. device_add() is called from USB core with intf->dev.name == NULL
> by some reason. A-ha, seems like fault injector, which looks like
> 
> 	dev_set_name(&intf->dev, "%d-%s:%d.%d", dev->bus->busnum,
> 		     dev->devpath, configuration, ifnum);
> 
> missed the return code check.
> 
> But I'm not familiar with that code at all, adding Linux USB ML and Alan.

I can't see any connection between this bug and acpi/sysfs.c.  Is it a 
bad bisection?

It looks like you're right about dev_set_name() failing.  In fact, the 
kernel appears to be littered with calls to that routine which do not 
check the return code (the entire subtree below drivers/usb/ contains 
only _one_ call that does check the return code!).  The function doesn't 
have any __must_check annotation, and its kerneldoc doesn't mention the 
return code or the possibility of a failure.

Apparently the assumption is that if dev_set_name() fails then 
device_add() later on will also fail, and the problem will be detected 
then.

So now what should happen when device_add() for an interface fails in 
usb_set_configuration()?  I guess the interface should be deleted; 
otherwise we have the possibility that people might still try to access 
it via usbfs, as in the syzbot test run.  Same goes for the 
of_device_is_available() check.

Fixing that will be a little painful.  Right now there are plenty of 
places in the USB core that aren't prepared to cope with a non-existent 
interface.

Alan Stern

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ