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

gregkh
2022-03-02T21:37:23Z

On Wed, Mar 02, 2022 at 02:10:15PM -0500, stern@rowland.harvard.edu wrote:
> On Sat, Feb 26, 2022 at 10:07:02AM +0100, gregkh@linuxfoundation.org wrote:
> > On Fri, Feb 25, 2022 at 10:51:48AM -0500, stern@rowland.harvard.edu wrote:
> > > On Fri, Feb 25, 2022 at 09:53:35AM +0100, gregkh@linuxfoundation.org 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