Re: [PATCH] USB: serial: qcserial: add Dell DW5818, DW5819

[Bjørn Mork]

Shrirang Bagul  writes:
> On Tue, 2017-10-03 at 15:37 +0200, Johan Hovold wrote:
>
>> Don't you want to add these to qmi_wwan as well?
>
> I haven't tested these devices with qmi_wwan. Perhaps a separate patch once 
> it's
> verified.

Please do, if you can.  I realize that QMI isn't a default config for
these modems, but it's always good to have the driver support in case
some user wants to switch configs.

Note that they most likely support two "RMNET" interfaces, so two
entries will be necessary to make the support complete.



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

configure uvc resolution and format

[Rail Shafigulin]

I've been trying to figure out how to configure UVC resolution and
format. My understanding is that I need to use descriptors for this,
but don't know which ones. I've been reading through the UVC
(http://www.usb.org/developers/docs/devclass_docs/USB_Video_Class_1_5.zip)
spec for several days now but I'm still not able to figure it out.
Would anyone be able to point me in right direction? I also tried to
find a UVC tutorial but came up empty.

Any help is appreciated

-- 
Rail Shafigulin
Software Engineer
Esencia Technologies

-- 




*ESENCIA TECHNOLOGIES, INC.*3945 Freedom Circle, Suite #360,
Santa Clara CA 95054


Phone: +1 408 736 8284 Fax: +1 408 519 3475 
http://www.esenciatech.com | http://www.lnttechservices.com


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

[PATCH] usb: isp1301-omap: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: Thomas Gleixner 
Signed-off-by: Kees Cook 
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/usb/phy/phy-isp1301-omap.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/phy/phy-isp1301-omap.c 
b/drivers/usb/phy/phy-isp1301-omap.c
index c6052c814bcc..43866e0f9217 100644
--- a/drivers/usb/phy/phy-isp1301-omap.c
+++ b/drivers/usb/phy/phy-isp1301-omap.c
@@ -1183,9 +1183,11 @@ static irqreturn_t isp1301_irq(int irq, void *isp)
return IRQ_HANDLED;
 }
 
-static void isp1301_timer(unsigned long _isp)
+static void isp1301_timer(struct timer_list *t)
 {
-   isp1301_defer_work((void *)_isp, WORK_TIMER);
+   struct isp1301 *isp = from_timer(isp, t, timer);
+
+   isp1301_defer_work(isp, WORK_TIMER);
 }
 
 /*-*/
@@ -1507,9 +1509,7 @@ isp1301_probe(struct i2c_client *i2c, const struct 
i2c_device_id *id)
}
 
INIT_WORK(>work, isp1301_work);
-   init_timer(>timer);
-   isp->timer.function = isp1301_timer;
-   isp->timer.data = (unsigned long) isp;
+   timer_setup(>timer, isp1301_timer, 0);
 
i2c_set_clientdata(i2c, isp);
isp->client = i2c;
-- 
2.7.4


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

[PATCH] xhci: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Mathias Nyman 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: Thomas Gleixner 
Signed-off-by: Kees Cook 
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/usb/host/xhci-mem.c  | 4 ++--
 drivers/usb/host/xhci-ring.c | 9 +++--
 drivers/usb/host/xhci.h  | 2 +-
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 2a82c927ded2..f71d46d0aaeb 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -801,8 +801,8 @@ void xhci_free_stream_info(struct xhci_hcd *xhci,
 static void xhci_init_endpoint_timer(struct xhci_hcd *xhci,
struct xhci_virt_ep *ep)
 {
-   setup_timer(>stop_cmd_timer, xhci_stop_endpoint_command_watchdog,
-   (unsigned long)ep);
+   timer_setup(>stop_cmd_timer, xhci_stop_endpoint_command_watchdog,
+   0);
ep->xhci = xhci;
 }
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a9443651ce0f..e28d1b064c6b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -946,15 +946,12 @@ void xhci_hc_died(struct xhci_hcd *xhci)
  * Instead we use a combination of that flag and checking if a new timer is
  * pending.
  */
-void xhci_stop_endpoint_command_watchdog(unsigned long arg)
+void xhci_stop_endpoint_command_watchdog(struct timer_list *t)
 {
-   struct xhci_hcd *xhci;
-   struct xhci_virt_ep *ep;
+   struct xhci_virt_ep *ep = from_timer(ep, t, stop_cmd_timer);
+   struct xhci_hcd *xhci = ep->xhci;
unsigned long flags;
 
-   ep = (struct xhci_virt_ep *) arg;
-   xhci = ep->xhci;
-
spin_lock_irqsave(>lock, flags);
 
/* bail out if cmd completed but raced with stop ep watchdog timer.*/
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2abaa4d6d39d..43291a0d44cd 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2066,7 +2066,7 @@ void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci,
struct xhci_dequeue_state *deq_state);
 void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci, unsigned int ep_index,
unsigned int stream_id, struct xhci_td *td);
-void xhci_stop_endpoint_command_watchdog(unsigned long arg);
+void xhci_stop_endpoint_command_watchdog(struct timer_list *t);
 void xhci_handle_command_timeout(struct work_struct *work);
 
 void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, unsigned int slot_id,
-- 
2.7.4


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

Re: [PATCH v6] xhci : AMD Promontory USB disable port support

[Kai-Heng Feng]

Hi Mathias,

On Mon, Sep 25, 2017 at 4:16 PM, Mathias Nyman
 wrote:
> On 25.09.2017 10:08, Joe Lee wrote:
>>
>> From: Joe Lee 
>>
>> For AMD Promontory xHCI host,
>> although you can disable USB 2.0 ports in BIOSsettings,
>> those ports will be enabled anyway after you remove a device on
>> that port and re-plug it in again. It's a known limitation of the chip.
>> As a workaround we can clear the PORT_WAKE_BITS.
>>
>> Signed-off-by: Joe Lee 
>>
>> ---
>
>
> Kai-Heng Feng,
> Do you have the time to check if this works on your Promontory setup?
>
> Does the usb2 ports stay alive with runtime PM enabled after this patch?

Yes. I think the patch is good to go.

Thanks!

>
> Thanks
> -Mathias
>
>
>> v6: Fix coding format.
>> v5: Add check disable port setting before set PORT_WAKE_BITS
>> v4: Remove the patch code in case USB_PORT_FEAT_REMOTE_WAKE_MASK
>> v3: Fix some checkpatch.pl
>> ---
>>   drivers/usb/host/pci-quirks.c | 116
>> +-
>>   drivers/usb/host/pci-quirks.h |   1 +
>>   drivers/usb/host/xhci-hub.c   |   7 ++-
>>   3 files changed, 121 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
>> index 658d9d1..934220c 100644
>> --- a/drivers/usb/host/pci-quirks.c
>> +++ b/drivers/usb/host/pci-quirks.c
>> @@ -64,7 +64,22 @@
>>   #define   AB_DATA(addr)   ((addr) + 0x04)
>>   #define   AX_INDXC0x30
>>   #define   AX_DATAC0x34
>> -
>> +#define PT_ADDR_INDEX  0xE8
>> +#define PT_READ_INDEX  0xE4
>> +#define PT_SIG_1_ADDR  0xA520
>> +#define PT_SIG_2_ADDR  0xA521
>> +#define PT_SIG_3_ADDR  0xA522
>> +#define PT_SIG_4_ADDR  0xA523
>> +#define PT_SIG_1_DATA  0x78
>> +#define PT_SIG_2_DATA  0x56
>> +#define PT_SIG_3_DATA  0x34
>> +#define PT_SIG_4_DATA  0x12
>> +#define PT_4_PORT_1_REG0xB521
>> +#define PT_4_PORT_2_REG0xB522
>> +#define PT_2_PORT_1_REG0xD520
>> +#define PT_2_PORT_2_REG0xD521
>> +#define PT_1_PORT_1_REG0xD522
>> +#define PT_1_PORT_2_REG0xD523
>>   #define   NB_PCIE_INDX_ADDR   0xe0
>>   #define   NB_PCIE_INDX_DATA   0xe4
>>   #define   PCIE_P_CNTL 0x10040
>> @@ -511,6 +526,105 @@ void usb_amd_dev_put(void)
>>   }
>>   EXPORT_SYMBOL_GPL(usb_amd_dev_put);
>>
>> +int usb_amd_pt_check_port(struct device *device, int port)
>> +{
>> +   unsigned char value;
>> +   struct pci_dev *pdev;
>> +
>> +   pdev = to_pci_dev(device);
>> +   pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_1_ADDR);
>> +   pci_read_config_byte(pdev, PT_READ_INDEX, );
>> +   if (value != PT_SIG_1_DATA)
>> +   return 0;
>> +   pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_2_ADDR);
>> +   pci_read_config_byte(pdev, PT_READ_INDEX, );
>> +   if (value != PT_SIG_2_DATA)
>> +   return 0;
>> +   pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_3_ADDR);
>> +   pci_read_config_byte(pdev, PT_READ_INDEX, );
>> +   if (value != PT_SIG_3_DATA)
>> +   return 0;
>> +   pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_4_ADDR);
>> +   pci_read_config_byte(pdev, PT_READ_INDEX, );
>> +   if (value != PT_SIG_4_DATA)
>> +   return 0;
>> +   if ((pdev->device == 0x43b9) || (pdev->device == 0x43ba)) {
>> +   /* device is AMD_PROMONTORYA_4(0x43b9) or
>> +* PROMONTORYA_3(0x43ba)
>> +* disable port setting PT_4_PORT_1_REG[7..1] is
>> +* USB2.0 port6 to 0
>> +* PT_4_PORT_2_REG[6..0] is USB 13 to port 7
>> +* 0 : disable ;1 : enable
>> +*/
>> +   if (port > 6) {
>> +   pci_write_config_word(pdev, PT_ADDR_INDEX,
>> +   PT_4_PORT_2_REG);
>> +   pci_read_config_byte(pdev, PT_READ_INDEX, );
>> +   if (value & (1<<(port - 7)))
>> +   return 0;
>> +   else
>> +   return 1;
>> +   } else {
>> +   pci_write_config_word(pdev, PT_ADDR_INDEX,
>> +   PT_4_PORT_1_REG);
>> +   pci_read_config_byte(pdev, PT_READ_INDEX, );
>> +   if (value & (1<<(port + 1)))
>> +   return 0;
>> +   else
>> +   return 1;
>> +   }
>> +   } else if (pdev->device == 0x43bb) {
>> +   /* device is AMD_PROMONTORYA_2(0x43bb)
>> +* disable port setting PT_2_PORT_1_REG[7..5] is
>> +* USB2.0 port2 to
>> + 

[PATCH] HID: usbhid: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. Adds pointer back to hid_device for
multitouch.

Cc: Jiri Kosina 
Cc: Benjamin Tissoires 
Cc: linux-in...@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: Thomas Gleixner 
Signed-off-by: Kees Cook 
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/hid/hid-multitouch.c  | 10 ++
 drivers/hid/usbhid/hid-core.c |  8 
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 440b999304a5..b0e3e2614b18 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -112,6 +112,7 @@ struct mt_device {
struct mt_slot curdata; /* placeholder of incoming data */
struct mt_class mtclass;/* our mt device class */
struct timer_list release_timer;/* to release sticky fingers */
+   struct hid_device *hdev;/* hid_device we're attached to */
struct mt_fields *fields;   /* temporary placeholder for storing the
   multitouch fields */
unsigned long mt_io_flags;  /* mt flags (MT_IO_FLAGS_*) */
@@ -1245,10 +1246,10 @@ static void mt_release_contacts(struct hid_device *hid)
td->num_received = 0;
 }
 
