[PATCH] USB: microtek: fix info-leak at probe
Add missing bulk-in endpoint sanity check to prevent uninitialised stack data from being reported to the system log and used as endpoint addresses. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable Reported-by: syzbot+5630ca7c3b2be5c9d...@syzkaller.appspotmail.com Signed-off-by: Johan Hovold --- drivers/usb/image/microtek.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c index 0a57c2cc8e5a..7a6b122c833f 100644 --- a/drivers/usb/image/microtek.c +++ b/drivers/usb/image/microtek.c @@ -716,6 +716,10 @@ static int mts_usb_probe(struct usb_interface *intf, } + if (ep_in_current != &ep_in_set[2]) { + MTS_WARNING("couldn't find two input bulk endpoints. Bailing out.\n"); + return -ENODEV; + } if ( ep_out == -1 ) { MTS_WARNING( "couldn't find an output bulk endpoint. Bailing out.\n" ); -- 2.23.0
Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
On 01/04/2019 14:42, Andrej Shadura wrote: > U2F Zero supports custom commands for blinking the LED and getting data > from the internal hardware RNG. Expose the blinking function as a LED > device, and the internal hardware RNG as an HWRNG so that it can be used > to feed the enthropy pool. > > Signed-off-by: Andrej Shadura I’ve been testing this with a different modification of U2F Zero, Nitrokey FIDO U2F, and on that device only I’m getting a kernel warning (see below). > +static int u2fzero_recv(struct u2fzero_device *dev, > + struct u2f_hid_report *req, > + struct u2f_hid_msg *resp) > +{ > + int ret; > + struct hid_device *hdev = dev->hdev; > + struct u2fzero_transfer_context ctx; > + > + mutex_lock(&dev->lock); > + > + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report)); > + > + dev->urb->context = &ctx; > + init_completion(&ctx.done); > + > + ret = usb_submit_urb(dev->urb, GFP_NOIO); Here, usb_submit_urb() returns EBUSY in the first attempt to read random numbers from the device: URB 917256d5d540 submitted while active WARNING: CPU: 3 PID: 31 at drivers/usb/core/urb.c:363 usb_submit_urb+0x4c2/0x5b0 <...> Call Trace: u2fzero_rng_read+0x16e/0x340 [hid_u2fzero] ? ttwu_do_activate+0x67/0x90 add_early_randomness+0x53/0xc0 hwrng_register+0x175/0x180 devm_hwrng_register+0x41/0x7e u2fzero_probe+0x2dd/0x350 [hid_u2fzero] hid_device_probe+0x119/0x180 [hid] really_probe+0xfe/0x3b0 driver_probe_device+0xba/0x100 __device_attach_driver+0x97/0x100 I don’t understand why since 1) it’s likely to be the first transmission of this URB, 2) there’s a mutex locked just before it. I received a comment from a colleague mentioning I’m probably not using the mutex correctly, but I don’t understand why. I’m trying to figure this out, so I’d welcome any help with it. Thanks in advance. > + if (unlikely(ret)) { > + hid_err(hdev, "usb_submit_urb failed: %d", ret); > + goto err; > + } > + > + ret = hid_hw_output_report(dev->hdev, dev->buf_out, > +sizeof(struct u2f_hid_msg)); > + > + if (ret < 0) { > + hid_err(hdev, "hid_hw_output_report failed: %d", ret); > + goto err; > + } > + > + ret = (wait_for_completion_timeout( > + &ctx.done, msecs_to_jiffies(USB_CTRL_SET_TIMEOUT))); > + if (ret < 0) { > + usb_kill_urb(dev->urb); > + hid_err(hdev, "urb submission timed out"); > + } else { > + ret = dev->urb->actual_length; > + memcpy(resp, dev->buf_in, ret); > + } > + > +err: > + mutex_unlock(&dev->lock); > + > + return ret; > +} <...> > +static int u2fzero_rng_read(struct hwrng *rng, void *data, > + size_t max, bool wait) > +{ > + struct u2fzero_device *dev = container_of(rng, > + struct u2fzero_device, hwrng); > + struct u2f_hid_report req = { > + .report_type = 0, > + .msg.cid = CID_BROADCAST, > + .msg.init = { > + .cmd = U2F_CUSTOM_GET_RNG, > + .bcnth = 0, > + .bcntl = 0, > + .data = {0}, > + } > + }; > + struct u2f_hid_msg resp; > + int ret; > + size_t actual_length; > + > + if (!dev->present) { > + hid_dbg(dev->hdev, "device not present"); > + return 0; > + } > + > + ret = u2fzero_recv(dev, &req, &resp); > + if (ret < 0) > + return 0; > + > + /* only take the minimum amount of data it is safe to take */ > + actual_length = min3((size_t)ret - offsetof(struct u2f_hid_msg, > + init.data), U2F_HID_MSG_LEN(resp), max); > + > + memcpy(data, resp.init.data, actual_length); > + > + return actual_length; > +} <...> > +static int u2fzero_init_hwrng(struct u2fzero_device *dev, > + unsigned int minor) > +{ > + dev->rng_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL, > + "%s-rng%u", DRIVER_SHORT, minor); > + if (dev->rng_name == NULL) > + return -ENOMEM; > + > + dev->hwrng.name = dev->rng_name; > + dev->hwrng.read = u2fzero_rng_read; > + dev->hwrng.quality = 1; > + > + return devm_hwrng_register(&dev->hdev->dev, &dev->hwrng); > +} > + > +static int u2fzero_fill_in_urb(struct u2fzero_device *dev) > +{ > + struct hid_device *hdev = dev->hdev; > + struct usb_device *udev; > + struct usbhid_device *usbhid = hdev->driver_data; > + unsigned int pipe_in; > + struct usb_host_endpoint *ep; > + > + if (dev->hdev->bus != BUS_USB) > + return -EINVAL; > + > + udev = hid_to_usb_dev(hdev); > + > + if (!usbhid->urbout || !usbhid->urbin) > + return -ENODEV; > + > + ep = usb_pipe_endpoint(udev, usbhid->urbin->pipe); > + if (!ep) > + return -ENODEV; > + > + dev->
Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration
On Wed, Oct 02, 2019 at 08:45:28PM -0700, Guenter Roeck wrote: > On 10/2/19 11:29 AM, Heikki Krogerus wrote: > > On Wed, Oct 02, 2019 at 09:36:39AM -0700, Guenter Roeck wrote: > > > port->cap->type used to be the role provided by the low level driver. > > > That was static, and it was not possible to override it. It did not > > > resemble the current port type, or a configured port type, it resembled > > > port capabilities. > > > > > > Your code changes that, meaning even if the low level driver (effectively > > > the hardware) states that it can not support DRP, it is now configurable > > > anyway. That seems to me like a substantial change to the original meaning > > > of this attribute. > > > > > > Effectiv ely there are now three values, > > > - port->port_type the current port tyle > > > - port->fixed_typethe type selected by the user > > > - port->cap->type the type provided by low level code, > > > now no longer available / used > > > > > > Even if the low level driver (hardware) says that it can not support > > > TYPEC_PORT_DRP, that can be overwritten by the user. Maybe there is a > > > good reason for that, but I don't see it, sorry. > > > > No, that was not my intention. Clearly there is a bug in my code. > > > > > Maybe it would make sense to introduce port->fixed_type in a separate > > > patch. > > > As part of that patch you could explain why port->cap->type, ie a > > > reflection > > > of the port capabilities, is no longer needed. > > > > Or, I could make this really simple. I could just copy the content of > > the cap as is to another struct typec_capability during port > > registration. My goal is really just remove the need for the device > > drivers keep the struct typec_capability instance if they don't need > > it, and I don't need to remove the cap member to achieve that. > > > > Maybe just use devm_kmemdup() ? For example. thanks, -- heikki
Re: [PATCH] USB: microtek: fix info-leak at probe
Am Donnerstag, den 03.10.2019, 09:09 +0200 schrieb Johan Hovold: > Add missing bulk-in endpoint sanity check to prevent uninitialised stack > data from being reported to the system log and used as endpoint > addresses. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Cc: stable > Reported-by: syzbot+5630ca7c3b2be5c9d...@syzkaller.appspotmail.com > Signed-off-by: Johan Hovold Acked-by: Oliver Neukum
Re: musb: cppi41: broken high speed FTDI functionality when connected to musb directly
Hi Tony, On Wed, Oct 2, 2019 at 6:52 PM Tony Lindgren wrote: > > * Yegor Yefremov [191002 06:57]: > > On Wed, Oct 2, 2019 at 12:03 AM Tony Lindgren wrote: > > > The other way to fix this would be to just wake up cpp41 in > > > cppi41_dma_prep_slave_sg() and return NULL so that we can > > > have musb_ep_program() continue with PIO while cppi41 is > > > asleep. > > > > > > Anyways, care to try it out and see if it fixes your issue? > > > > The fix is working but on the first invocation, I get this output > > (minicom provokes the same output): > > > # serialtest.py -c 2 /dev/ttyUSB0 /dev/ttyUSB0 > ... > > [ 210.940612] [] (__rpm_callback) from [] > > (rpm_callback+0x20/0x80) > > [ 210.948402] [] (rpm_callback) from [] > > (rpm_resume+0x468/0x7a0) > > [ 210.956018] [] (rpm_resume) from [] > > (__pm_runtime_resume+0x4c/0x64) > > [ 210.964086] [] (__pm_runtime_resume) from [] > > (cppi41_dma_prep_slave_sg+0x20/0xfc [cppi41]) > > OK so that won't work, thanks for testing. Here's the alternative > patch to try along the lines described above that just wakes up > cppi41 and returns NULL so musb_ep_program() can continue with PIO > until cppi41 is awake. I'm out of the office for some weeks and don't have access to my hw. I'll make the test as soon as I'm back. Thanks. Yegor > 8< --- > diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c > --- a/drivers/dma/ti/cppi41.c > +++ b/drivers/dma/ti/cppi41.c > @@ -586,9 +586,22 @@ static struct dma_async_tx_descriptor > *cppi41_dma_prep_slave_sg( > enum dma_transfer_direction dir, unsigned long tx_flags, void > *context) > { > struct cppi41_channel *c = to_cpp41_chan(chan); > + struct dma_async_tx_descriptor *txd = NULL; > + struct cppi41_dd *cdd = c->cdd; > struct cppi41_desc *d; > struct scatterlist *sg; > unsigned int i; > + int error; > + > + error = pm_runtime_get(cdd->ddev.dev); > + if (error < 0) { > + pm_runtime_put_noidle(cdd->ddev.dev); > + > + return NULL; > + } > + > + if (cdd->is_suspended) > + goto err_out_not_ready; > > d = c->desc; > for_each_sg(sgl, sg, sg_len, i) { > @@ -611,7 +624,13 @@ static struct dma_async_tx_descriptor > *cppi41_dma_prep_slave_sg( > d++; > } > > - return &c->txd; > + txd = &c->txd; > + > +err_out_not_ready: > + pm_runtime_mark_last_busy(cdd->ddev.dev); > + pm_runtime_put_autosuspend(cdd->ddev.dev); > + > + return txd; > } > > static void cppi41_compute_td_desc(struct cppi41_desc *d) > -- > 2.23.0
Re: Regression: USB/xhci issues on some systems with newer kernel versions
On 2.10.2019 15.28, Bernhard Gebetsberger wrote: Hi, There has been a regression in the xhci driver since kernel version 4.20, on some systems some usb devices won't work until the system gets rebooted. The error message in dmesg is "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state", although for some reason there are some usb devices that are affected by this issue but don't throw the error message(including the device I'm using, I got the error in previous kernel versions though). It seems like this bug can also lead to system instability, one user reported in the bug tracker(https://bugzilla.kernel.org/show_bug.cgi?id=202541#c58) that he got a system freeze because of this when using kernel 5.3.1. Ok, lets take a look at this. Some of the symptoms vary a bit in the report, so lets focus on ones that show: "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state" When looking at the responses in the bug tracker, it looks like it mostly affects Ryzen based systems with 300 series motherboards, although there are some other affected systems as well. It doesn't only affect wifi/bluetooth sticks, some users even got this issue when connecting their smartphone or their external hard drive to their PC. I have uploaded the whole dmesg file and the tracing file to transfer.sh: https://transfer.sh/zYohl/dmesg and https://transfer.sh/KNbFL/xhci-trace Hmm, trying to download these just shows "Not Found" Could someone with a affected system enable tracing and dynamic debug on a recent kernel, take logs and traces of one failing instance where the message "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state" is seen. mount -t debugfs none /sys/kernel/debug echo 'module xhci_hcd =p' >/sys/kernel/debug/dynamic_debug/control echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable < Trigger the issue > Send output of dmesg Send content of /sys/kernel/debug/tracing/trace The issues occur since commit f8f80be501aa2f10669585c3e328fad079d8cb3a "xhci: Use soft retry to recover faster from transaction errors". I think this commit should be reverted at least until a workaround has been found, especially since the next two kernel versions will be used by a lot of distributions(5.4 because it's a LTS kernel and 5.5 will probably be used in Ubuntu 20.04) so more users would be affected by this. There some time left before 5.4 is out, lets see if we can find the root cause first. -Mathias
Re: [PATCH] USB: rio500: Remove Rio 500 kernel driver
On Mon, 2019-09-23 at 18:18 +0200, Bastien Nocera wrote: > The Rio500 kernel driver has not been used by Rio500 owners since 2001 > not long after the rio500 project added support for a user-space USB stack > through the very first versions of usbdevfs and then libusb. > > Support for the kernel driver was removed from the upstream utilities > in 2008: > https://gitlab.freedesktop.org/hadess/rio500/commit/943f624ab721eb8281c287650fcc9e2026f6f5db > > Cc: Cesar Miquel > Signed-off-by: Bastien Nocera Anything else I need to do to land this? Cheers
Re: [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration
On Wed, Oct 02, 2019 at 08:51:32PM -0700, Guenter Roeck wrote: > On 10/2/19 12:16 PM, Heikki Krogerus wrote: > > On Wed, Oct 02, 2019 at 07:06:55PM +0300, Heikki Krogerus wrote: > > > This is a bit off topic, but that attribute file is really horrible. > > > Right now there is no way to know the actual capability of the > > > port in user space. If something changes a DRP port into sink or > > > source, there is no way to know after that that the port is actually > > > DRP capable. > > > > That statement is not correct. I'm sorry. I don't know why did I put > > it that way. > > > > I wanted to say that now the userpsace is forced to keep track of a > > specific sysfs file in order to be sure of the currently supported > > role, That alone is wrong, but there is no way to guarantee that > > one day we would not need to support another capability in a similar > > fashion, and that would mean another sysfs file, and we just forced > > the userspace to be modified. The capabilities of the port really need > > to be static. > > > > I assume you refer to port_type. If I remember correctly, this is actually > used by Android userspace code to specifically select if a port can be > source, sink, or drp. I am not sure if it is a good idea to enforce > port removal and re-registration in conjunction with this - the port > can, after all, be connected to some storage device or to a monitor. > I am not sure how we could "sell" to users the idea that if they change > the port type, their screen will go dark for a few seconds. > > Am I missing something ? I'm not sure how can you avoid flickering screen even with the current port_type sysfs flag? If you change the port type, you will reset the port, which means you have to also re-enter the altmode again, no? If so, then does unregistering and registering the ports actually make that much difference from the users perspective? But why do you need the supported roles to be changed when there is already a partner device plugged to the connector? What is the use-case/requirement here? One note about my proposal: With my proposal the userspace can be given the possibility to influence the port capabilities even before the ports have been registered. I would imagine that should cover at least some of the requirements. The major problem here is that we can not guarantee how the ports behave if the supported roles are changed when a device is already plugged to the connector (don't forget, we are not always using tcpm.c). With UCSI at least it really depends on the underlying firmware implementation. It also creates risks for the user, like writing "sink" to that port_type and ending up with a black screen, and I mean permanently black screen. Un-plugging and re-plugging the monitor back will not help. Only changing the port_type again, or reboot work. Ideally we should not allow the capabilities to be changed after the partner device is already registered at all, but if we really have to allow it, then I still think we should do it the way I proposed. thanks, > Thanks, > Guenter > > > We can handle the capability changes like I propose below without > > affecting the userspace. > > > > > So that ABI is "buggy", but even without the problem, I still really > > > think that allowing the capabilities to be changed like that in > > > general is completely wrong. I don't have a problem with changing the > > > capabilities, but IMO it should be handled at one level higher, at the > > > controller device level. If the capabilities of a port need to be > > > changed, the old port should be removed, and a new with the new > > > capabilities registered. That is the only way to handle it without > > > making things unnecessarily difficult for the user space. > > > > > > I'm pretty sure that that was my counter proposal already at the time > > > when the attribute file was introduced, but I don't remember why > > > wasn't it accepted at the time? My protest against adding that > > > attribute file was in any case ignored :-(. In any case, my plan was > > > to later propose a new sysfs group that we offer to the parent > > > controller devices instead assigning it to the port devices, and that > > > group is meant to allow port capability changes the way I explained > > > above. Unless of course you are against it? > > > > > > thanks, > > > > > > -- > > > heikki > > -- heikki
Re: [PATCH] USB: rio500: Remove Rio 500 kernel driver
On Thu, Oct 03, 2019 at 03:18:16PM +0200, Bastien Nocera wrote: > On Mon, 2019-09-23 at 18:18 +0200, Bastien Nocera wrote: > > The Rio500 kernel driver has not been used by Rio500 owners since 2001 > > not long after the rio500 project added support for a user-space USB stack > > through the very first versions of usbdevfs and then libusb. > > > > Support for the kernel driver was removed from the upstream utilities > > in 2008: > > https://gitlab.freedesktop.org/hadess/rio500/commit/943f624ab721eb8281c287650fcc9e2026f6f5db > > > > Cc: Cesar Miquel > > Signed-off-by: Bastien Nocera > > Anything else I need to do to land this? Patience, 5.4-rc1 just came out, my queue is 1500+ patches deep, I will dig through it in the next week... thanks, greg k-h
[PATCH] USB: serial: keyspan: fix NULL-derefs on open() and write()
Fix NULL-pointer dereferences on open() and write() which can be triggered by a malicious USB device. The current URB allocation helper would fail to initialise the newly allocated URB if the device has unexpected endpoint descriptors, something which could lead NULL-pointer dereferences in a number of open() and write() paths when accessing the URB. For example: BUG: kernel NULL pointer dereference, address: ... RIP: 0010:usb_clear_halt+0x11/0xc0 ... Call Trace: ? tty_port_open+0x4d/0xd0 keyspan_open+0x70/0x160 [keyspan] serial_port_activate+0x5b/0x80 [usbserial] tty_port_open+0x7b/0xd0 ? check_tty_count+0x43/0xa0 tty_open+0xf1/0x490 BUG: kernel NULL pointer dereference, address: ... RIP: 0010:keyspan_write+0x14e/0x1f3 [keyspan] ... Call Trace: serial_write+0x43/0xa0 [usbserial] n_tty_write+0x1af/0x4f0 ? do_wait_intr_irq+0x80/0x80 ? process_echoes+0x60/0x60 tty_write+0x13f/0x2f0 BUG: kernel NULL pointer dereference, address: ... RIP: 0010:keyspan_usa26_send_setup+0x298/0x305 [keyspan] ... Call Trace: keyspan_open+0x10f/0x160 [keyspan] serial_port_activate+0x5b/0x80 [usbserial] tty_port_open+0x7b/0xd0 ? check_tty_count+0x43/0xa0 tty_open+0xf1/0x490 Fixes: fdcba53e2d58 ("fix for bugzilla #7544 (keyspan USB-to-serial converter)") Cc: stable # 2.6.21 Signed-off-by: Johan Hovold --- drivers/usb/serial/keyspan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c index d34779fe4a8d..e66a59ef43a1 100644 --- a/drivers/usb/serial/keyspan.c +++ b/drivers/usb/serial/keyspan.c @@ -1741,8 +1741,8 @@ static struct urb *keyspan_setup_urb(struct usb_serial *serial, int endpoint, ep_desc = find_ep(serial, endpoint); if (!ep_desc) { - /* leak the urb, something's wrong and the callers don't care */ - return urb; + usb_free_urb(urb); + return NULL; } if (usb_endpoint_xfer_int(ep_desc)) { ep_type_name = "INT"; -- 2.23.0
Re: [PATCH] USB: rio500: Remove Rio 500 kernel driver
On Thu, 2019-10-03 at 15:42 +0200, Greg Kroah-Hartman wrote: > On Thu, Oct 03, 2019 at 03:18:16PM +0200, Bastien Nocera wrote: > > On Mon, 2019-09-23 at 18:18 +0200, Bastien Nocera wrote: > > > The Rio500 kernel driver has not been used by Rio500 owners since > > > 2001 > > > not long after the rio500 project added support for a user-space > > > USB stack > > > through the very first versions of usbdevfs and then libusb. > > > > > > Support for the kernel driver was removed from the upstream > > > utilities > > > in 2008: > > > https://gitlab.freedesktop.org/hadess/rio500/commit/943f624ab721eb8281c287650fcc9e2026f6f5db > > > > > > Cc: Cesar Miquel > > > Signed-off-by: Bastien Nocera > > > > Anything else I need to do to land this? > > Patience, 5.4-rc1 just came out, my queue is 1500+ patches deep, I > will > dig through it in the next week... No worries, was just expecting some/any feedback before long.
Re: [PATCH 00/14] usb: typec: UCSI driver overhaul
Hi Ajay, On Tue, Oct 01, 2019 at 06:36:25PM +, Ajay Gupta wrote: > Hi Heikki > > > -Original Message- > > From: Heikki Krogerus > > Sent: Thursday, September 26, 2019 3:07 AM > > To: Ajay Gupta > > Cc: linux-usb@vger.kernel.org > > Subject: [PATCH 00/14] usb: typec: UCSI driver overhaul > > > > Hi Ajay, > > > > Here's the pretty much complete rewrite of the I/O handling that I was > > talking about. The first seven patches are not actually related to > > this stuff, but I'm including them here because the rest of the series > > is made on top of them. I'm including also that fix patch I send you > > earlier. > > > > After this it should be easier to handle quirks. My idea how to handle > > the multi-instance connector alt modes is that we "emulate" the PPM in > > ucsi_ccg.c in order to handle them, so ucsi.c is not touched at all. > > > > We can now get the connector alternate modes that the actual > > controller supplies during probe - before registering the ucsi > > interface > How can ucsi_ccg.c get the connector alternate modes before > registering ucsi interface? PPM reset, notification enable, etc. > is done during ucsi registration. UCSI spec says: > " The only commands the PPM is required to process in the > "PPM Idle (Notifications Disabled)" state are > SET_NOTIFICATION_ENABLE and PPM_RESE" > > Also, it doesn't look correct if ucsi_ccg.c has to replicate most > of the stuff done in ucsi_init() of ucsi.c. How about if we split ucsi_init() into a function that first simply constructs the struct ucsi and struct ucsi_connector instances without registering anything, and into separate functions that then register the ports, altmodes and what have you. I don't think that should be a huge problem. It will make ucsi.c even more like a library, which is probable a good thing. I can prepare patches for that too if you like? After that you should be able to get the struct ucsi instance that represents the "real" PPM without registering anything by calling a single function, most likely ucsi_init(). And after getting that you can construct the connector alternate modes that we actually register. Finally you register the final interface which does not use ucsi_ccg_ops, but instead something like ucsi_nvidia_ops. How would this sound to you? Br, -- heikki
Re: Regression: USB/xhci issues on some systems with newer kernel versions
I sent the instructions to one of the users in the bug tracker. Here is the download link for his logs: https://www.sendspace.com/file/413hlj - Bernhard Am 03.10.19 um 12:23 schrieb Mathias Nyman: > On 2.10.2019 15.28, Bernhard Gebetsberger wrote: >> Hi, >> >> There has been a regression in the xhci driver since kernel version 4.20, on >> some systems some usb devices won't work until the system gets rebooted. >> The error message in dmesg is "WARN Set TR Deq Ptr cmd failed due to >> incorrect slot or ep state", although for some reason there are some usb >> devices that are affected by this issue but don't throw the error >> message(including the device I'm using, I got the error in previous kernel >> versions though). >> It seems like this bug can also lead to system instability, one user >> reported in the bug >> tracker(https://bugzilla.kernel.org/show_bug.cgi?id=202541#c58) that he got >> a system freeze because of this when using kernel 5.3.1. >> > > Ok, lets take a look at this. > Some of the symptoms vary a bit in the report, so lets focus on ones that > show: "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state" > >> When looking at the responses in the bug tracker, it looks like it mostly >> affects Ryzen based systems with 300 series motherboards, although there are >> some other affected systems as well. It doesn't only affect wifi/bluetooth >> sticks, some users even got this issue when connecting their smartphone or >> their external hard drive to their PC. > >> >> I have uploaded the whole dmesg file and the tracing file to transfer.sh: >> https://transfer.sh/zYohl/dmesg and https://transfer.sh/KNbFL/xhci-trace > > Hmm, trying to download these just shows "Not Found" > > Could someone with a affected system enable tracing and dynamic debug on a > recent kernel, take logs and traces of one failing instance where the message > "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state" is seen. > > mount -t debugfs none /sys/kernel/debug > echo 'module xhci_hcd =p' >/sys/kernel/debug/dynamic_debug/control > echo 'module usbcore =p' >/sys/kernel/debug/dynamic_debug/control > echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb > echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable > > < Trigger the issue > > > Send output of dmesg > Send content of /sys/kernel/debug/tracing/trace > >> >> The issues occur since commit f8f80be501aa2f10669585c3e328fad079d8cb3a >> "xhci: Use soft retry to recover faster from transaction errors". I think >> this commit should be reverted at least until a workaround has been found, >> especially since the next two kernel versions will be used by a lot of >> distributions(5.4 because it's a LTS kernel and 5.5 will probably be used in >> Ubuntu 20.04) so more users would be affected by this. >> > > There some time left before 5.4 is out, lets see if we can find the root > cause first. > > -Mathias >
Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
On Thu, 3 Oct 2019, Andrej Shadura wrote: > On 01/04/2019 14:42, Andrej Shadura wrote: > > U2F Zero supports custom commands for blinking the LED and getting data > > from the internal hardware RNG. Expose the blinking function as a LED > > device, and the internal hardware RNG as an HWRNG so that it can be used > > to feed the enthropy pool. > > > > Signed-off-by: Andrej Shadura > > I’ve been testing this with a different modification of U2F Zero, > Nitrokey FIDO U2F, and on that device only I’m getting a kernel warning > (see below). > > > +static int u2fzero_recv(struct u2fzero_device *dev, > > + struct u2f_hid_report *req, > > + struct u2f_hid_msg *resp) > > +{ > > + int ret; > > + struct hid_device *hdev = dev->hdev; > > + struct u2fzero_transfer_context ctx; > > + > > + mutex_lock(&dev->lock); > > + > > + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report)); > > + > > + dev->urb->context = &ctx; > > + init_completion(&ctx.done); > > + > > + ret = usb_submit_urb(dev->urb, GFP_NOIO); > > Here, usb_submit_urb() returns EBUSY in the first attempt to read random > numbers from the device: > > URB 917256d5d540 submitted while active > WARNING: CPU: 3 PID: 31 at drivers/usb/core/urb.c:363 > usb_submit_urb+0x4c2/0x5b0 > <...> > Call Trace: > u2fzero_rng_read+0x16e/0x340 [hid_u2fzero] > ? ttwu_do_activate+0x67/0x90 > add_early_randomness+0x53/0xc0 > hwrng_register+0x175/0x180 > devm_hwrng_register+0x41/0x7e > u2fzero_probe+0x2dd/0x350 [hid_u2fzero] > hid_device_probe+0x119/0x180 [hid] > really_probe+0xfe/0x3b0 > driver_probe_device+0xba/0x100 > __device_attach_driver+0x97/0x100 > > I don’t understand why since 1) it’s likely to be the first transmission > of this URB, 2) there’s a mutex locked just before it. I received a > comment from a colleague mentioning I’m probably not using the mutex > correctly, but I don’t understand why. > > I’m trying to figure this out, so I’d welcome any help with it. You can try using usbmon to see exactly what URBs are actually running. Alan Stern
[PATCH] USB: serial: option: add support for Cinterion CLS8 devices
Add support for the serial ports of Cinterion CLS8 devices. T: Bus=01 Lev=03 Prnt=05 Port=01 Cnt=02 Dev#= 25 Spd=480 MxCh= 0 D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 P: Vendor=1e2d ProdID=00b0 Rev= 3.18 S: Manufacturer=GEMALTO S: Product=USB Modem C:* #Ifs= 5 Cfg#= 1 Atr=80 MxPwr=500mA I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none) E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms I:* If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option E: Ad=83(I) Atr=03(Int.) MxPS= 10 Ivl=32ms E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option E: Ad=85(I) Atr=03(Int.) MxPS= 10 Ivl=32ms E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms I:* If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option E: Ad=87(I) Atr=03(Int.) MxPS= 10 Ivl=32ms E: Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms I:* If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan E: Ad=89(I) Atr=03(Int.) MxPS= 8 Ivl=32ms E: Ad=88(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms Signed-off-by: Reinhard Speyerer --- diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c index 38e920ac7f82..b4cd8ec23738 100644 --- a/drivers/usb/serial/option.c +++ b/drivers/usb/serial/option.c @@ -419,6 +419,7 @@ static void option_instat_callback(struct urb *urb); #define CINTERION_PRODUCT_PH8_AUDIO0x0083 #define CINTERION_PRODUCT_AHXX_2RMNET 0x0084 #define CINTERION_PRODUCT_AHXX_AUDIO 0x0085 +#define CINTERION_PRODUCT_CLS8 0x00b0 /* Olivetti products */ #define OLIVETTI_VENDOR_ID 0x0b3c @@ -1847,6 +1848,8 @@ static const struct usb_device_id option_ids[] = { .driver_info = RSVD(4) }, { USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_AHXX_2RMNET, 0xff) }, { USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_AHXX_AUDIO, 0xff) }, + { USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_CLS8, 0xff), + .driver_info = RSVD(0) | RSVD(4) }, { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_HC28_MDM) }, { USB_DEVICE(CINTERION_VENDOR_ID, CINTERION_PRODUCT_HC28_MDMNET) }, { USB_DEVICE(SIEMENS_VENDOR_ID, CINTERION_PRODUCT_HC25_MDM) },
RE: [PATCH 00/14] usb: typec: UCSI driver overhaul
Hi Heikki, > -Original Message- > From: Heikki Krogerus > Sent: Thursday, October 3, 2019 7:25 AM > To: Ajay Gupta > Cc: linux-usb@vger.kernel.org > Subject: Re: [PATCH 00/14] usb: typec: UCSI driver overhaul > > Hi Ajay, > > On Tue, Oct 01, 2019 at 06:36:25PM +, Ajay Gupta wrote: > > Hi Heikki > > > > > -Original Message- > > > From: Heikki Krogerus > > > Sent: Thursday, September 26, 2019 3:07 AM > > > To: Ajay Gupta > > > Cc: linux-usb@vger.kernel.org > > > Subject: [PATCH 00/14] usb: typec: UCSI driver overhaul > > > > > > Hi Ajay, > > > > > > Here's the pretty much complete rewrite of the I/O handling that I > > > was talking about. The first seven patches are not actually related > > > to this stuff, but I'm including them here because the rest of the > > > series is made on top of them. I'm including also that fix patch I > > > send you earlier. > > > > > > After this it should be easier to handle quirks. My idea how to > > > handle the multi-instance connector alt modes is that we "emulate" > > > the PPM in ucsi_ccg.c in order to handle them, so ucsi.c is not touched at > all. > > > > > > We can now get the connector alternate modes that the actual > > > controller supplies during probe - before registering the ucsi > > > interface > > How can ucsi_ccg.c get the connector alternate modes before > > registering ucsi interface? PPM reset, notification enable, etc. > > is done during ucsi registration. UCSI spec says: > > " The only commands the PPM is required to process in the "PPM Idle > > (Notifications Disabled)" state are SET_NOTIFICATION_ENABLE and > > PPM_RESE" > > > > Also, it doesn't look correct if ucsi_ccg.c has to replicate most of > > the stuff done in ucsi_init() of ucsi.c. > > How about if we split ucsi_init() into a function that first simply > constructs the > struct ucsi and struct ucsi_connector instances without registering anything, > and into separate functions that then register the ports, altmodes and what > have you. I don't think that should be a huge problem. It will make ucsi.c > even > more like a library, which is probable a good thing. Do you mean the solution to follow steps (a->b->c->d1) or (a->b->c->d2) ? a) ucsi_ccg.c calls first part of split ucsi_init() b) ucsi_ccg.c uses ucsi_send_command() to collect actual alternate modes. c) ucsi_ccg.c looks into actual alternate modes and squashes if duplicate altmode found. d1) ucsi_ccg.c calls new method to register connector alternate modes. This method issues GET_ALTERNATE_MODES command again and ucsi_ccg.c is expected to send squashed alternate mode. This will require changes in .read(), .sync_write() and .async_write() to make it appear as if the squashed data coming from the ppm. OR d2) ucsi_ccg.c calls new method to register squashed connector alternate modes. This method doesn't issue GET_ALTERNATE_MODES commands to PPM but simply registers the alternate mode values passed to this function. If you mean the (a->b->c->d2) solution then it looks fine to me and would wait for patches from you. This solution would mean that GET_ALTERNATE_MODES for connector is done only by each ucsi_xxx.c and not by ucsi.c > I can prepare patches for that too if you like? > After that you should be able to get the struct ucsi instance that represents > the "real" PPM without registering anything by calling a single function, most > likely ucsi_init(). And after getting that you can construct the connector > alternate modes that we actually register. > Finally you register the final interface which does not use ucsi_ccg_ops, but > instead something like ucsi_nvidia_ops. I didn't understand this part. ucsi_ccg_ops has .read(), .sync_write() and .async_write() interface and they remain same for all ucsi_ccg controllers. Thanks >nvpublic > > How would this sound to you? > > Br, > > -- > heikki
[PATCH] HID: Fix assumption that devices have inputs
The syzbot fuzzer found a slab-out-of-bounds write bug in the hid-gaff driver. The problem is caused by the driver's assumption that the device must have an input report. While this will be true for all normal HID input devices, a suitably malicious device can violate the assumption. The same assumption is present in over a dozen other HID drivers. This patch fixes them by checking that the list of hid_inputs for the hid_device is nonempty before allowing it to be used. Reported-and-tested-by: syzbot+403741a091bf41d4a...@syzkaller.appspotmail.com Signed-off-by: Alan Stern CC: --- [as1921] drivers/hid/hid-axff.c | 11 +-- drivers/hid/hid-dr.c | 12 +--- drivers/hid/hid-emsff.c | 12 +--- drivers/hid/hid-gaff.c | 12 +--- drivers/hid/hid-holtekff.c | 12 +--- drivers/hid/hid-lg2ff.c | 12 +--- drivers/hid/hid-lg3ff.c | 11 +-- drivers/hid/hid-lg4ff.c | 11 +-- drivers/hid/hid-lgff.c | 11 +-- drivers/hid/hid-logitech-hidpp.c | 11 +-- drivers/hid/hid-microsoft.c | 12 +--- drivers/hid/hid-sony.c | 12 +--- drivers/hid/hid-tmff.c | 12 +--- drivers/hid/hid-zpff.c | 12 +--- 14 files changed, 126 insertions(+), 37 deletions(-) Index: usb-devel/drivers/hid/hid-gaff.c === --- usb-devel.orig/drivers/hid/hid-gaff.c +++ usb-devel/drivers/hid/hid-gaff.c @@ -64,14 +64,20 @@ static int gaff_init(struct hid_device * { struct gaff_device *gaff; struct hid_report *report; - struct hid_input *hidinput = list_entry(hid->inputs.next, - struct hid_input, list); + struct hid_input *hidinput; struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list; struct list_head *report_ptr = report_list; - struct input_dev *dev = hidinput->input; + struct input_dev *dev; int error; + if (list_empty(&hid->inputs)) { + hid_err(hid, "no inputs found\n"); + return -ENODEV; + } + hidinput = list_entry(hid->inputs.next, struct hid_input, list); + dev = hidinput->input; + if (list_empty(report_list)) { hid_err(hid, "no output reports found\n"); return -ENODEV; Index: usb-devel/drivers/hid/hid-axff.c === --- usb-devel.orig/drivers/hid/hid-axff.c +++ usb-devel/drivers/hid/hid-axff.c @@ -63,13 +63,20 @@ static int axff_init(struct hid_device * { struct axff_device *axff; struct hid_report *report; - struct hid_input *hidinput = list_first_entry(&hid->inputs, struct hid_input, list); + struct hid_input *hidinput; struct list_head *report_list =&hid->report_enum[HID_OUTPUT_REPORT].report_list; - struct input_dev *dev = hidinput->input; + struct input_dev *dev; int field_count = 0; int i, j; int error; + if (list_empty(&hid->inputs)) { + hid_err(hid, "no inputs found\n"); + return -ENODEV; + } + hidinput = list_first_entry(&hid->inputs, struct hid_input, list); + dev = hidinput->input; + if (list_empty(report_list)) { hid_err(hid, "no output reports found\n"); return -ENODEV; Index: usb-devel/drivers/hid/hid-dr.c === --- usb-devel.orig/drivers/hid/hid-dr.c +++ usb-devel/drivers/hid/hid-dr.c @@ -75,13 +75,19 @@ static int drff_init(struct hid_device * { struct drff_device *drff; struct hid_report *report; - struct hid_input *hidinput = list_first_entry(&hid->inputs, - struct hid_input, list); + struct hid_input *hidinput; struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list; - struct input_dev *dev = hidinput->input; + struct input_dev *dev; int error; + if (list_empty(&hid->inputs)) { + hid_err(hid, "no inputs found\n"); + return -ENODEV; + } + hidinput = list_first_entry(&hid->inputs, struct hid_input, list); + dev = hidinput->input; + if (list_empty(report_list)) { hid_err(hid, "no output reports found\n"); return -ENODEV; Index: usb-devel/drivers/hid/hid-emsff.c === --- usb-devel.orig/drivers/hid/hid-emsff.c +++ usb-devel/drivers/hid/hid-emsff.c @@ -47,13 +47,19 @@ static int emsff_init(struct hid_device { struct emsff_device *emsff; struct hid_report *repo
Reminder: 67 active syzbot reports in usb subsystem
[This email was generated by a script. Let me know if you have any suggestions to make it better, or if you want it re-generated with the latest status.] Of the syzbot reports that have (re-)occurred in the last 7 days, I've manually marked 67 of them as possibly being bugs in the usb subsystem. This category mostly includes USB driver bugs, but it might include some core USB bugs too. I've listed these bug reports below. If you believe a bug is no longer valid, please close it by sending a '#syz fix', '#syz dup', or '#syz invalid' command in reply to the original thread, as explained at https://goo.gl/tpsmEJ#status If you believe I misattributed a bug to the usb subsystem, please let me know and (if possible) forward it to the correct place. Note: in total, I've actually assigned 97 open syzbot reports to this subsystem. But to help focus people's efforts, I've only listed the 67 that have (re-)occurred in the last week. Let me know if you want the full list. Here are the bug reports: Title: possible deadlock in mon_bin_vma_fault Last occurred: 0 days ago Reported: 395 days ago Branches: Mainline and others Dashboard link: https://syzkaller.appspot.com/bug?id=2b061d1fabd9760e98f92163543189b637c4af36 Original thread: https://lore.kernel.org/lkml/6ad6030574fea...@google.com/T/#u This bug has a C reproducer. No one replied to the original thread for this bug. If you fix this bug, please add the following tag to the commit: Reported-by: syzbot+56f9673bb4cdcbeb0...@syzkaller.appspotmail.com If you send any email or patch for this bug, please consider replying to the original thread. For the git send-email command to use, or tips on how to reply if the thread isn't in your mailbox, see the "Reply instructions" at https://lore.kernel.org/r/6ad6030574fea...@google.com Title: general protection fault in flexcop_usb_probe Last occurred: 0 days ago Reported: 174 days ago Branches: Mainline (with usb-fuzzer patches) Dashboard link: https://syzkaller.appspot.com/bug?id=c0203bd72037d07493f4b7562411e4f5f4553a8f Original thread: https://lore.kernel.org/lkml/10fe260586536...@google.com/T/#u This bug has a C reproducer. No one replied to the original thread for this bug. This looks like a bug in a media USB driver. If you fix this bug, please add the following tag to the commit: Reported-by: syzbot+d93dff37e6a89431c...@syzkaller.appspotmail.com If you send any email or patch for this bug, please consider replying to the original thread. For the git send-email command to use, or tips on how to reply if the thread isn't in your mailbox, see the "Reply instructions" at https://lore.kernel.org/r/10fe260586536...@google.com Title: KASAN: use-after-free Read in adu_disconnect Last occurred: 0 days ago Reported: 59 days ago Branches: Mainline (with usb-fuzzer patches) Dashboard link: https://syzkaller.appspot.com/bug?id=a8593e333f207fe272db7fff3bbc651d52562c9d Original thread: https://lore.kernel.org/lkml/d12d24058f5d6...@google.com/T/#u This bug has a C reproducer. The original thread for this bug has received 13 replies; the last was 13 days ago. This looks like a bug in a USB driver. If you fix this bug, please add the following tag to the commit: Reported-by: syzbot+0243cb250a51eeefb...@syzkaller.appspotmail.com If you send any email or patch for this bug, please reply to the original thread, which had activity only 13 days ago. For the git send-email command to use, or tips on how to reply if the thread isn't in your mailbox, see the "Reply instructions" at https://lore.kernel.org/r/d12d24058f5d6...@google.com Title: WARNING: ODEBUG bug in rsi_probe Last occurred: 0 days ago Reported: 172 days ago Branches: Mainline (with usb-fuzzer patches) Dashboard link: https://syzkaller.appspot.com/bug?id=3b35267abf182bd98ba95c0943bc0f957e021101 Original thread: https://lore.kernel.org/lkml/24bbd7058682e...@google.com/T/#u This bug has a C reproducer. No one replied to the original thread for this bug. This looks like a bug in a net/wireless USB driver. If you fix this bug, please add the following tag to the commit: Reported-by: syzbot+1d1597a5aa3679c65...@syzkaller.appspotmail.com If you send any email or patch for this bug, please consider replying to the original thread. For the git send-email command to use, or tips on how to reply if the thread isn't in your mailbox, see the "Reply instructions
Re: [PATCH] USB: serial: option: add support for Cinterion CLS8 devices
On Thu, Oct 03, 2019 at 06:53:21PM +0200, Reinhard Speyerer wrote: > Add support for the serial ports of Cinterion CLS8 devices. > > T: Bus=01 Lev=03 Prnt=05 Port=01 Cnt=02 Dev#= 25 Spd=480 MxCh= 0 > D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 > P: Vendor=1e2d ProdID=00b0 Rev= 3.18 > S: Manufacturer=GEMALTO > S: Product=USB Modem > C:* #Ifs= 5 Cfg#= 1 Atr=80 MxPwr=500mA > I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none) > E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > I:* If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > E: Ad=83(I) Atr=03(Int.) MxPS= 10 Ivl=32ms > E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > E: Ad=85(I) Atr=03(Int.) MxPS= 10 Ivl=32ms > E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > I:* If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option > E: Ad=87(I) Atr=03(Int.) MxPS= 10 Ivl=32ms > E: Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > I:* If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan > E: Ad=89(I) Atr=03(Int.) MxPS= 8 Ivl=32ms > E: Ad=88(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms > E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms > > Signed-off-by: Reinhard Speyerer Applied, thanks. Johan