Phil and I found out a problem with commit:
7e860a6e7a ("cdc-acm: add sanity checks")
It added some sanity checks to ignore potential garbage in CDC headers but
also introduced a potential infinite loop. This can happen at the first
loop iteration (elength = 0 in that case) if the description isn't a
DT_CS_INTERFACE or later if 'buffer[0]' is zero.
It should also be noted that the wrong length was being added to 'buffer'
in case 'buffer[1]' was not a DT_CS_INTERFACE descriptor, since elength was
assigned after that check in the loop.
A specially crafted USB device could be used to trigger this infinite loop.
Fixes: 7e860a6e7a ("cdc-acm: add sanity checks")
Signed-off-by: Phil Turnbull <phil.turnbull@oracle.com>
Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
CC: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
CC: Oliver Neukum <oneukum@suse.de>
CC: Adam Lee <adam8157@gmail.com>
CC: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
One more case of error codes not correctly being
correctly returned to user space.
Signed-off-by: Olive Neukum <oneukum@suse.com>0
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Values directly from descriptors given in debug statements
must be converted to native endianness.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
CC: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This makes sure the error handling path is the same for
all error conditions, thus reducing code duplication.
Signed-off-by: Oliver Neukum <oneukum@suse.de>0
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
A step on the road to passing status as a parameter
Signed-off-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
During the entry intro suspend a misleading message can be
printed. Surpress it by checking the specific error.
Signed-off-by: Oliver Neukum <oneukum@suse.de>0
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Abn URB may be may marked free only after the buffer has been
processed or there is a small window during which it could
be submitted on another CPU and overwrite an unprocessed buffer
Signed-off-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Lieing to user space is wrong. The real reason for a failure
to write should be returned to user space.
Signed-off-by: Oliver Neukum <oneukum@suse.de>0
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
In order to support an older USB cradle by Denso, I added its vendor- and product-ID to the array of usb_device_id acm_ids. In this way cdc-acm feels responsible for this cradle. The related /dev/ttyACM node is being created properly, and the data transfer works.
However, later cradle models by Denso do have proper descriptors, so the patch is not required for these. At the same time both the older and the later model have the same vendor- and product-ID, but they both work with the patched driver.
Declaration of the Denso cradles I tested:
- both models have the same IDs: vendorID 0x076d, productID 0x0006
- older model: Denso CU-321 (descriptors not properly set)
- later model: Denso CU-821 (with proper descriptors)
Signed-off-by: Bjoern Gerhart <oss@airbjorn.de>
Acked-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Memory allocation failures are reported by a central facility.
No need to repeat the job.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Check the special CDC headers for a plausible minimum length.
Another big operating systems ignores such garbage.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
CC: stable@vger.kernel.org
Reviewed-by: Adam Lee <adam8157@gmail.com>
Tested-by: Adam Lee <adam8157@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We need to check that we have both a valid data and control inteface for both
types of headers (union and not union.)
References: https://bugzilla.kernel.org/show_bug.cgi?id=83551
Reported-by: Simon Schubert <2+kernel@0x2c.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If probe() fails not only the attributes need to be removed
but also the memory freed.
Reported-by: Ahmed Tamrawi <ahmedtamrawi@gmail.com>
Signed-off-by: Oliver Neukum <oneukum@suse.de>
CC: stable@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add new quirk for devices that cannot handle control-line state
requests.
Note that we currently send these requests to all devices, regardless of
whether they claim to support it, but that errors are only logged if
support is claimed.
Since commit 0943d8ead3 ("USB: cdc-acm: use tty-port dtr_rts"), which
only changed the timings for these requests slightly, this has been
reported to cause occasional firmware crashes on Simtec Electronics
Entropy Key devices after re-enumeration. Enable the quirk for this
device.
Reported-by: Nix <nix@esperi.org.uk>
Tested-by: Nix <nix@esperi.org.uk>
Cc: stable <stable@vger.kernel.org> # v3.16
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Make sure to only raise DTR on transitions from B0 in set_termios.
Also allow set_termios to be called from open with a termios_old of
NULL. Note that DTR will not be raised prematurely in this case.
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The memory subsystem has already had similar message for it.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Let's drop the warning for modems with unusual capabilities,
the associated quirk and blacklist. They made little sense.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The driver wasn't properly configuring the hardware for the current
termios settings under all conditions. Ensure that termios are
written to the device when the port is activated.
Signed-off-by: Jim Paris <jim@jtan.com>
Reviewed-by: Johan Hovold <johan@kernel.org>
Acked-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add device-id entry for GW Instek AFG-2225, which has a byte swapped
bInterfaceSubClass (0x20).
Reported-by: Karl Palsson <karlp@tweak.net.au>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
There is a risk that the variable will be used without being initialized.
This was largely found by using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
send_request_dev_dep_msg_in() use a buffer allocated on the stack.
Fix by kmalloc()ing the buffer.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Converting the header to BIT for readability. No functional
change.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add dtr_rts tty-port operation which implements proper DTR/RTS handling
(e.g. only lower DTR/RTS during shutdown if HUPCL is set).
Note that modem-control locking still needs to be added throughout the
driver.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
There's no need to call usb_mark_last_busy after having increased the PM
counter in write(). The device will be marked busy by USB core when the
PM counter is balanced in the completion handler.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
There's no need to update the runtime PM last_busy field on read urb
errors (e.g. when the urb is being killed on shutdown).
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that acm_set_control() handles runtime PM properly, the only
remaining reason for the PM operations in shutdown is to clear the
needs_remote_wakeup flag before the final put.
Note that this also means that we now need to grab the write_lock to
prevent racing with resume.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove redundant disconnect test from shutdown(), which is never called
post disconnect() where we do synchronous hangup.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We can simply the runtime PM locking as there's no need to check the
susp_count in the read path (at least not since killing the rx tasklet).
Specifically, the read urbs will never be resubmitted by the completion
handler when killed during suspend.
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Make sure only to decrement the PM counters if they were actually
incremented.
Note that the USB PM counter, but not necessarily the driver core PM
counter, is reset when the interface is unbound.
Fixes: 11ea859d64 ("USB: additional power savings for cdc-acm devices
that support remote wakeup")
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Make sure to kill any already submitted read urbs on read-urb submission
failures in open in order to prevent doing I/O for a closed port.
Fixes: 088c64f812 ("USB: cdc-acm: re-write read processing")
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Fix errors during open not being returned to userspace. Specifically,
failed control-line manipulations or control or read urb submissions
would not be detected.
Fixes: 7fb57a019f ("USB: cdc-acm: Fix potential deadlock (lockdep
warning)")
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We must not do the usb_autopm_put_interface() before submitting the read
urbs or we might end up doing I/O to a suspended device.
Fixes: 088c64f812 ("USB: cdc-acm: re-write read processing")
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Make sure to check return value of autopm get in write() in order to
avoid urb leak and PM counter imbalance on errors.
Fixes: 11ea859d64 ("USB: additional power savings for cdc-acm devices
that support remote wakeup")
Cc: <stable@vger.kernel.org> # v2.6.27
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We should stop I/O unconditionally at suspend rather than rely on the
tty-port initialised flag (which is set prior to stopping I/O during
shutdown) in order to prevent suspend returning with URBs still active.
Fixes: 11ea859d64 ("USB: additional power savings for cdc-acm devices
that support remote wakeup")
Cc: <stable@vger.kernel.org> # v2.6.27
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Fix runtime PM handling of control messages by adding the required PM
counter operations.
Fixes: 11ea859d64 ("USB: additional power savings for cdc-acm devices
that support remote wakeup")
Cc: <stable@vger.kernel.org> # v2.6.27
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The current ACM runtime-suspend implementation is broken in several
ways:
Firstly, it buffers only the first write request being made while
suspended -- any further writes are silently dropped.
Secondly, writes being dropped also leak write urbs, which are never
reclaimed (until the device is unbound).
Thirdly, even the single buffered write is not cleared at shutdown
(which may happen before the device is resumed), something which can
lead to another urb leak as well as a PM usage-counter leak.
Fix this by implementing a delayed-write queue using urb anchors and
making sure to discard the queue properly at shutdown.
Fixes: 11ea859d64 ("USB: additional power savings for cdc-acm devices
that support remote wakeup")
Reported-by: Xiao Jin <jin.xiao@intel.com>
Cc: <stable@vger.kernel.org> # v2.6.27
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Fix race between write() and resume() due to improper locking that could
lead to writes being reordered.
Resume must be done atomically and susp_count be protected by the
write_lock in order to prevent racing with write(). This could otherwise
lead to writes being reordered if write() grabs the write_lock after
susp_count is decremented, but before the delayed urb is submitted.
Fixes: 11ea859d64 ("USB: additional power savings for cdc-acm devices
that support remote wakeup")
Cc: <stable@vger.kernel.org> # v2.6.27
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Fix race between write() and suspend() which could lead to writes being
dropped (or I/O while suspended) if the device is runtime suspended
while a write request is being processed.
Specifically, suspend() releases the write_lock after determining the
device is idle but before incrementing the susp_count, thus leaving a
window where a concurrent write() can submit an urb.
Fixes: 11ea859d64 ("USB: additional power savings for cdc-acm devices
that support remote wakeup")
Cc: <stable@vger.kernel.org> # v2.6.27
Signed-off-by: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
By specifying NO_UNION_NORMAL the ACM driver does only use the first two
USB interfaces (modem data & control). The AT Port, Diagnostic and NMEA
interfaces are left to the USB serial driver.
Signed-off-by: Michael Ulbricht <michael.ulbricht@systec-electronic.com>
Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: Oliver Neukum <oliver@neukum.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If acm_submit_read_urbs() fails in acm_port_activate(), error handling
code calls usb_autopm_put_interface() while it is already called
before acm_submit_read_urbs(). The patch reorganizes error handling code
to avoid double decrement of USB interface's PM-usage counter.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Acked-by: Oliver Neukum <oliver@neukum.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Do not decrement resp_count if it's already 0.
We set resp_count to 0 when the device is closed. The next open and
read will try to clear the WDM_READ flag if there was leftover data
in the read buffer. This fix is necessary to prevent resubmitting
the read URB in a tight loop because resp_count becomes negative.
The bug can easily be triggered from userspace by not reading all
data in the read buffer, and then closing and reopening the chardev.
Fixes: 8dd5cd5395 ("usb: cdc-wdm: avoid hanging on zero length reads")
Cc: <stable@vger.kernel.org> # 3.13
Signed-off-by: Bjørn Mork <bjorn@mork.no>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
None of these files are actually using any __init type directives
and hence don't need to include <linux/init.h>. Most are just a
left over from __devinit and __cpuinit removal, or simply due to
code getting copied from one driver to the next.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 73e06865ea ("USB: cdc-wdm: support back-to-back
USB_CDC_NOTIFY_RESPONSE_AVAILABLE notifications") implemented
queued response handling. This added a new requirement: The read
urb must be resubmitted every time we clear the WDM_READ flag if
the response counter indicates that the device is waiting for a
read.
Fix by factoring out the code handling the WMD_READ clearing and
possible urb submission, calling it everywhere we clear the flag.
Without this fix, the driver ends up in a state where the read urb
is inactive, but the response counter is positive after a zero
length read. This prevents the read urb from ever being submitted
again and the driver appears to be hanging.
Fixes: 73e06865ea ("USB: cdc-wdm: support back-to-back USB_CDC_NOTIFY_RESPONSE_AVAILABLE notifications")
Cc: Greg Suarez <gsuarez@smithmicro.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
Cc: stable <stable@vger.kernel.org> # 3.13
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
An ioctl that does depends on communication with a device should
prevent suspension of teh device.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Simple straightforward implementation. Just returning the statistics
gathered for TIOCMIWAIT
Signed-off-by: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Quentin Casasnovas