-static void mt_expired_timeout(unsigned long arg)
+static void mt_expired_timeout(struct timer_list *t)
 {
-   struct hid_device *hdev = (void *)arg;
-   struct mt_device *td = hid_get_drvdata(hdev);
+   struct mt_device *td = from_timer(td, t, release_timer);
+   struct hid_device *hdev = td->hdev;
 
/*
 * An input report came in just before we release the sticky fingers,
@@ -1279,6 +1280,7 @@ static int mt_probe(struct hid_device *hdev, const struct 
hid_device_id *id)
dev_err(>dev, "cannot allocate multitouch data\n");
return -ENOMEM;
}
+   td->hdev = hdev;
td->mtclass = *mtclass;
td->inputmode = -1;
td->maxcontact_report_id = -1;
@@ -1330,7 +1332,7 @@ static int mt_probe(struct hid_device *hdev, const struct 
hid_device_id *id)
 */
hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
 
-   setup_timer(>release_timer, mt_expired_timeout, (long)hdev);
+   timer_setup(>release_timer, mt_expired_timeout, 0);
 
ret = hid_parse(hdev);
if (ret != 0)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 089bad8a9a21..9f9fe0e58f5b 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -101,10 +101,10 @@ static int hid_start_in(struct hid_device *hid)
 }
 
 /* I/O retry timer routine */
