All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net,stable] qmi_wwan: do not steal interfaces from class drivers
@ 2018-05-02 20:22 ` Bjørn Mork
  0 siblings, 0 replies; 4+ messages in thread
From: Bjørn Mork @ 2018-05-02 20:22 UTC (permalink / raw)
  To: netdev; +Cc: linux-usb, Bjørn Mork

The USB_DEVICE_INTERFACE_NUMBER matching macro assumes that
the { vendorid, productid, interfacenumber } set uniquely
identifies one specific function.  This has proven to fail
for some configurable devices. One example is the Quectel
EM06/EP06 where the same interface number can be either
QMI or MBIM, without the device ID changing either.

Fix by requiring the vendor-specific class for interface number
based matching.  Functions of other classes can and should use
class based matching instead.

Fixes: 03304bcb5ec4 ("net: qmi_wwan: use fixed interface number matching")
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
It's quite possible that the fix should be integrated in the
USB_DEVICE_INTERFACE_NUMBER macro instead.  But that has grown a few
other users since it was added, so changing it now seems risky. 
Another option is of course adding a new match macro with the
USB_CLASS_VENDOR_SPEC match integrated. Maybe best?

But I'm proposing this as-is for now, since this quickfix seems most
suitable for stable backporting.

 drivers/net/usb/qmi_wwan.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 51c68fc416fa..42565dd33aa6 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1344,6 +1344,18 @@ static int qmi_wwan_probe(struct usb_interface *intf,
 		id->driver_info = (unsigned long)&qmi_wwan_info;
 	}
 
+	/* There are devices where the same interface number can be
+	 * configured as different functions. We should only bind to
+	 * vendor specific functions when matching on interface number
+	 */
+	if (id->match_flags & USB_DEVICE_ID_MATCH_INT_NUMBER &&
+	    desc->bInterfaceClass != USB_CLASS_VENDOR_SPEC) {
+		dev_dbg(&intf->dev,
+			"Rejecting interface number match for class %02x\n",
+			desc->bInterfaceClass);
+		return -ENODEV;
+	}
+
 	/* Quectel EC20 quirk where we've QMI on interface 4 instead of 0 */
 	if (quectel_ec20_detected(intf) && desc->bInterfaceNumber == 0) {
 		dev_dbg(&intf->dev, "Quectel EC20 quirk, skipping interface 0\n");
-- 
2.11.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [net,stable] qmi_wwan: do not steal interfaces from class drivers
@ 2018-05-02 20:22 ` Bjørn Mork
  0 siblings, 0 replies; 4+ messages in thread
From: Bjørn Mork @ 2018-05-02 20:22 UTC (permalink / raw)
  To: netdev; +Cc: linux-usb, Bjørn Mork

The USB_DEVICE_INTERFACE_NUMBER matching macro assumes that
the { vendorid, productid, interfacenumber } set uniquely
identifies one specific function.  This has proven to fail
for some configurable devices. One example is the Quectel
EM06/EP06 where the same interface number can be either
QMI or MBIM, without the device ID changing either.

Fix by requiring the vendor-specific class for interface number
based matching.  Functions of other classes can and should use
class based matching instead.

Fixes: 03304bcb5ec4 ("net: qmi_wwan: use fixed interface number matching")
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
It's quite possible that the fix should be integrated in the
USB_DEVICE_INTERFACE_NUMBER macro instead.  But that has grown a few
other users since it was added, so changing it now seems risky. 
Another option is of course adding a new match macro with the
USB_CLASS_VENDOR_SPEC match integrated. Maybe best?

But I'm proposing this as-is for now, since this quickfix seems most
suitable for stable backporting.

 drivers/net/usb/qmi_wwan.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 51c68fc416fa..42565dd33aa6 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1344,6 +1344,18 @@ static int qmi_wwan_probe(struct usb_interface *intf,
 		id->driver_info = (unsigned long)&qmi_wwan_info;
 	}
 
