Re: [PATCH v3 02/10] usb: roles: Handle driver reference counting

2018-09-07 Thread Heikki Krogerus 
On Thu, Sep 06, 2018 at 10:59:34PM +0200, Hans de Goede wrote:
> HI,
> 
> On 04-09-18 13:22, Heikki Krogerus wrote:
> > This fixes potential "BUG: unable to handle kernel paging
> > request at ..." from happening.
> > 
> > Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
> > Cc: 
> > Signed-off-by: Heikki Krogerus 
> > ---
> >   drivers/usb/common/roles.c | 15 ---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
> > index 15cc76e22123..3d8a776e55ee 100644
> > --- a/drivers/usb/common/roles.c
> > +++ b/drivers/usb/common/roles.c
> > @@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct 
> > device_connection *con, int ep,
> >*/
> >   struct usb_role_switch *usb_role_switch_get(struct device *dev)
> >   {
> > -   return device_connection_find_match(dev, "usb-role-switch", NULL,
> > -   usb_role_switch_match);
> > +   struct usb_role_switch *sw;
> > +
> > +   sw = device_connection_find_match(dev, "usb-role-switch", NULL,
> > + usb_role_switch_match);
> > +
> > +   if (!IS_ERR(sw))
> > +   WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
> 
> While testing I found a bug, so sorry but this is going to need a v4,
> device_connection_find_match() may return NULL here, so this needs
> to be if (!IS_ERR_OR_NULL(sw)) to avoid an oops.
> 
> Note I'm also seeing some other issues which I need to debug I will
> do so tomorrow morning so you may want to wait a bit with v4.

Np. Take your time. And thanks for testing these.


Cheers,

-- 
heikki

Re: [PATCH v3 02/10] usb: roles: Handle driver reference counting

2018-09-06 Thread Hans de Goede 

HI,

On 04-09-18 13:22, Heikki Krogerus wrote:

This fixes potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Cc: 
Signed-off-by: Heikki Krogerus 
---
  drivers/usb/common/roles.c | 15 ---
  1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 15cc76e22123..3d8a776e55ee 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct 
device_connection *con, int ep,
   */
  struct usb_role_switch *usb_role_switch_get(struct device *dev)
  {
-   return device_connection_find_match(dev, "usb-role-switch", NULL,
-   usb_role_switch_match);
+   struct usb_role_switch *sw;
+
+   sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);
+
+   if (!IS_ERR(sw))
+   WARN_ON(!try_module_get(sw->dev.parent->driver->owner));


While testing I found a bug, so sorry but this is going to need a v4,
device_connection_find_match() may return NULL here, so this needs
to be if (!IS_ERR_OR_NULL(sw)) to avoid an oops.

Note I'm also seeing some other issues which I need to debug I will
do so tomorrow morning so you may want to wait a bit with v4.

Regards,

Hans



+
+   return sw;
  }
  EXPORT_SYMBOL_GPL(usb_role_switch_get);
  
@@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get);

   */
  void usb_role_switch_put(struct usb_role_switch *sw)
  {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
put_device(>dev);
+   module_put(sw->dev.parent->driver->owner);
+   }
  }
  EXPORT_SYMBOL_GPL(usb_role_switch_put);

[PATCH v3 02/10] usb: roles: Handle driver reference counting

2018-09-04 Thread Heikki Krogerus 
This fixes potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Cc: 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/common/roles.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 15cc76e22123..3d8a776e55ee 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct 
device_connection *con, int ep,
  */
 struct usb_role_switch *usb_role_switch_get(struct device *dev)
 {
-   return device_connection_find_match(dev, "usb-role-switch", NULL,
-   usb_role_switch_match);
+   struct usb_role_switch *sw;
+
+   sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);
+
+   if (!IS_ERR(sw))
+   WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+
+   return sw;
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get);
 
@@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get);
  */
 void usb_role_switch_put(struct usb_role_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
put_device(>dev);
+   module_put(sw->dev.parent->driver->owner);
+   }
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_put);
 
-- 
2.18.0