-static void hid_retry_timeout(unsigned long _hid)
+static void hid_retry_timeout(struct timer_list *t)
 {
-   struct hid_device *hid = (struct hid_device *) _hid;
-   struct usbhid_device *usbhid = hid->driver_data;
+   struct usbhid_device *usbhid = from_timer(usbhid, t, io_retry);
+   struct hid_device *hid = usbhid->hid;
 
dev_dbg(>intf->dev, "retrying intr urb\n");
if (hid_start_in(hid))
@@ -1363,7 +1363,7 @@ static int usbhid_probe(struct usb_interface *intf, const 
struct usb_device_id *
 
init_waitqueue_head(>wait);
INIT_WORK(>reset_work, hid_reset);
-   setup_timer(>io_retry, hid_retry_timeout, (unsigned long) hid);
+   timer_setup(>io_retry, hid_retry_timeout, 0);
spin_lock_init(>lock);
 
ret = hid_add_device(hid);
-- 
2.7.4


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

[PATCH] net/usb/usbnet: Convert timers to use timer_setup()

[Kees Cook]

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. Since the callback is called from
both a timer and a tasklet, adjust the tasklet to pass the timer address
too. When tasklets have their .data field removed, this can be refactored
to call a central function after resolving the correct container_of() for a
separate callback function for timer and tasklet.

Cc: Oliver Neukum 
Cc: net...@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: Thomas Gleixner 
Signed-off-by: Kees Cook 
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
 drivers/net/usb/usbnet.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 6510e5cc1817..80348b6a8646 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1509,9 +1509,9 @@ static int rx_alloc_submit(struct usbnet *dev, gfp_t 
flags)
 
 // tasklet (work deferred from completions, in_irq) or timer
 
-static void usbnet_bh (unsigned long param)
+static void usbnet_bh (struct timer_list *t)
 {
-   struct usbnet   *dev = (struct usbnet *) param;
+   struct usbnet   *dev = from_timer(dev, t, delay);
struct sk_buff  *skb;
struct skb_data *entry;
 
@@ -1694,13 +1694,11 @@ usbnet_probe (struct usb_interface *udev, const struct 
usb_device_id *prod)
skb_queue_head_init (>txq);
skb_queue_head_init (>done);
skb_queue_head_init(>rxq_pause);
-   dev->bh.func = usbnet_bh;
-   dev->bh.data = (unsigned long) dev;
+   dev->bh.func = (void (*)(unsigned long))usbnet_bh;
+   dev->bh.data = (unsigned long)>delay;
INIT_WORK (>kevent, usbnet_deferred_kevent);
init_usb_anchor(>deferred);
-   dev->delay.function = usbnet_bh;
-   dev->delay.data = (unsigned long) dev;
-   init_timer (>delay);
+   timer_setup(>delay, usbnet_bh, 0);
mutex_init (>phy_mutex);
mutex_init(>interrupt_mutex);
dev->interrupt_count = 0;
-- 
2.7.4


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

Re: [PATCH] USB: serial: qcserial: add Dell DW5818, DW5819

[Shrirang Bagul]

On Tue, 2017-10-03 at 15:37 +0200, Johan Hovold wrote:
> On Fri, Sep 29, 2017 at 12:39:51PM +0800, Shrirang Bagul wrote:
> > Dell Wireless 5819/5818 devices are re-branded Sierra Wireless MC74
> > series which will by default boot with vid 0x413c and pid's 0x81cf,
> > 0x81d0, 0x81d1,0x81d2.
> > 
> > Signed-off-by: Shrirang Bagul 
> 
> Now applied.
Thank you.
> 
> Don't you want to add these to qmi_wwan as well?
I haven't tested these devices with qmi_wwan. Perhaps a separate patch once it's
verified.
> 
> > ---
> >  drivers/usb/serial/qcserial.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/usb/serial/qcserial.c b/drivers/usb/serial/qcserial.c
> > index ebc0beea69d6..eb9928963a53 100644
> > --- a/drivers/usb/serial/qcserial.c
> > +++ b/drivers/usb/serial/qcserial.c
> > @@ -174,6 +174,10 @@ static const struct usb_device_id id_table[] = {
> >     {DEVICE_SWI(0x413c, 0x81b3)},   /* Dell Wireless 5809e Gobi(TM) 4G
> > LTE Mobile Broadband Card (rev3) */
> >     {DEVICE_SWI(0x413c, 0x81b5)},   /* Dell Wireless 5811e QDL */
> >     {DEVICE_SWI(0x413c, 0x81b6)},   /* Dell Wireless 5811e QDL */
> > +   {DEVICE_SWI(0x413c, 0x81cf)},   /* Dell Wireless 5819 */
> > +   {DEVICE_SWI(0x413c, 0x81d0)},   /* Dell Wireless 5819 */
> > +   {DEVICE_SWI(0x413c, 0x81d1)},   /* Dell Wireless 5818 */
> > +   {DEVICE_SWI(0x413c, 0x81d2)},   /* Dell Wireless 5818 */
> >  
> >     /* Huawei devices */
> >     {DEVICE_HWI(0x03f0, 0x581d)},   /* HP lt4112 LTE/HSPA+ Gobi 4G
> > Modem (Huawei me906e) */
> 
> Thanks,
> Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb/serial: use-after-free in usb_serial_disconnect/__lock_acquire

[Johan Hovold]

On Wed, Sep 27, 2017 at 04:36:11PM +0200, Andrey Konovalov wrote:
> Hi!
> 
> I've got the following report while fuzzing the kernel with syzkaller.
> 
> On commit e19b205be43d11bff638cad4487008c48d21c103 (4.14-rc2).
> 
> gadgetfs: bound to dummy_udc driver
> usb 1-1: new full-speed USB device number 2 using dummy_hcd
> gadgetfs: connected
> gadgetfs: disconnected
> gadgetfs: connected
> usb 1-1: config 4 has an invalid interface number: 1 but max is 0
> usb 1-1: config 4 has an invalid interface number: 153 but max is 0
> usb 1-1: config 4 has 2 interfaces, different from the descriptor's value: 1
> usb 1-1: config 4 has no interface number 0
> usb 1-1: config 4 interface 1 altsetting 255 has an invalid endpoint
> with address 0x0, skipping
> usb 1-1: config 4 interface 1 altsetting 255 has an invalid endpoint
> with address 0xFF, skipping
> usb 1-1: config 4 interface 1 altsetting 255 has an invalid endpoint
> with address 0x56, skipping
> usb 1-1: too many endpoints for config 4 interface 153 altsetting 67:
> 174, using maximum allowed: 30
> usb 1-1: config 4 interface 153 altsetting 67 has 0 endpoint
> descriptors, different from the interface d
> escriptor's value: 174
> usb 1-1: config 4 interface 1 has no altsetting 0
> usb 1-1: config 4 interface 153 has no altsetting 0
> usb 1-1: New USB device found, idVendor=1199, idProduct=6832
> usb 1-1: New USB device strings: Mfr=4, Product=20, SerialNumber=3
> usb 1-1: Product: a
> usb 1-1: Manufacturer: a
> usb 1-1: SerialNumber: a
> gadgetfs: configuration #4
> sierra 1-1:4.1: Sierra USB modem converter detected
> usb 1-1: Sierra USB modem converter now attached to ttyUSB0
> sierra 1-1:4.153: Sierra USB modem converter detected
> gadgetfs: disconnected
> usb 1-1: USB disconnect, device number 2
> sierra ttyUSB0: Sierra USB modem converter now disconnected from ttyUSB0
> sierra 1-1:4.1: device disconnected
> ==
> BUG: KASAN: use-after-free in __lock_acquire+0x4504/0x4550
> Read of size 8 at addr 8800674df790 by task kworker/1:2/1846
> 
> CPU: 1 PID: 1846 Comm: kworker/1:2 Not tainted
> 4.14.0-rc2-42660-g24b7bd59eec0 #277
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: usb_hub_wq hub_event
> Call Trace:
>  __dump_stack lib/dump_stack.c:16
>  dump_stack+0x292/0x395 lib/dump_stack.c:52
>  print_address_description+0x78/0x280 mm/kasan/report.c:252
>  kasan_report_error mm/kasan/report.c:351
>  kasan_report+0x23d/0x350 mm/kasan/report.c:409
>  __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
>  __lock_acquire+0x4504/0x4550 kernel/locking/lockdep.c:3376
>  lock_acquire+0x259/0x620 kernel/locking/lockdep.c:4002
>  __mutex_lock_common kernel/locking/mutex.c:756
>  __mutex_lock+0x18e/0x1a50 kernel/locking/mutex.c:893
>  mutex_lock_nested+0x1b/0x20 kernel/locking/mutex.c:908
>  usb_serial_disconnect+0x69/0x2e0 drivers/usb/serial/usb-serial.c:1084
>  usb_unbind_interface+0x21c/0xa90 drivers/usb/core/driver.c:423
>  __device_release_driver drivers/base/dd.c:861
>  device_release_driver_internal+0x4f4/0x5c0 drivers/base/dd.c:893
>  device_release_driver+0x1e/0x30 drivers/base/dd.c:918
>  bus_remove_device+0x2f4/0x4b0 drivers/base/bus.c:565
>  device_del+0x5c4/0xab0 drivers/base/core.c:1985
>  usb_disable_device+0x1e9/0x680 drivers/usb/core/message.c:1170
>  usb_disconnect+0x260/0x7a0 drivers/usb/core/hub.c:2124
>  hub_port_connect drivers/usb/core/hub.c:4754
>  hub_port_connect_change drivers/usb/core/hub.c:5009
>  port_event drivers/usb/core/hub.c:5115
>  hub_event+0x1318/0x3740 drivers/usb/core/hub.c:5195
>  process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
>  worker_thread+0x221/0x1850 kernel/workqueue.c:2253
>  kthread+0x3a1/0x470 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431

Thanks for reporting this. I was able to reproduce the bug from the
information you provided here, and incidentally discovered a
long-standing related issue which I've now also fixed.

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

Re: [PATCHv2 0/2] dwc3 on XU3

[Andrzej Pietrasiewicz]

Hi,

W dniu 04.10.2017 o 06:22, Kishon Vijay Abraham I pisze:

Hi,







@Kishon:

As far as I understand what you suggest is to move phy_init() and
phy_power_on() invocations from dwc3/core.c to xhci-plat, but,
to the best of my knowledge, they are needed in device mode, too.


What I meant is perform phy initializations for host mode in xhci and keep only
device mode phy initialization in dwc3.



Ah, I should have been more explicit.
Unfortunately, without phy initialization dwc3 registers are unavailable,
so it looks like it is really needed at the point it is done now.

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

[PATCH 2/2] USB: serial: console: fix use-after-free after failed setup

[Johan Hovold]

Make sure to reset the USB-console port pointer when console setup fails
in order to avoid having the struct usb_serial be prematurely freed by
the console code when the device is later disconnected.

Fixes: 73e487fdb75f ("[PATCH] USB console: fix disconnection issues")
Cc: stable  # 2.6.18
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/console.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index ed8ba3ef5c79..43a862a90a77 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -186,6 +186,7 @@ static int usb_console_setup(struct console *co, char 
*options)
tty_kref_put(tty);
  reset_open_count:
port->port.count = 0;
+   info->port = NULL;
usb_autopm_put_interface(serial->interface);
  error_get_interface:
usb_serial_put(serial);
-- 
2.14.2

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

[PATCH 0/2] USB: serial: console: fix two use-after-free bugs

[Johan Hovold]

A recent clean-up patch of mine did more than intended and introduced
the potential for a use-after-free on disconnect under some very
specific circumstances. Fortunately those circumstances were not obscure
enough to prevent Andrey's fuzzing from triggering the bug.

While fixing this one I found a related issue through inspection which
dates back to 2006.

Johan


Johan Hovold (2):
  USB: serial: console: fix use-after-free on disconnect
  USB: serial: console: fix use-after-free after failed setup

 drivers/usb/serial/console.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.14.2

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

[PATCH 1/2] USB: serial: console: fix use-after-free on disconnect

[Johan Hovold]

A clean-up patch removing removing two redundant NULL-checks from the
console disconnect handler inadvertently also removed a third check.
This could lead to the struct usb_serial being prematurely freed by the
console code when a driver accepts but does not register any ports for
an interface which also lacks endpoint descriptors.

Fixes: 0e517c93dc02 ("USB: serial: console: clean up sanity checks")
Cc: stable  # 4.11
Reported-by: Andrey Konovalov 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index fdf89800ebc3..ed8ba3ef5c79 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -265,7 +265,7 @@ static struct console usbcons = {
 
 void usb_serial_console_disconnect(struct usb_serial *serial)
 {
-   if (serial->port[0] == usbcons_info.port) {
+   if (serial->port[0] && serial->port[0] == usbcons_info.port) {
usb_serial_console_exit();
usb_serial_put(serial);
}
-- 
2.14.2

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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Takashi Iwai]

On Tue, 03 Oct 2017 19:42:21 +0200,
Greg Kroah-Hartman wrote:
> 
> On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > 
> > > > It's a dev_WARN because it indicates a potentially serious error in the 
> > > > driver: The driver has submitted an interrupt URB to a bulk endpoint.  
> > > > That may not sound bad, but the same check gets triggered if a driver 
> > > > submits a bulk URB to an isochronous endpoint, or any other invalid 
> > > > combination.
> > > > 
> > > > Most likely the explanation here is that the driver doesn't bother to
> > > > check the endpoint type because it expects the endpoint will always be
> > > > interrupt.  But that is not a safe strategy.  USB devices and their
> > > > firmware should not be trusted unnecessarily.
> > > > 
> > > > The best fix is, like you said, to add a sanity check in the caller.
> > > 
> > > OK, but then do we have some handy helper for the check?
> > > As other bug reports by syzkaller suggest, there are a few other
> > > drivers that do the same, submitting a urb with naive assumption of
> > > the fixed EP for specific devices.  In the end we'll need to put the
> > > very same checks there in multiple places.
> > 
> > Perhaps we could add a helper routine that would take a list of 
> > expected endpoint types and check that the actual endpoints match the 
> > types.  But of course, all the drivers you're talking about would have 
> > to add a call to this helper routine.
> 
> We have almost this type of function, usb_find_common_endpoints(),
> what's wrong with using that?  Johan has already swept the tree and
> added a lot of these checks, odds are no one looked at the sound/
> subdir...

Well, what I had in my mind is just a snippet from usb_submit_urb(),
something like:

bool usb_sanity_check_urb_pipe(struct urb *urb)
{
struct usb_host_endpoint *ep;
int xfertype;
static const int pipetypes[4] = {
PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
};

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
xfertype = usb_endpoint_type(>desc);
return usb_pipetype(urb->pipe) != pipetypes[xfertype];
}

And calling this before usb_submit_urb() in each place that assigns
the fixed EP as device-specific quirks.
Does it make sense?


thanks,

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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Takashi Iwai]

On Wed, 04 Oct 2017 09:52:36 +0200,
Greg Kroah-Hartman wrote:
> 
> On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> > On Tue, 03 Oct 2017 19:42:21 +0200,
> > Greg Kroah-Hartman wrote:
> > > 
> > > On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > > > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > > > 
> > > > > > It's a dev_WARN because it indicates a potentially serious error in 
> > > > > > the 
> > > > > > driver: The driver has submitted an interrupt URB to a bulk 
> > > > > > endpoint.  
> > > > > > That may not sound bad, but the same check gets triggered if a 
> > > > > > driver 
> > > > > > submits a bulk URB to an isochronous endpoint, or any other invalid 
> > > > > > combination.
> > > > > > 
> > > > > > Most likely the explanation here is that the driver doesn't bother 
> > > > > > to
> > > > > > check the endpoint type because it expects the endpoint will always 
> > > > > > be
> > > > > > interrupt.  But that is not a safe strategy.  USB devices and their
> > > > > > firmware should not be trusted unnecessarily.
> > > > > > 
> > > > > > The best fix is, like you said, to add a sanity check in the caller.
> > > > > 
> > > > > OK, but then do we have some handy helper for the check?
> > > > > As other bug reports by syzkaller suggest, there are a few other
> > > > > drivers that do the same, submitting a urb with naive assumption of
> > > > > the fixed EP for specific devices.  In the end we'll need to put the
> > > > > very same checks there in multiple places.
> > > > 
> > > > Perhaps we could add a helper routine that would take a list of 
> > > > expected endpoint types and check that the actual endpoints match the 
> > > > types.  But of course, all the drivers you're talking about would have 
> > > > to add a call to this helper routine.
> > > 
> > > We have almost this type of function, usb_find_common_endpoints(),
> > > what's wrong with using that?  Johan has already swept the tree and
> > > added a lot of these checks, odds are no one looked at the sound/
> > > subdir...
> > 
> > Well, what I had in my mind is just a snippet from usb_submit_urb(),
> > something like:
> > 
> > bool usb_sanity_check_urb_pipe(struct urb *urb)
> > {
> > struct usb_host_endpoint *ep;
> > int xfertype;
> > static const int pipetypes[4] = {
> > PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
> > };
> > 
> > ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > xfertype = usb_endpoint_type(>desc);
> > return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> > }
> > 
> > And calling this before usb_submit_urb() in each place that assigns
> > the fixed EP as device-specific quirks.
> > Does it make sense?
> 
> Yes, kind of, but checking the endpoint type/direction is what you are
> expecting it to be as you "know" what the type should be for each
> driver as it is unique.

Yes, it can be simplified, but if we want a common helper function,
this style would have an advantage that it can be used generically for
all drivers.

> Anyway, a "real" patch might make more sense to me.

I can cook up a patch if you find it a good idea to add such a common
function to usb core side.  OTOH, if each driver should open-code this
in each place, I can work on that, too.  Which would you prefer?


thanks,

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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Takashi Iwai]

On Wed, 04 Oct 2017 11:24:42 +0200,
Johan Hovold wrote:
> 
> On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> > On Tue, 03 Oct 2017 19:42:21 +0200,
> > Greg Kroah-Hartman wrote:
> > > 
> > > On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > > > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > > > 
> > > > > > It's a dev_WARN because it indicates a potentially serious error in 
> > > > > > the 
> > > > > > driver: The driver has submitted an interrupt URB to a bulk 
> > > > > > endpoint.  
> > > > > > That may not sound bad, but the same check gets triggered if a 
> > > > > > driver 
> > > > > > submits a bulk URB to an isochronous endpoint, or any other invalid 
> > > > > > combination.
> > > > > > 
> > > > > > Most likely the explanation here is that the driver doesn't bother 
> > > > > > to
> > > > > > check the endpoint type because it expects the endpoint will always 
> > > > > > be
> > > > > > interrupt.  But that is not a safe strategy.  USB devices and their
> > > > > > firmware should not be trusted unnecessarily.
> > > > > > 
> > > > > > The best fix is, like you said, to add a sanity check in the caller.
> > > > > 
> > > > > OK, but then do we have some handy helper for the check?
> > > > > As other bug reports by syzkaller suggest, there are a few other
> > > > > drivers that do the same, submitting a urb with naive assumption of
> > > > > the fixed EP for specific devices.  In the end we'll need to put the
> > > > > very same checks there in multiple places.
> > > > 
> > > > Perhaps we could add a helper routine that would take a list of 
> > > > expected endpoint types and check that the actual endpoints match the 
> > > > types.  But of course, all the drivers you're talking about would have 
> > > > to add a call to this helper routine.
> > > 
> > > We have almost this type of function, usb_find_common_endpoints(),
> > > what's wrong with using that?  Johan has already swept the tree and
> > > added a lot of these checks, odds are no one looked at the sound/
> > > subdir...
> 
> Yeah, I only swept the tree for instances were a missing endpoint could
> lead to a NULL-deref. This is not the case here were the endpoint
> addresses are hardcoded in the driver.
> 
> I also never got around to applying the new helper outside of
> drivers/usb.
> 
> > Well, what I had in my mind is just a snippet from usb_submit_urb(),
> > something like:
> > 
> > bool usb_sanity_check_urb_pipe(struct urb *urb)
> > {
> > struct usb_host_endpoint *ep;
> > int xfertype;
> > static const int pipetypes[4] = {
> > PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
> > };
> > 
> > ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > xfertype = usb_endpoint_type(>desc);
> > return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> > }
> > 
> > And calling this before usb_submit_urb() in each place that assigns
> > the fixed EP as device-specific quirks.
> > Does it make sense?
> 
> Not really. Your driver should not even bind to an interface which lacks
> the expected endpoints (rather than check this at a potentially later
> point in time when URBs are submitted).

The endpoint may exist but it may be invalid, as the problem is
triggered by a VM.  It doesn't parse but tries a fixed EP as it's no
compliant device.

> The new helper which Greg mentioned would allow this to implemented with
> just a few lines of code. Just add it to bcd2000_init_midi() or similar.  

Could you give an example?  Then I can ask Andrey whether such a call
really addresses the issue.


thanks,

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

Re: [PATCH 2/9 v2] usb: usb251xb: Add USB251x specific port count setting

[Richard Leitner]

On 09/16/2017 12:42 PM, Serge Semin wrote:
> USB251xb as well as USB2517 datasheet states, that all these
> hubs differ by number of ports declared as the last digit in the
> model name. So USB2512 got two ports, USB2513 - three, and so on.
> Such setting must be reflected in the device specific data
> structure and corresponding dts property should be checked whether
> it doesn't get out of available ports.
> 
> Signed-off-by: Serge Semin 
> ---
>  drivers/usb/misc/usb251xb.c | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> index 96a8c20ac..5cb0e5570 100644
> --- a/drivers/usb/misc/usb251xb.c
> +++ b/drivers/usb/misc/usb251xb.c

...

>  
> @@ -422,8 +431,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
>   for (i = 0; i < len / sizeof(u32); i++) {
>   u32 port = be32_to_cpu(cproperty_u32[i]);
>  
> - if ((port >= 1) && (port <= 4))
> + if ((port >= 1) && (port <= data->port_cnt))
>   hub->non_rem_dev |= BIT(port);
> + else
> + dev_warn(dev, "port %u doesn't exist\n", port);

I'd prefer to add the (invalid) property trying to be set in this
warning messages. So someone is able to find the invalid dt setting
faster. Something like;

dev_warn(dev, "requested NRD port %u doesn't exist\n", port);

>   }
>   }
>  
> @@ -433,8 +444,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
>   for (i = 0; i < len / sizeof(u32); i++) {
>   u32 port = be32_to_cpu(cproperty_u32[i]);
>  
> - if ((port >= 1) && (port <= 4))
> + if ((port >= 1) && (port <= data->port_cnt))
>   hub->port_disable_sp |= BIT(port);
> + else
> + dev_warn(dev, "port %u doesn't exist\n", port);

... same as above. For example:

dev_warn(dev, "requested PDS port %u doesn't exist\n", port);

>   }
>   }
>  
> @@ -444,8 +457,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
>   for (i = 0; i < len / sizeof(u32); i++) {
>   u32 port = be32_to_cpu(cproperty_u32[i]);
>  
> - if ((port >= 1) && (port <= 4))
> + if ((port >= 1) && (port <= data->port_cnt))
>   hub->port_disable_bp |= BIT(port);
> + else
> + dev_warn(dev, "port %u doesn't exist\n", port);

... same as above. For example:

dev_warn(dev, "requested PDB port %u doesn't exist\n", port);


Apart from that this patch looks fine for me. Thanks for the spot of the
hardcoded max ports check.

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

Re: [PATCH 4/9 v2] usb: usb251xb: Add 5,6,7 ports boost settings

[Richard Leitner]

On 09/16/2017 12:42 PM, Serge Semin wrote:
> USB electrical signaling drive strength boost bit is also supported
> by USB2517 hub. Since it got three addition ports, the designers
> needed to add one more register for initialization. It turned out
> to be formerly reserved 0xF7. As before we just initialize it with
> default zeros.
> 
> Signed-off-by: Serge Semin 
> ---
>  drivers/usb/misc/usb251xb.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 

Looks good to me. Feel free to add:

Acked-by: Richard Leitner 

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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Johan Hovold]

On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> On Tue, 03 Oct 2017 19:42:21 +0200,
> Greg Kroah-Hartman wrote:
> > 
> > On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > > 
> > > > > It's a dev_WARN because it indicates a potentially serious error in 
> > > > > the 
> > > > > driver: The driver has submitted an interrupt URB to a bulk endpoint. 
> > > > >  
> > > > > That may not sound bad, but the same check gets triggered if a driver 
> > > > > submits a bulk URB to an isochronous endpoint, or any other invalid 
> > > > > combination.
> > > > > 
> > > > > Most likely the explanation here is that the driver doesn't bother to
> > > > > check the endpoint type because it expects the endpoint will always be
> > > > > interrupt.  But that is not a safe strategy.  USB devices and their
> > > > > firmware should not be trusted unnecessarily.
> > > > > 
> > > > > The best fix is, like you said, to add a sanity check in the caller.
> > > > 
> > > > OK, but then do we have some handy helper for the check?
> > > > As other bug reports by syzkaller suggest, there are a few other
> > > > drivers that do the same, submitting a urb with naive assumption of
> > > > the fixed EP for specific devices.  In the end we'll need to put the
> > > > very same checks there in multiple places.
> > > 
> > > Perhaps we could add a helper routine that would take a list of 
> > > expected endpoint types and check that the actual endpoints match the 
> > > types.  But of course, all the drivers you're talking about would have 
> > > to add a call to this helper routine.
> > 
> > We have almost this type of function, usb_find_common_endpoints(),
> > what's wrong with using that?  Johan has already swept the tree and
> > added a lot of these checks, odds are no one looked at the sound/
> > subdir...

Yeah, I only swept the tree for instances were a missing endpoint could
lead to a NULL-deref. This is not the case here were the endpoint
addresses are hardcoded in the driver.

I also never got around to applying the new helper outside of
drivers/usb.

> Well, what I had in my mind is just a snippet from usb_submit_urb(),
> something like:
> 
> bool usb_sanity_check_urb_pipe(struct urb *urb)
> {
>   struct usb_host_endpoint *ep;
>   int xfertype;
>   static const int pipetypes[4] = {
>   PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
>   };
> 
>   ep = usb_pipe_endpoint(urb->dev, urb->pipe);
>   xfertype = usb_endpoint_type(>desc);
>   return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> }
> 
> And calling this before usb_submit_urb() in each place that assigns
> the fixed EP as device-specific quirks.
> Does it make sense?

Not really. Your driver should not even bind to an interface which lacks
the expected endpoints (rather than check this at a potentially later
point in time when URBs are submitted).

The new helper which Greg mentioned would allow this to implemented with
just a few lines of code. Just add it to bcd2000_init_midi() or similar.  

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

Re: Random xHCI HC died on device disconnect

[Mathias Nyman]

On 03.10.2017 18:27, Kristian Evensen wrote:

On Tue, Oct 3, 2017 at 4:51 PM, Kristian Evensen
 wrote:

Disabling the timer caused a different error to be triggered. Instead
of "HC died...", I now get the following message looping over and
over:

[16870.871935] qmi_wwan 4-1:1.4: Tx URB error: -19


I was not thinking clearly when I wrote the email. The reason this
message kept looping over and over was that I kept trying to
communicate with the modem. When I stopped my tool, the message
stopped. However, the host controller is still dead, so even if I try
to disconnect the device using GPIO, nothing happens. I thought the
log of when the error occurred was lost due to limited buffers, but I
was wrong and I have the following in dmesg when the error strikes:

[15986.400431] usb 4-1: USB disconnect, device number 13
[15986.400454] qmi_wwan 4-1:1.4: nonzero urb status received: -71
[15986.411366] qmi_wwan 4-1:1.4: wdm_int_callback - 0 bytes
[15986.416692] qmi_wwan 4-1:1.4: wdm_int_callback - usb_submit_urb
failed with result -19
[15986.424816] option1 ttyUSB0: GSM modem (1-port) converter now
disconnected from ttyUSB0
[15986.432886] option 4-1:1.0: device disconnected
[15986.437647] option1 ttyUSB1: GSM modem (1-port) converter now
disconnected from ttyUSB1
[15986.445765] option 4-1:1.1: device disconnected
[16001.110698] xhci-hcd f10f8000.usb3: Stopped the command ring
failed, maybe the host is dead
[16001.119077] xhci-hcd f10f8000.usb3: Abort command ring failed
[16001.124949] xhci-hcd f10f8000.usb3: HC died; cleaning up
[16100.854819] qmi_wwan 4-1:1.4: Tx URB error: -19
[16105.854944] qmi_wwan 4-1:1.4: Tx URB error: -19
[16110.855052] qmi_wwan 4-1:1.4: Tx URB error: -19
[16115.855159] qmi_wwan 4-1:1.4: Tx URB error: -19
[16120.855285] qmi_wwan 4-1:1.4: Tx URB error: -19
[16125.855396] qmi_wwan 4-1:1.4: Tx URB error: -19

I am bit surprised to see the HC-related messages. Maybe I missed a
place where the timer is stopped or something, time to investigate!



This is the xhci->cmd_timer (delayed work) that has a five second timeout
for the currently processing command on the command ring.
When triggered it will abort the current command by stopping the command ring
and remove/move past the current command.

Logs shows the command first timed out, and xhci then failing to stop the 
command ring.
when trying to abort the command.

To me it looks like xHC ends up in a state that we can't recover from without 
resetting xHC.
xhci Module reload or rebinding device and driver is needed

-Mathias


 


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

Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support

[Richard Leitner]

On 09/21/2017 07:10 PM, Serge Semin wrote:
> On Thu, Sep 21, 2017 at 11:26:04AM -0500, Rob Herring  wrote:
>> On Wed, Sep 20, 2017 at 4:27 PM, Serge Semin  wrote:

...

>>> These are different parameters of the device. They got different 
>>> configuration
>>> registers and descriptions:
>>> max_power* - ... This value also includes the power consumption of a
>>> permanently attached peripheral if the hub is configured as a compound
>>> device, and the embedded peripheral reports 0mA in its descriptors.
>>> max_current* - ... This value does NOT include the power consumption of a
>>> permanently attached peripheral if the hub is configured as a compound
>>> device.
>>
>> Then the names here should somehow reflect the above. Perhaps
>> "composite-current" and "hub-current" or something like that.
>>
> 
> I left the naming in accordance with the device datasheet. I thought it would 
> be
> better since the driver user would still need to consult with the device
> documentation to properly set them. I don't really get how the difference is 
> reflected
> with the naming declared there though. So what naming would you prefer then? 
> Might be
> something like:
> {sp,bp}-max-total-current - for so named {sp,bp}-max-power, since it includes 
> all the
> permanently attached peripherals.
> {sp,bp}-max-removable-current - for so named {sp,bp}-max-current, since it 
> doesn't
> include the permanently attached peripherals.
> 
> Or is it better to leave it in compliance with the documentation naming?

I'd prefer the naming to be in accordance with the device datasheet too.
Changing it will IMHO generate avoidable misunderstandings...

But if we should go with Robs proposal please make sure the name from
the device datasheet is mentioned somewhere in the description of the
binding.

> 
>>>
>>> Additionally as you can see, they both are measured in "mA", so it isn't
>>> a real physical power.
>>
>> Well, I can't because there's no units.
>>
> 
> What this line means then?
> - sp-max-{power,current} : ... The value is given in mA in a 0 - 100 range 
> (default is 1mA).
> - bp-max-{power,current} : ... The value is given in mA in a 0 - 510 range 
> (default is 100mA).
> 
> Maybe I don't know something and the description line should state the units 
> somehow
> clearer?

Append the unit to the binding name, just like in "power-on-time-ms". As
there's no "Standard Unit Suffix" for mA stated in the documentation
(Documentation/devicetree/bindings/property-units.txt) I think it should
be "-milliamp"?

regards,
Richard.L


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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Greg Kroah-Hartman]

On Wed, Oct 04, 2017 at 10:08:24AM +0200, Takashi Iwai wrote:
> On Wed, 04 Oct 2017 09:52:36 +0200,
> Greg Kroah-Hartman wrote:
> > 
> > On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> > > On Tue, 03 Oct 2017 19:42:21 +0200,
> > > Greg Kroah-Hartman wrote:
> > > > 
> > > > On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > > > > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > > > > 
> > > > > > > It's a dev_WARN because it indicates a potentially serious error 
> > > > > > > in the 
> > > > > > > driver: The driver has submitted an interrupt URB to a bulk 
> > > > > > > endpoint.  
> > > > > > > That may not sound bad, but the same check gets triggered if a 
> > > > > > > driver 
> > > > > > > submits a bulk URB to an isochronous endpoint, or any other 
> > > > > > > invalid 
> > > > > > > combination.
> > > > > > > 
> > > > > > > Most likely the explanation here is that the driver doesn't 
> > > > > > > bother to
> > > > > > > check the endpoint type because it expects the endpoint will 
> > > > > > > always be
> > > > > > > interrupt.  But that is not a safe strategy.  USB devices and 
> > > > > > > their
> > > > > > > firmware should not be trusted unnecessarily.
> > > > > > > 
> > > > > > > The best fix is, like you said, to add a sanity check in the 
> > > > > > > caller.
> > > > > > 
> > > > > > OK, but then do we have some handy helper for the check?
> > > > > > As other bug reports by syzkaller suggest, there are a few other
> > > > > > drivers that do the same, submitting a urb with naive assumption of
> > > > > > the fixed EP for specific devices.  In the end we'll need to put the
> > > > > > very same checks there in multiple places.
> > > > > 
> > > > > Perhaps we could add a helper routine that would take a list of 
> > > > > expected endpoint types and check that the actual endpoints match the 
> > > > > types.  But of course, all the drivers you're talking about would 
> > > > > have 
> > > > > to add a call to this helper routine.
> > > > 
> > > > We have almost this type of function, usb_find_common_endpoints(),
> > > > what's wrong with using that?  Johan has already swept the tree and
> > > > added a lot of these checks, odds are no one looked at the sound/
> > > > subdir...
> > > 
> > > Well, what I had in my mind is just a snippet from usb_submit_urb(),
> > > something like:
> > > 
> > > bool usb_sanity_check_urb_pipe(struct urb *urb)
> > > {
> > >   struct usb_host_endpoint *ep;
> > >   int xfertype;
> > >   static const int pipetypes[4] = {
> > >   PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
> > >   };
> > > 
> > >   ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > >   xfertype = usb_endpoint_type(>desc);
> > >   return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> > > }
> > > 
> > > And calling this before usb_submit_urb() in each place that assigns
> > > the fixed EP as device-specific quirks.
> > > Does it make sense?
> > 
> > Yes, kind of, but checking the endpoint type/direction is what you are
> > expecting it to be as you "know" what the type should be for each
> > driver as it is unique.
> 
> Yes, it can be simplified, but if we want a common helper function,
> this style would have an advantage that it can be used generically for
> all drivers.
> 
> > Anyway, a "real" patch might make more sense to me.
> 
> I can cook up a patch if you find it a good idea to add such a common
> function to usb core side.  OTOH, if each driver should open-code this
> in each place, I can work on that, too.  Which would you prefer?

A common function is good, open-coding is bad :)

