Re: [PATCH 08/11] usb: Add USB subsystem notifications [ver #7]

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Sep 03, 2019 at 08:53:31AM +0000, Yoshihiro Shimoda wrote:
> Hi,
> 
> > From: David Howells, Sent: Friday, August 30, 2019 10:58 PM
> <snip>
> > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> > index 9063ede411ae..b8572e4d6a1b 100644
> > --- a/drivers/usb/core/devio.c
> > +++ b/drivers/usb/core/devio.c
> > @@ -41,6 +41,7 @@
> >  #include <linux/dma-mapping.h>
> >  #include <asm/byteorder.h>
> >  #include <linux/moduleparam.h>
> > +#include <linux/watch_queue.h>
> > 
> >  #include "usb.h"
> > 
> > @@ -2660,13 +2661,68 @@ static void usbdev_remove(struct usb_device *udev)
> >  	}
> >  }
> > 
> > +#ifdef CONFIG_USB_NOTIFICATIONS
> > +static noinline void post_usb_notification(const char *devname,
> > +					   enum usb_notification_type subtype,
> > +					   u32 error)
> > +{
> > +	unsigned int gran = WATCH_LENGTH_GRANULARITY;
> > +	unsigned int name_len, n_len;
> > +	u64 id = 0; /* Might want to put a dev# here. */
> > +
> > +	struct {
> > +		struct usb_notification n;
> > +		char more_name[USB_NOTIFICATION_MAX_NAME_LEN -
> > +			       (sizeof(struct usb_notification) -
> > +				offsetof(struct usb_notification, name))];
> > +	} n;
> > +
> > +	name_len = strlen(devname);
> > +	name_len = min_t(size_t, name_len, USB_NOTIFICATION_MAX_NAME_LEN);
> > +	n_len = round_up(offsetof(struct usb_notification, name) + name_len,
> > +			 gran) / gran;
> > +
> > +	memset(&n, 0, sizeof(n));
> > +	memcpy(n.n.name, devname, n_len);
> > +
> > +	n.n.watch.type		= WATCH_TYPE_USB_NOTIFY;
> > +	n.n.watch.subtype	= subtype;
> > +	n.n.watch.info		= n_len;
> > +	n.n.error		= error;
> > +	n.n.name_len		= name_len;
> > +
> > +	post_device_notification(&n.n.watch, id);
> > +}
> > +
> > +void post_usb_device_notification(const struct usb_device *udev,
> > +				  enum usb_notification_type subtype, u32 error)
> > +{
> > +	post_usb_notification(dev_name(&udev->dev), subtype, error);
> > +}
> > +
> > +void post_usb_bus_notification(const struct usb_bus *ubus,
> 
> This function's argument is struct usb_bus *, but ...
> 
> > +			       enum usb_notification_type subtype, u32 error)
> > +{
> > +	post_usb_notification(ubus->bus_name, subtype, error);
> > +}
> > +#endif
> > +
> >  static int usbdev_notify(struct notifier_block *self,
> >  			       unsigned long action, void *dev)
> >  {
> >  	switch (action) {
> >  	case USB_DEVICE_ADD:
> > +		post_usb_device_notification(dev, NOTIFY_USB_DEVICE_ADD, 0);
> >  		break;
> >  	case USB_DEVICE_REMOVE:
> > +		post_usb_device_notification(dev, NOTIFY_USB_DEVICE_REMOVE, 0);
> > +		usbdev_remove(dev);
> > +		break;
> > +	case USB_BUS_ADD:
> > +		post_usb_bus_notification(dev, NOTIFY_USB_BUS_ADD, 0);
> > +		break;
> > +	case USB_BUS_REMOVE:
> > +		post_usb_bus_notification(dev, NOTIFY_USB_BUS_REMOVE, 0);
> >  		usbdev_remove(dev);
> 
> this function calls usbdev_remove() with incorrect argument if the action
> is USB_BUS_REMOVE. So, this seems to cause the following issue [1] on
> my environment (R-Car H3 / r8a7795 on next-20190902) [2]. However, I have
> no idea how to fix the issue, so I report this issue at the first step.

As a few of us just discussed this on IRC, these bus notifiers should
probably be dropped as these are the incorrect structure type as you
found out.  Thanks for the report.

greg k-h