+	/* There are devices where the same interface number can be
+	 * configured as different functions. We should only bind to
+	 * vendor specific functions when matching on interface number
+	 */
+	if (id->match_flags & USB_DEVICE_ID_MATCH_INT_NUMBER &&
+	    desc->bInterfaceClass != USB_CLASS_VENDOR_SPEC) {
+		dev_dbg(&intf->dev,
+			"Rejecting interface number match for class %02x\n",
+			desc->bInterfaceClass);
+		return -ENODEV;
+	}
+
 	/* Quectel EC20 quirk where we've QMI on interface 4 instead of 0 */
 	if (quectel_ec20_detected(intf) && desc->bInterfaceNumber == 0) {
 		dev_dbg(&intf->dev, "Quectel EC20 quirk, skipping interface 0\n");

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net,stable] qmi_wwan: do not steal interfaces from class drivers
@ 2018-05-03 15:25   ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2018-05-03 15:25 UTC (permalink / raw)
  To: bjorn; +Cc: netdev, linux-usb

From: Bjørn Mork <bjorn@mork.no>
Date: Wed,  2 May 2018 22:22:54 +0200

> The USB_DEVICE_INTERFACE_NUMBER matching macro assumes that
> the { vendorid, productid, interfacenumber } set uniquely
> identifies one specific function.  This has proven to fail
> for some configurable devices. One example is the Quectel
> EM06/EP06 where the same interface number can be either
> QMI or MBIM, without the device ID changing either.
> 
> Fix by requiring the vendor-specific class for interface number
> based matching.  Functions of other classes can and should use
> class based matching instead.
> 
> Fixes: 03304bcb5ec4 ("net: qmi_wwan: use fixed interface number matching")
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
> It's quite possible that the fix should be integrated in the
> USB_DEVICE_INTERFACE_NUMBER macro instead.  But that has grown a few
> other users since it was added, so changing it now seems risky. 
> Another option is of course adding a new match macro with the
> USB_CLASS_VENDOR_SPEC match integrated. Maybe best?
> 
> But I'm proposing this as-is for now, since this quickfix seems most
> suitable for stable backporting.

Yes, this simpler approache is better for net and -stable.

Applied.

If you want to do something more sophisticated, that can be done
in net-next.

Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [net,stable] qmi_wwan: do not steal interfaces from class drivers
@ 2018-05-03 15:25   ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2018-05-03 15:25 UTC (permalink / raw)
  To: bjorn; +Cc: netdev, linux-usb

From: Bjørn Mork <bjorn@mork.no>
Date: Wed,  2 May 2018 22:22:54 +0200

> The USB_DEVICE_INTERFACE_NUMBER matching macro assumes that
> the { vendorid, productid, interfacenumber } set uniquely
> identifies one specific function.  This has proven to fail
> for some configurable devices. One example is the Quectel
> EM06/EP06 where the same interface number can be either
> QMI or MBIM, without the device ID changing either.
> 
> Fix by requiring the vendor-specific class for interface number
> based matching.  Functions of other classes can and should use
> class based matching instead.
> 
> Fixes: 03304bcb5ec4 ("net: qmi_wwan: use fixed interface number matching")
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
> It's quite possible that the fix should be integrated in the
> USB_DEVICE_INTERFACE_NUMBER macro instead.  But that has grown a few
> other users since it was added, so changing it now seems risky. 
> Another option is of course adding a new match macro with the
> USB_CLASS_VENDOR_SPEC match integrated. Maybe best?
> 
> But I'm proposing this as-is for now, since this quickfix seems most
> suitable for stable backporting.

Yes, this simpler approache is better for net and -stable.

Applied.

If you want to do something more sophisticated, that can be done
in net-next.

Thanks.
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-05-03 15:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-02 20:22 [PATCH net,stable] qmi_wwan: do not steal interfaces from class drivers Bjørn Mork
2018-05-02 20:22 ` [net,stable] " Bjørn Mork
2018-05-03 15:25 ` [PATCH net,stable] " David Miller
2018-05-03 15:25   ` [net,stable] " David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.