Try it with a driver or two to see what it looks like?

thanks,

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

Re: [PATCH 1/9 v2] usb: usb251xb: Add USB2517i specific struct and IDs

[Richard Leitner]

Hi Serge,

On 09/16/2017 12:42 PM, Serge Semin wrote:
> There are USB2517 and USB2517i hubs, which have almost the same
> registers space as already supported USB251xbi series. The difference
> it in DIDs and in few functions. This patch adds the USB2517/i data
> structures to the driver, so it would have different setting depending
> on the device discovered on i2c-bus.
> 
> Signed-off-by: Serge Semin 
> ---
>  Documentation/devicetree/bindings/usb/usb251xb.txt  |  3 ++-
>  drivers/usb/misc/usb251xb.c | 21 
> -
>  2 files changed, 22 insertions(+), 2 deletions(-)

...

Please also mention support for the USB2517(i) hubs in the Kconfig
helptext of the driver @ drivers/usb/misc/Kconfig. Thanks.

Apart from that the patch looks good to me.

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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Greg Kroah-Hartman]

On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> On Tue, 03 Oct 2017 19:42:21 +0200,
> Greg Kroah-Hartman wrote:
> > 
> > On Tue, Oct 03, 2017 at 12:50:08PM -0400, Alan Stern wrote:
> > > On Tue, 3 Oct 2017, Takashi Iwai wrote:
> > > 
> > > > > It's a dev_WARN because it indicates a potentially serious error in 
> > > > > the 
> > > > > driver: The driver has submitted an interrupt URB to a bulk endpoint. 
> > > > >  
> > > > > That may not sound bad, but the same check gets triggered if a driver 
> > > > > submits a bulk URB to an isochronous endpoint, or any other invalid 
> > > > > combination.
> > > > > 
> > > > > Most likely the explanation here is that the driver doesn't bother to
> > > > > check the endpoint type because it expects the endpoint will always be
> > > > > interrupt.  But that is not a safe strategy.  USB devices and their
> > > > > firmware should not be trusted unnecessarily.
> > > > > 
> > > > > The best fix is, like you said, to add a sanity check in the caller.
> > > > 
> > > > OK, but then do we have some handy helper for the check?
> > > > As other bug reports by syzkaller suggest, there are a few other
> > > > drivers that do the same, submitting a urb with naive assumption of
> > > > the fixed EP for specific devices.  In the end we'll need to put the
> > > > very same checks there in multiple places.
> > > 
> > > Perhaps we could add a helper routine that would take a list of 
> > > expected endpoint types and check that the actual endpoints match the 
> > > types.  But of course, all the drivers you're talking about would have 
> > > to add a call to this helper routine.
> > 
> > We have almost this type of function, usb_find_common_endpoints(),
> > what's wrong with using that?  Johan has already swept the tree and
> > added a lot of these checks, odds are no one looked at the sound/
> > subdir...
> 
> Well, what I had in my mind is just a snippet from usb_submit_urb(),
> something like:
> 
> bool usb_sanity_check_urb_pipe(struct urb *urb)
> {
>   struct usb_host_endpoint *ep;
>   int xfertype;
>   static const int pipetypes[4] = {
>   PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
>   };
> 
>   ep = usb_pipe_endpoint(urb->dev, urb->pipe);
>   xfertype = usb_endpoint_type(>desc);
>   return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> }
> 
> And calling this before usb_submit_urb() in each place that assigns
> the fixed EP as device-specific quirks.
> Does it make sense?

