RE: [EXT] Re: [PATCH] usb: roles: try to get/put all relevant modules

Xu Yang
2024-01-12T09:28:07Z

Hi Greg,

>
> On Fri, Jan 12, 2024 at 09:24:11AM +0100, Greg KH wrote:
> > On Fri, Jan 12, 2024 at 04:01:08PM +0800, Xu Yang wrote:
> > > +void usb_role_switch_get_modules(struct device *dev)
> > > +{
> > > + while (dev) {
> > > + if (dev->driver)
> > > + WARN_ON(!try_module_get(dev->driver->owner));
> >
> > You just crashed all systems that have panic-on-warn enabled, which is
> > by far (i.e. in the billions) the huge majority of Linux systems in the
> > world.
> >
> > If this is something that can fail, then properly handle the issue,
> > don't just give up and say "let's fill the kernel log with a mess and
> > reboot the box!".
>
> Ah, I see now you are just moving the code, but please, let's fix this
> properly, don't persist in the wrong code here.

This is a true module dependency issue and it only occurs when
load/unload modules. The dependency of usb controller glue driver,
usb controller driver and the user driver (such as tcpm) of usb role
switch is not correctly established.

This patch is used to adjust dependency of them, without it, two issues
may happen:
1. "NULL pointer dereference" kernel dump will be shown
2. The reference count of usb controller module may never decrease to 0

Thanks,
Xu Yang

>
> thanks,
>
> greg k-h