Re: [syzbot] KASAN: use-after-free Read in dev_uevent

[gregkh@xxxxxxxxxxxxxxxxxxx]

On Wed, Mar 02, 2022 at 02:10:15PM -0500, stern@xxxxxxxxxxxxxxxxxxx wrote:
> On Sat, Feb 26, 2022 at 10:07:02AM +0100, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
> > On Fri, Feb 25, 2022 at 10:51:48AM -0500, stern@xxxxxxxxxxxxxxxxxxx wrote:
> > > On Fri, Feb 25, 2022 at 09:53:35AM +0100, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
> > > > The aux bus might make this easier:
> > > > 	Documentation/driver-api/auxiliary_bus.rst
> > > 
> > > Won't this end up changing the user-visible filenames and directories in 
> > > sysfs for gadgets and gadget drivers?
> > > 
> > > For instance, currently gadgets get registered under their UDC driver 
> > > name, like "net2280" or "at91".  If we put them on the aux bus then they 
> > > will have to get registered under a name looking something like 
> > > "udc.gadget.0", i.e., module name, generic device name, and ID number.
> > 
> > Ah, yeah, that isn't good.
> > 
> > > We will be forced to use a generic device name because the aux bus does 
> > > matching based on it, and we want every gadget driver to be able to 
> > > match every UDC.  We don't want some gadget drivers restricted to 
> > > net2280 gadgets, others restricted to fotg210 gadgets, and so on.
> > 
> > So yes, I guess it does need to be a "real" bus, sorry.
> 
> It turns out not to be so bad.  In fact there are only five
> gadget drivers (that is, instances of struct usb_gadget_driver) in the 
> kernel:
> 
> 	composite_driver_template	(gadget/composite.c)
> 	configfs_driver_template	(gadget/configfs.c)
> 	gadgetfs_driver			(gadget/legacy/inode.c)
> 	driver				(gadget/legacy/raw_gadget.c)
> 	dbgp_driver			(gadget/legacy/dbgp.c)

I would really love to drop the gadget/legacy/ stuff if at all possible
entirely.  I wonder if that's possible.  I know that Android has moved
off of this, and that used to be the largest user (in the billions), so
we might be ok to possibly just delete these entirely.

> Everything else is implemented as a "function" driver.  So the gadget 
> driver's name doesn't mean very much to the user anyway.

That's good to know.

> The interaction between the gadget subsystem and the device core is 
> rather peculiar.  Each UDC controller is represented by a pair of device 
> structures: the .dev fields in struct usb_udc and struct usb_gadget.  
> These two are siblings -- they always have the same parent (see 
> usb_add_gadget() in gadget/udc/core.c).  Furthermore, they have the same 
> driver; that is, udc->dev.driver is always the same as 
> gadget->dev.driver (see udc_bind_to_driver()).  Which is doubly odd, 
> because gadget drivers manage only gadget devices, not udc devices.  The 
> major difference between them is that the usb_udc is a class device 
> whereas the usb_gadget is a "real" device.
> 
> Currently neither the udc device nor the gadget device is registered on 
> any bus.  IMO it would make sense to leave udc->dev.driver always set to 
> NULL, keep the udc devices bus-less, and put the gadget devices on the 
> aux bus.
> 
> Does that sound reasonable?  I'll work on a patch to do it.

That's odd, but it might work, so sure, let's see how it turns out.

thanks,

greg k-h