Yes, kind of, but checking the endpoint type/direction is what you are
expecting it to be as you "know" what the type should be for each
driver as it is unique.

Anyway, a "real" patch might make more sense to me.

thanks,

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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Johan Hovold]

On Wed, Oct 04, 2017 at 12:41:55PM +0200, Takashi Iwai wrote:
> On Wed, 04 Oct 2017 12:23:11 +0200,
> Johan Hovold wrote:
> > 
> > On Wed, Oct 04, 2017 at 12:04:06PM +0200, Takashi Iwai wrote:
> > > On Wed, 04 Oct 2017 11:24:42 +0200, Johan Hovold wrote:
> > > > On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> > 
> > > > > Well, what I had in my mind is just a snippet from usb_submit_urb(),
> > > > > something like:
> > > > > 
> > > > > bool usb_sanity_check_urb_pipe(struct urb *urb)
> > > > > {
> > > > >   struct usb_host_endpoint *ep;
> > > > >   int xfertype;
> > > > >   static const int pipetypes[4] = {
> > > > >   PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, 
> > > > > PIPE_INTERRUPT
> > > > >   };
> > > > > 
> > > > >   ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > > > >   xfertype = usb_endpoint_type(>desc);
> > > > >   return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> > > > > }
> > > > > 
> > > > > And calling this before usb_submit_urb() in each place that assigns
> > > > > the fixed EP as device-specific quirks.
> > > > > Does it make sense?
> > > > 
> > > > Not really. Your driver should not even bind to an interface which lacks
> > > > the expected endpoints (rather than check this at a potentially later
> > > > point in time when URBs are submitted).
> > > 
> > > The endpoint may exist but it may be invalid, as the problem is
> > > triggered by a VM.  It doesn't parse but tries a fixed EP as it's no
> > > compliant device.
> > 
> > Yes, that's why a driver should verify that the endpoints it expects are
> > indeed present (and of the right type) already at probe.
> > 
> > In Andrey's fuzzing it's triggered by in a VM using the dummy_hcd
> > driver, but this could just as well be a (malicious) physical device
> > with unexpected descriptors.
> > 
> > > > The new helper which Greg mentioned would allow this to implemented with
> > > > just a few lines of code. Just add it to bcd2000_init_midi() or 
> > > > similar.  
> > > 
> > > Could you give an example?  Then I can ask Andrey whether such a call
> > > really addresses the issue.
> > 
> > If you grep for usb_find_common_endpoints you'll find a few examples
> > of how that function may be used (e.g. in drivers/usb/misc/usblcd.c).
> > 
> > The helper iterates of the endpoint descriptors of an interface
> > alt-setting and returns a descriptor for each requested type if found.
> > After a vetting of our current drivers I concluded that this would
> > cover the needs of the vast majority of drivers.
> > 
> > So for the driver in question you'd only need to add something like:
> > 
> > struct usb_endpoint_descriptor *int_in, *int_out;
> > int ret;
> > 
> > ret = usb_find_common_endpoints(interface->cur_altsetting,
> > NULL, NULL, _in, _out);
> > if (ret) {
> > dev_err(>dev, "required endpoints not found\n");
> > return -ENODEV;
> > }
> > 
> > Then you can use int_in->bEndpointAddress etc. when initialising your
> > URBs.
> 
> OK, but in our cases, it's not about using the returned one but
> checking whether it's the expected address, right?  The device is
> non-compliant and that's the reason the driver takes the fixed EP.

There's nothing preventing you from verifying that the returned
descriptors have the expected addresses if tightening the constraints
this ways makes sense for your application.

Or you can implement your own sanity checks, just do it at probe.

But note that you'd introduce NULL-deref that can be triggered by a
malicious device with your outlined helper above, as

ep = usb_pipe_endpoint(urb->dev, urb->pipe);

will be NULL when the corresponding descriptor is missing.

> In anyway, the check will be shortly before the URB submission because
> the EP is often determined a late stage of probe, as most of errors
> happened for the MIDI interface that are device-specific.

As long as you do the check during probe and refuse to bind to a
non-compliant device you should be fine. Some drivers do not submit URBs
until the user tries to use whatever interface the driver exposes (e.g.
when opening a character device), which IMO is too late for such sanity
checks.

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

[PATCH] usb: gadget: udc: renesas_usb3: Use of_device_get_match_data() helper

[Geert Uytterhoeven]

Use the of_device_get_match_data() helper instead of open coding,
postponing the matching until when it's really needed.
Note that the renesas_usb3 driver is used with DT only, so there's
always a valid match.

Signed-off-by: Geert Uytterhoeven 
---
 drivers/usb/gadget/udc/renesas_usb3.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index df37c1e6e9d5cc3e..acbf0feb47c85942 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2549,20 +2549,15 @@ static int renesas_usb3_probe(struct platform_device 
*pdev)
 {
struct renesas_usb3 *usb3;
struct resource *res;
-   const struct of_device_id *match;
int irq, ret;
const struct renesas_usb3_priv *priv;
const struct soc_device_attribute *attr;
 
-   match = of_match_node(usb3_of_match, pdev->dev.of_node);
-   if (!match)
-   return -ENODEV;
-
attr = soc_device_match(renesas_usb3_quirks_match);
if (attr)
priv = attr->data;
else
-   priv = match->data;
+   priv = of_device_get_match_data(>dev);
 
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
-- 
2.7.4

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

Re: [PATCH v4 2/3] usb: host: add a generic platform USB roothub driver

[Mathias Nyman]

On 04.09.2017 00:38, Martin Blumenstingl wrote:

Many SoC platforms have separate devices for the USB PHY which are
registered through the generic PHY framework. These PHYs have to be
enabled to make the USB controller actually work. They also have to be
disabled again on shutdown/suspend.

Currently (at least) the following HCI platform drivers are using custom
code to obtain all PHYs via devicetree for the roothub/controller and
disable/enable them when required:
- ehci-platform.c has ehci_platform_power_{on,off}
- xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
- ohci-platform.c has ohci_platform_power_{on,off}

These drivers are not using the generic devicetree USB device bindings
yet which were only introduced recently (documentation is available in
devicetree/bindings/usb/usb-device.txt).
With this new driver the usb2-phy and usb3-phy can be specified directly
in the child-node of the corresponding port of the roothub via
devicetree. This can be extended by not just parsing PHYs (some of the
other drivers listed above are for example also parsing a list of clocks
as well) when required.


usb_add_hcd() in usb/core/hcd.c is already finding, initializing and turning
on a phy, would it make sense to expand that one to support several phys 
instead?

xhci will add two hcd's one for USB2 and one for USB3



Signed-off-by: Martin Blumenstingl 
Tested-by: Chunfeng Yun 
---
  drivers/usb/host/Kconfig|   3 +
  drivers/usb/host/Makefile   |   2 +
  drivers/usb/host/platform-roothub.c | 180 
  drivers/usb/host/platform-roothub.h |  12 +++
  4 files changed, 197 insertions(+)
  create mode 100644 drivers/usb/host/platform-roothub.c
  create mode 100644 drivers/usb/host/platform-roothub.h



Instead of creating  platform-roothub files could this content
be added into into core/hcd.*, core/phy.* and host/xhci-plat.c


diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index fa5692dec832..b8b05c786b2a 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -805,6 +805,9 @@ config USB_HCD_SSB

  If unsure, say N.

+config USB_PLATFORM_ROOTHUB
+   bool
+
  config USB_HCD_TEST_MODE
bool "HCD test mode support"
---help---
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index cf2691fffcc0..dc817f82d632 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD)+= whci/

  obj-$(CONFIG_USB_PCI) += pci-quirks.o

+obj-$(CONFIG_USB_PLATFORM_ROOTHUB) += platform-roothub.o
+
  obj-$(CONFIG_USB_EHCI_HCD)+= ehci-hcd.o
  obj-$(CONFIG_USB_EHCI_PCI)+= ehci-pci.o
  obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)   += ehci-platform.o
diff --git a/drivers/usb/host/platform-roothub.c 
b/drivers/usb/host/platform-roothub.c
new file mode 100644
index ..70d2d97aa8b2
--- /dev/null
+++ b/drivers/usb/host/platform-roothub.c
@@ -0,0 +1,180 @@
+/*
+ * platform roothub driver - a virtual PHY device which passes all phy_*
+ * function calls to multiple (actual) PHY devices. This is comes handy when
+ * initializing all PHYs on a root-hub (to keep them all in the same state).
+ *
+ * Copyright (C) 2017 Martin Blumenstingl 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "platform-roothub.h"
+
+#define ROOTHUB_PORTNUM0
+
+struct platform_roothub {
+   struct phy  *phy;
+   struct list_headlist;
+};
+
+static struct platform_roothub *platform_roothub_alloc(struct device *dev)
+{
+   struct platform_roothub *roothub_entry;
+
+   roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
+   if (!roothub_entry)
+   return ERR_PTR(-ENOMEM);
+
+   INIT_LIST_HEAD(_entry->list);
+
+   return roothub_entry;
+}
+
+static int platform_roothub_add_phy(struct device *dev,
+   struct device_node *port_np,
+   const char *con_id, struct list_head *list)
+{
+   struct platform_roothub *roothub_entry;
+   struct phy *phy = devm_of_phy_get(dev, port_np, con_id);
+
+   if (IS_ERR_OR_NULL(phy)) {
+   if (!phy || PTR_ERR(phy) == -ENODEV)
+   return 0;
+   else
+   return PTR_ERR(phy);
+   }
+
+   roothub_entry = platform_roothub_alloc(dev);
+   if (IS_ERR(roothub_entry))
+   return PTR_ERR(roothub_entry);
+
+   roothub_entry->phy = phy;
+
+ 

Re: Huawei integrated modem causes instant resume from suspend on Acer P648-G3

[Oliver Neukum]

Am Montag, den 02.10.2017, 10:23 +0800 schrieb Daniel Drake:
> On Wed, Sep 27, 2017 at 3:12 PM, Oliver Neukum  wrote:
> > 
> > 1) kernel version?
> 
> Currently testing 4.14.0-rc2 but reproduced on multiple older versions
> too going back to 4.8. (Have not found a working kernel version)
> 
> > 
> > 2) driver used?
> 
> The problem is reproducible just with the base usb driver, without any
> USB modem drivers loaded. (It would ordinarily load option and
> cdc-ether)

Hi,

this sucks in a major way. I am dismayed at the quality of some hardware.
It looks like your patch, which is a brutal sledge hammer, is indeed
needed and the correct approach.

It suggest you resubmit it with an updated change log containing
the what you wrote in your last mail.

Regards
Oliver

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

Re: [PATCH 8/9 v2] usb: usb251xb: Add max power/current dts property support

[Rob Herring]

On Wed, Oct 4, 2017 at 3:12 AM, Richard Leitner
 wrote:
>
> On 09/21/2017 07:10 PM, Serge Semin wrote:
>> On Thu, Sep 21, 2017 at 11:26:04AM -0500, Rob Herring  
>> wrote:
>>> On Wed, Sep 20, 2017 at 4:27 PM, Serge Semin  
>>> wrote:
>
> ...
>
 These are different parameters of the device. They got different 
 configuration
 registers and descriptions:
 max_power* - ... This value also includes the power consumption of a
 permanently attached peripheral if the hub is configured as a compound
 device, and the embedded peripheral reports 0mA in its descriptors.
 max_current* - ... This value does NOT include the power consumption of a
 permanently attached peripheral if the hub is configured as a compound
 device.
>>>
>>> Then the names here should somehow reflect the above. Perhaps
>>> "composite-current" and "hub-current" or something like that.
>>>
>>
>> I left the naming in accordance with the device datasheet. I thought it 
>> would be
>> better since the driver user would still need to consult with the device
>> documentation to properly set them. I don't really get how the difference is 
>> reflected
>> with the naming declared there though. So what naming would you prefer then? 
>> Might be
>> something like:
>> {sp,bp}-max-total-current - for so named {sp,bp}-max-power, since it 
>> includes all the
>> permanently attached peripherals.
>> {sp,bp}-max-removable-current - for so named {sp,bp}-max-current, since it 
>> doesn't
>> include the permanently attached peripherals.
>>
>> Or is it better to leave it in compliance with the documentation naming?
>
> I'd prefer the naming to be in accordance with the device datasheet too.
> Changing it will IMHO generate avoidable misunderstandings...

Okay, then add a vendor prefix.

> But if we should go with Robs proposal please make sure the name from
> the device datasheet is mentioned somewhere in the description of the
> binding.
>
>>

 Additionally as you can see, they both are measured in "mA", so it isn't
 a real physical power.
>>>
>>> Well, I can't because there's no units.
>>>
>>
>> What this line means then?
>> - sp-max-{power,current} : ... The value is given in mA in a 0 - 100 range 
>> (default is 1mA).
>> - bp-max-{power,current} : ... The value is given in mA in a 0 - 510 range 
>> (default is 100mA).
>>
>> Maybe I don't know something and the description line should state the units 
>> somehow
>> clearer?

The reason we have units in the name is so we don't have to go lookup
the documentation to find the units and so people don't use the same
property name with different units.

> Append the unit to the binding name, just like in "power-on-time-ms". As
> there's no "Standard Unit Suffix" for mA stated in the documentation
> (Documentation/devicetree/bindings/property-units.txt) I think it should
> be "-milliamp"?

The reason being is we align around microamps and try to avoid
everyone picking their own units (celliamps anyone?). So unless
microamps is not enough range for you, I would stick with that.

If you have both units and vendor prefix, then I care less about the
poorly written datasheet naming used.

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

Re: [PATCH] rndis_host: support Novatel Verizon USB730L

[Oliver Neukum]

Am Dienstag, den 03.10.2017, 16:01 +0200 schrieb Bjørn Mork:
> Adding anything else, e.g. based on the table at
> http://www.usb.org/developers/defined_class/#BaseClassEFh , is a bit
> more risky.  We don't know if a driver will work with *any* such device
> until we've actually seen one.
> 
> This is just my opinion, and probably full of bogus assumptions as
> usual.  I was sort of hoping that some expert would speak up so I didn't
> have to :-)

Outside the vendors, there is nobody.

Regards
Oliver

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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Takashi Iwai]

On Wed, 04 Oct 2017 14:03:25 +0200,
Johan Hovold wrote:
> 
> On Wed, Oct 04, 2017 at 12:41:55PM +0200, Takashi Iwai wrote:
> > On Wed, 04 Oct 2017 12:23:11 +0200,
> > Johan Hovold wrote:
> > > 
> > > On Wed, Oct 04, 2017 at 12:04:06PM +0200, Takashi Iwai wrote:
> > > > On Wed, 04 Oct 2017 11:24:42 +0200, Johan Hovold wrote:
> > > > > On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> > > 
> > > > > > Well, what I had in my mind is just a snippet from usb_submit_urb(),
> > > > > > something like:
> > > > > > 
> > > > > > bool usb_sanity_check_urb_pipe(struct urb *urb)
> > > > > > {
> > > > > > struct usb_host_endpoint *ep;
> > > > > > int xfertype;
> > > > > > static const int pipetypes[4] = {
> > > > > > PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, 
> > > > > > PIPE_INTERRUPT
> > > > > > };
> > > > > > 
> > > > > > ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > > > > > xfertype = usb_endpoint_type(>desc);
> > > > > > return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> > > > > > }
> > > > > > 
> > > > > > And calling this before usb_submit_urb() in each place that assigns
> > > > > > the fixed EP as device-specific quirks.
> > > > > > Does it make sense?
> > > > > 
> > > > > Not really. Your driver should not even bind to an interface which 
> > > > > lacks
> > > > > the expected endpoints (rather than check this at a potentially later
> > > > > point in time when URBs are submitted).
> > > > 
> > > > The endpoint may exist but it may be invalid, as the problem is
> > > > triggered by a VM.  It doesn't parse but tries a fixed EP as it's no
> > > > compliant device.
> > > 
> > > Yes, that's why a driver should verify that the endpoints it expects are
> > > indeed present (and of the right type) already at probe.
> > > 
> > > In Andrey's fuzzing it's triggered by in a VM using the dummy_hcd
> > > driver, but this could just as well be a (malicious) physical device
> > > with unexpected descriptors.
> > > 
> > > > > The new helper which Greg mentioned would allow this to implemented 
> > > > > with
> > > > > just a few lines of code. Just add it to bcd2000_init_midi() or 
> > > > > similar.  
> > > > 
> > > > Could you give an example?  Then I can ask Andrey whether such a call
> > > > really addresses the issue.
> > > 
> > > If you grep for usb_find_common_endpoints you'll find a few examples
> > > of how that function may be used (e.g. in drivers/usb/misc/usblcd.c).
> > > 
> > > The helper iterates of the endpoint descriptors of an interface
> > > alt-setting and returns a descriptor for each requested type if found.
> > > After a vetting of our current drivers I concluded that this would
> > > cover the needs of the vast majority of drivers.
> > > 
> > > So for the driver in question you'd only need to add something like:
> > > 
> > >   struct usb_endpoint_descriptor *int_in, *int_out;
> > >   int ret;
> > > 
> > >   ret = usb_find_common_endpoints(interface->cur_altsetting,
> > >   NULL, NULL, _in, _out);
> > >   if (ret) {
> > >   dev_err(>dev, "required endpoints not found\n");
> > >   return -ENODEV;
> > >   }
> > > 
> > > Then you can use int_in->bEndpointAddress etc. when initialising your
> > > URBs.
> > 
> > OK, but in our cases, it's not about using the returned one but
> > checking whether it's the expected address, right?  The device is
> > non-compliant and that's the reason the driver takes the fixed EP.
> 
> There's nothing preventing you from verifying that the returned
> descriptors have the expected addresses if tightening the constraints
> this ways makes sense for your application.

OK.

> Or you can implement your own sanity checks, just do it at probe.
> 
> But note that you'd introduce NULL-deref that can be triggered by a
> malicious device with your outlined helper above, as
> 
>   ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> 
> will be NULL when the corresponding descriptor is missing.

Yes, if we do that.  But I'm working on a version using
usb_find_*_endpoint*() instead.

> > In anyway, the check will be shortly before the URB submission because
> > the EP is often determined a late stage of probe, as most of errors
> > happened for the MIDI interface that are device-specific.
> 
> As long as you do the check during probe and refuse to bind to a
> non-compliant device you should be fine. Some drivers do not submit URBs
> until the user tries to use whatever interface the driver exposes (e.g.
> when opening a character device), which IMO is too late for such sanity
> checks.

Right, and this won't be a problem as the issue is triggered before
the actual device registration (ALSA has a staged registration
scheme).


thanks,

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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Johan Hovold]

On Wed, Oct 04, 2017 at 12:04:06PM +0200, Takashi Iwai wrote:
> On Wed, 04 Oct 2017 11:24:42 +0200, Johan Hovold wrote:
> > On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:

> > > Well, what I had in my mind is just a snippet from usb_submit_urb(),
> > > something like:
> > > 
> > > bool usb_sanity_check_urb_pipe(struct urb *urb)
> > > {
> > >   struct usb_host_endpoint *ep;
> > >   int xfertype;
> > >   static const int pipetypes[4] = {
> > >   PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT
> > >   };
> > > 
> > >   ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > >   xfertype = usb_endpoint_type(>desc);
> > >   return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> > > }
> > > 
> > > And calling this before usb_submit_urb() in each place that assigns
> > > the fixed EP as device-specific quirks.
> > > Does it make sense?
> > 
> > Not really. Your driver should not even bind to an interface which lacks
> > the expected endpoints (rather than check this at a potentially later
> > point in time when URBs are submitted).
> 
> The endpoint may exist but it may be invalid, as the problem is
> triggered by a VM.  It doesn't parse but tries a fixed EP as it's no
> compliant device.

Yes, that's why a driver should verify that the endpoints it expects are
indeed present (and of the right type) already at probe.

In Andrey's fuzzing it's triggered by in a VM using the dummy_hcd
driver, but this could just as well be a (malicious) physical device
with unexpected descriptors.

> > The new helper which Greg mentioned would allow this to implemented with
> > just a few lines of code. Just add it to bcd2000_init_midi() or similar.  
> 
> Could you give an example?  Then I can ask Andrey whether such a call
> really addresses the issue.

If you grep for usb_find_common_endpoints you'll find a few examples
of how that function may be used (e.g. in drivers/usb/misc/usblcd.c).

The helper iterates of the endpoint descriptors of an interface
alt-setting and returns a descriptor for each requested type if found.
After a vetting of our current drivers I concluded that this would
cover the needs of the vast majority of drivers.

So for the driver in question you'd only need to add something like:

struct usb_endpoint_descriptor *int_in, *int_out;
int ret;

ret = usb_find_common_endpoints(interface->cur_altsetting,
NULL, NULL, _in, _out);
if (ret) {
dev_err(>dev, "required endpoints not found\n");
return -ENODEV;
}

Then you can use int_in->bEndpointAddress etc. when initialising your
URBs.

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

Re: usb/sound/bcd2000: warning in bcd2000_init_device

[Takashi Iwai]

On Wed, 04 Oct 2017 12:23:11 +0200,
Johan Hovold wrote:
> 
> On Wed, Oct 04, 2017 at 12:04:06PM +0200, Takashi Iwai wrote:
> > On Wed, 04 Oct 2017 11:24:42 +0200, Johan Hovold wrote:
> > > On Wed, Oct 04, 2017 at 08:10:59AM +0200, Takashi Iwai wrote:
> 
> > > > Well, what I had in my mind is just a snippet from usb_submit_urb(),
> > > > something like:
> > > > 
> > > > bool usb_sanity_check_urb_pipe(struct urb *urb)
> > > > {
> > > > struct usb_host_endpoint *ep;
> > > > int xfertype;
> > > > static const int pipetypes[4] = {
> > > > PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, 
> > > > PIPE_INTERRUPT
> > > > };
> > > > 
> > > > ep = usb_pipe_endpoint(urb->dev, urb->pipe);
> > > > xfertype = usb_endpoint_type(>desc);
> > > > return usb_pipetype(urb->pipe) != pipetypes[xfertype];
> > > > }
> > > > 
> > > > And calling this before usb_submit_urb() in each place that assigns
> > > > the fixed EP as device-specific quirks.
> > > > Does it make sense?
> > > 
> > > Not really. Your driver should not even bind to an interface which lacks
> > > the expected endpoints (rather than check this at a potentially later
> > > point in time when URBs are submitted).
> > 
> > The endpoint may exist but it may be invalid, as the problem is
> > triggered by a VM.  It doesn't parse but tries a fixed EP as it's no
> > compliant device.
> 
> Yes, that's why a driver should verify that the endpoints it expects are
> indeed present (and of the right type) already at probe.
> 
> In Andrey's fuzzing it's triggered by in a VM using the dummy_hcd
> driver, but this could just as well be a (malicious) physical device
> with unexpected descriptors.
> 
> > > The new helper which Greg mentioned would allow this to implemented with
> > > just a few lines of code. Just add it to bcd2000_init_midi() or similar.  
> > 
> > Could you give an example?  Then I can ask Andrey whether such a call
> > really addresses the issue.
> 
> If you grep for usb_find_common_endpoints you'll find a few examples
> of how that function may be used (e.g. in drivers/usb/misc/usblcd.c).
> 
> The helper iterates of the endpoint descriptors of an interface
> alt-setting and returns a descriptor for each requested type if found.
> After a vetting of our current drivers I concluded that this would
> cover the needs of the vast majority of drivers.
> 
> So for the driver in question you'd only need to add something like:
> 
>   struct usb_endpoint_descriptor *int_in, *int_out;
>   int ret;
> 
>   ret = usb_find_common_endpoints(interface->cur_altsetting,
>   NULL, NULL, _in, _out);
>   if (ret) {
>   dev_err(>dev, "required endpoints not found\n");
>   return -ENODEV;
>   }
> 
> Then you can use int_in->bEndpointAddress etc. when initialising your
> URBs.

OK, but in our cases, it's not about using the returned one but
checking whether it's the expected address, right?  The device is
non-compliant and that's the reason the driver takes the fixed EP.

In anyway, the check will be shortly before the URB submission because
the EP is often determined a late stage of probe, as most of errors
happened for the MIDI interface that are device-specific.


thanks,

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

[PATCH] usb: renesas_usbhs: Use of_device_get_match_data() helper

[Geert Uytterhoeven]

Use the of_device_get_match_data() helper instead of open coding.

Signed-off-by: Geert Uytterhoeven 
---
 drivers/usb/renesas_usbhs/common.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index f0ce304c5aaf54f1..32f86935d59f6b09 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -501,7 +501,6 @@ static struct renesas_usbhs_platform_info 
*usbhs_parse_dt(struct device *dev)
 {
struct renesas_usbhs_platform_info *info;
struct renesas_usbhs_driver_param *dparam;
-   const struct of_device_id *of_id = of_match_device(usbhs_of_match, dev);
u32 tmp;
int gpio;
 
@@ -510,7 +509,7 @@ static struct renesas_usbhs_platform_info 
*usbhs_parse_dt(struct device *dev)
return NULL;
 
dparam = >driver_param;
-   dparam->type = of_id ? (uintptr_t)of_id->data : 0;
+   dparam->type = (uintptr_t)of_device_get_match_data(dev);
if (!of_property_read_u32(dev->of_node, "renesas,buswait", ))
dparam->buswait_bwait = tmp;
gpio = of_get_named_gpio_flags(dev->of_node, "renesas,enable-gpio", 0,
-- 
2.7.4

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

[PATCH] usb: host: xhci-plat: Use of_device_get_match_data() helper

[Geert Uytterhoeven]

Use the of_device_get_match_data() helper instead of open coding.

Signed-off-by: Geert Uytterhoeven 
---
 drivers/usb/host/xhci-plat.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 163bafde709f79bf..a1b42348e0c8c6c8 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -152,7 +153,7 @@ MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
 
 static int xhci_plat_probe(struct platform_device *pdev)
 {
-   const struct of_device_id *match;
+   const struct xhci_plat_priv *priv_match;
const struct hc_driver  *driver;
struct device   *sysdev;
struct xhci_hcd *xhci;
@@ -238,9 +239,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
}
 
xhci = hcd_to_xhci(hcd);
-   match = of_match_node(usb_xhci_of_match, pdev->dev.of_node);
-   if (match) {
-   const struct xhci_plat_priv *priv_match = match->data;
+   priv_match = of_device_get_match_data(>dev);
+   if (priv_match) {
struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
 
/* Just copy data for now */
-- 
2.7.4

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

Re: [PATCH] extcon: Split out extcon header file for consumer and provider device

[Lee Jones]

On Fri, 29 Sep 2017, Chanwoo Choi wrote:

> The extcon has two type of extcon devices as following.
> - 'extcon provider deivce' adds new extcon device and detect the
>state/properties of external connector. Also, it notifies the
>state/properties to the extcon consumer device.
> - 'extcon consumer device' gets the change state/properties
>from extcon provider device.
> Prior to that, include/linux/extcon.h contains all exported API for
> both provider and consumer device driver. To clarify the meaning of
> header file and to remove the wrong use-case on consumer device,
> this patch separates into extcon.h and extcon-provider.h.
> 
> [Description for include/linux/{extcon.h|extcon-provider.h}]
> - extcon.h includes the extcon API and data structure for extcon consumer
>   device driver. This header file contains the following APIs:
>   : Register/unregister the notifier to catch the change of extcon device
>   : Get the extcon device instance
>   : Get the extcon device name
>   : Get the state of each external connector
>   : Get the property value of each external connector
>   : Get the property capability of each external connector
> 
> - extcon-provider.h includes the extcon API and data structure for extcon
>   provider device driver. This header file contains the following APIs:
>   : Include 'include/linux/extcon.h'
>   : Allocate the memory for extcon device instance
>   : Register/unregister extcon device
>   : Set the state of each external connector
>   : Set the property value of each external connector
>   : Set the property capability of each external connector
> 
> Cc: Felipe Balbi 
> Cc: Kishon Vijay Abraham I 
> Cc: Greg Kroah-Hartman 
> Cc: Sebastian Reichel 
> Cc: Lee Jones 
> Signed-off-by: Chanwoo Choi 
> ---
>  drivers/extcon/extcon-adc-jack.c  |   2 +-
>  drivers/extcon/extcon-arizona.c   |   2 +-
>  drivers/extcon/extcon-axp288.c|   2 +-
>  drivers/extcon/extcon-gpio.c  |   2 +-
>  drivers/extcon/extcon-intel-cht-wc.c  |   2 +-
>  drivers/extcon/extcon-intel-int3496.c |   2 +-
>  drivers/extcon/extcon-max14577.c  |   2 +-
>  drivers/extcon/extcon-max3355.c   |   2 +-
>  drivers/extcon/extcon-max77693.c  |   2 +-
>  drivers/extcon/extcon-max77843.c  |   2 +-
>  drivers/extcon/extcon-max8997.c   |   2 +-
>  drivers/extcon/extcon-qcom-spmi-misc.c|   2 +-
>  drivers/extcon/extcon-rt8973a.c   |   2 +-
>  drivers/extcon/extcon-sm5502.c|   2 +-
>  drivers/extcon/extcon-usb-gpio.c  |   2 +-
>  drivers/extcon/extcon-usbc-cros-ec.c  |   2 +-
>  drivers/extcon/extcon.h   |   2 +-
>  drivers/phy/allwinner/phy-sun4i-usb.c |   2 +-
>  drivers/phy/broadcom/phy-bcm-ns2-usbdrd.c |   2 +-
>  drivers/phy/renesas/phy-rcar-gen3-usb2.c  |   2 +-
>  drivers/phy/rockchip/phy-rockchip-inno-usb2.c |   2 +-
>  drivers/power/supply/qcom_smbb.c  |   2 +-
>  drivers/usb/gadget/udc/renesas_usb3.c |   2 +-
>  drivers/usb/phy/phy-tahvo.c   |   2 +-
>  drivers/usb/renesas_usbhs/common.h|   2 +-
>  include/linux/extcon-provider.h   | 142 
> ++
>  include/linux/extcon.h| 109 +---

>  include/linux/mfd/palmas.h|   2 +-

Acked-by: Lee Jones 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3][for 4.14] xhci: allow TRACE to work with EVENT ring dequeue

[Adam Wallis]

On 9/26/2017 2:44 AM, Mathias Nyman wrote:
> On 25.09.2017 19:09, David Laight wrote:
>> From: Adam Wallis
>>> Sent: 25 September 2017 13:26
>>> inc_deq() currently bails earlier for EVENT rings than the common return
>>> point of the function, due to the fact that EVENT rings do not have
>>> link TRBs. The unfortunate side effect of this is that the very useful
>>> trace_xhci_inc_deq() function is not called/usable for EVENT ring
>>> debug.
>>
>> Is it actually worth using different functions for the different
>> ring types?
>>  From what I remember there are conditionals in a lot of the functions
>> but they are fixed for most of the call sites.
>>
> 
> There's some restructuring and refactoring that could be done in xhci,
> but that's not part of this patch.
> 
> This will just enable better debugging.
> 
> Applying this patch
Sounds great, thanks! Will this be going in on 4.14 sometime shortly? I hadn't
seen it in your tree and was curious since we are tracking internally. Thanks!

> 
> Thanks
> -Mathias


-- 
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: wusbcore: Use put_unaligned_le32

[Himanshu Jha]

Use put_unaligned_le32 rather than using byte ordering function and
memcpy which makes code clear.
Also, add the header file where it is declared.

Done using Coccinelle and semantic patch used is :

@ rule1 @
identifier tmp; expression ptr,x; type T;
@@

- tmp = cpu_to_le32(x);

  <+... when != tmp
- memcpy(ptr, (T), ...);
+ put_unaligned_le32(x,ptr);
  ...+>

@ depends on rule1 @
type j; identifier tmp;
@@

- j tmp;
  ...when != tmp

Signed-off-by: Himanshu Jha 
---
 drivers/usb/wusbcore/security.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
index 170f2c3..256b56f 100644
--- a/drivers/usb/wusbcore/security.c
+++ b/drivers/usb/wusbcore/security.c
@@ -22,6 +22,7 @@
  *
  * FIXME: docs
  */
+#include 
 #include 
 #include 
 #include 
@@ -367,7 +368,6 @@ int wusb_dev_4way_handshake(struct wusbhc *wusbhc, struct 
wusb_dev *wusb_dev,
struct usb_device *usb_dev = wusb_dev->usb_dev;
struct device *dev = _dev->dev;
u32 tkid;
-   __le32 tkid_le;
struct usb_handshake *hs;
struct aes_ccm_nonce ccm_n;
u8 mic[8];
@@ -385,11 +385,10 @@ int wusb_dev_4way_handshake(struct wusbhc *wusbhc, struct 
wusb_dev *wusb_dev,
goto error_dev_set_encryption;
 
tkid = wusbhc_next_tkid(wusbhc, wusb_dev);
-   tkid_le = cpu_to_le32(tkid);
 
hs[0].bMessageNumber = 1;
hs[0].bStatus = 0;
-   memcpy(hs[0].tTKID, _le, sizeof(hs[0].tTKID));
+   put_unaligned_le32(tkid, hs[0].tTKID);
hs[0].bReserved = 0;
memcpy(hs[0].CDID, _dev->cdid, sizeof(hs[0].CDID));
get_random_bytes([0].nonce, sizeof(hs[0].nonce));
@@ -441,7 +440,7 @@ int wusb_dev_4way_handshake(struct wusbhc *wusbhc, struct 
wusb_dev *wusb_dev,
 
/* Setup the CCM nonce */
memset(_n.sfn, 0, sizeof(ccm_n.sfn)); /* Per WUSB1.0[6.5.2] */
-   memcpy(ccm_n.tkid, _le, sizeof(ccm_n.tkid));
+   put_unaligned_le32(tkid, ccm_n.tkid);
ccm_n.src_addr = wusbhc->uwb_rc->uwb_dev.dev_addr;
ccm_n.dest_addr.data[0] = wusb_dev->addr;
ccm_n.dest_addr.data[1] = 0;
@@ -472,7 +471,7 @@ int wusb_dev_4way_handshake(struct wusbhc *wusbhc, struct 
wusb_dev *wusb_dev,
/* Send Handshake3 */
hs[2].bMessageNumber = 3;
hs[2].bStatus = 0;
-   memcpy(hs[2].tTKID, _le, sizeof(hs[2].tTKID));
+   put_unaligned_le32(tkid, hs[2].tTKID);
hs[2].bReserved = 0;
memcpy(hs[2].CDID, _dev->cdid, sizeof(hs[2].CDID));
memcpy(hs[2].nonce, hs[0].nonce, sizeof(hs[2].nonce));
-- 
2.7.4

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

Re: [PATCH 2/2] USB: serial: console: fix use-after-free after failed setup

[Greg KH]

On Wed, Oct 04, 2017 at 11:01:13AM +0200, Johan Hovold wrote:
> Make sure to reset the USB-console port pointer when console setup fails
> in order to avoid having the struct usb_serial be prematurely freed by
> the console code when the device is later disconnected.
> 
> Fixes: 73e487fdb75f ("[PATCH] USB console: fix disconnection issues")
> Cc: stable    # 2.6.18
> Signed-off-by: Johan Hovold 

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

Re: usb/serial/visor: slab-out-of-bounds in palm_os_3_probe

[Greg Kroah-Hartman]

On Tue, Oct 03, 2017 at 11:29:40AM +0200, Johan Hovold wrote:
> On Fri, Sep 29, 2017 at 10:37:55AM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Sep 28, 2017 at 07:57:46PM +0200, Andrey Konovalov wrote:
> > > Hi!
> > > 
> > > I've got the following report while fuzzing the kernel with syzkaller.
> > > 
> > > On commit dc972a67cc54585bd83ad811c4e9b6ab3dcd427e (4.14-rc2+).
> > > 
> > > There's no check on the connection_info->num_ports value when
> > > iterating over ports.
> > > 
> > > usb 1-1: Handspring Visor / Palm OS: port 162, is for unknown use
> > > usb 1-1: Handspring Visor / Palm OS: port 81, is for unknown use
> > > ==
> > > BUG: KASAN: slab-out-of-bounds in palm_os_3_probe+0x4e4/0x570
> > > Read of size 1 at addr 8800686daa26 by task kworker/0:1/24
> 
> Thanks for the report, Andrey.
> 
> > Ah, nice catch, this bug is _old_, sorry about that.
> > 
> > The patch below should resolve this.  It looks bigger than it really is,
> > as I'm just moving the error checking higher up in the function, and
> > loosing an indentation for when there is invalid data.
> > 
> > Can you let me know if this solves the issue?
> 
> And thanks for fixing this up, Greg. Will you send a proper patch that I
> can apply?

Yes, let me redo it based on your comments, and will send it out
"correctly" in a few days.

thanks for the review,

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

Re: [PATCH 1/2] USB: serial: console: fix use-after-free on disconnect

[Greg KH]

On Wed, Oct 04, 2017 at 11:01:12AM +0200, Johan Hovold wrote:
> A clean-up patch removing removing two redundant NULL-checks from the
> console disconnect handler inadvertently also removed a third check.
> This could lead to the struct usb_serial being prematurely freed by the
> console code when a driver accepts but does not register any ports for
> an interface which also lacks endpoint descriptors.
> 
> Fixes: 0e517c93dc02 ("USB: serial: console: clean up sanity checks")
> Cc: stable  # 4.11
> Reported-by: Andrey Konovalov 
> Signed-off-by: Johan Hovold 

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