[PATCH] usb: cdc_acm: Add quirk for Elatec TWN3
Elatec TWN3 has the union descriptor on data interface. This results in
failure to bind the device to the driver with the following log:
usb 1-1.2: new full speed USB device using streamplug-ehci and address 4
usb 1-1.2: New USB device found, idVendor=09d8, idProduct=0320
usb 1-1.2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
usb 1-1.2: Product: RFID Device (COM)
usb 1-1.2: Manufacturer: OEM
cdc_acm 1-1.2:1.0: Zero length descriptor references
cdc_acm: probe of 1-1.2:1.0 failed with error -22
Adding the NO_UNION_NORMAL quirk for the device fixes the issue.
Signed-off-by: Maksim Salau
---
`lsusb -v` of the device:
Bus 001 Device 003: ID 09d8:0320
Device Descriptor:
bLength18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass2 Communications
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize032
idVendor 0x09d8
idProduct 0x0320
bcdDevice3.00
iManufacturer 1 OEM
iProduct2 RFID Device (COM)
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 67
bNumInterfaces 2
bConfigurationValue 1
iConfiguration 0
bmAttributes 0x80
(Bus Powered)
MaxPower 250mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 2 Communications
bInterfaceSubClass 2 Abstract (modem)
bInterfaceProtocol 1 AT-commands (v.25ter)
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes3
Transfer TypeInterrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 2
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber1
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass10 CDC Data
bInterfaceSubClass 0 Unused
bInterfaceProtocol 0
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes2
Transfer TypeBulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes2
Transfer TypeBulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 0
CDC Header:
bcdCDC 1.10
CDC Call Management:
bmCapabilities 0x03
call management
use DataInterface
bDataInterface 1
CDC ACM:
bmCapabilities 0x06
sends break
line coding and serial state
CDC Union:
bMasterInterface0
bSlaveInterface 1
Device Status: 0x
(Bus Powered)
drivers/usb/class/cdc-acm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 5e056064..18c923a 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1832,6 +1832,9 @@ static const struct usb_device_id acm_ids[] = {
{ USB_DEVICE(0xfff0, 0x0100), /* DATECS FP-2000 */
.driver_info = NO_UNION_NORMAL, /* reports zero length descriptor */
},
+ { USB_DEVICE(0x09d8, 0x0320), /* Elatec GmbH TWN3 */
+ .driver_info = NO_UNION_NORMAL, /* has misplaced union descriptor */
+ },
{ USB_DEVICE(0x2912, 0x0001), /* ATOL FPrint */
.driver_info = CLEAR_HALT_CONDITIONS,
--
2.7.4
Re: [PATCH 3.16 084/134] usb: misc: legousbtower: Fix buffers on stack
On Fri, 18 Aug 2017 14:13:20 +0100 Ben Hutchings wrote: > 3.16.47-rc1 review patch. If anyone has any objections, please let me know. > > -- > > From: Maksim Salau > > commit 942a48730faf149ccbf3e12ac718aee120bb3529 upstream. > > Allocate buffers on HEAP instead of STACK for local structures > that are to be received using usb_control_msg(). Hi Ben, The change has a memory leak, which is fixed by the commit 0bd193d62b4270a2a7a09da43ad1034c7ca5b3d3 If the commit is not in your queue, please add it. Thanks, Maksim.
[PATCH v2] usb: misc: legousbtower: Fix memory leak
get_version_reply is not freed if function returns with success.
Fixes: 942a48730faf ("usb: misc: legousbtower: Fix buffers on stack")
Reported-by: Heikki Krogerus
Signed-off-by: Maksim Salau
---
v2:
Changed tags to match guidelines.
drivers/usb/misc/legousbtower.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index aa3c280fdf8d..0782ac6f5edf 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -926,6 +926,7 @@ static int tower_probe (struct usb_interface *interface,
const struct usb_device
USB_MAJOR, dev->minor);
exit:
+ kfree(get_version_reply);
return retval;
error:
--
2.9.3
[PATCH] usb: misc: legousbtower: Fix memory leak
get_version_reply is not freed if function returns with success. Memory leak was introduced by commit 942a48730faf149ccbf3e12ac718aee120bb3529 Signed-off-by: Heikki Krogerus Signed-off-by: Maksim Salau --- drivers/usb/misc/legousbtower.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c index aa3c280..0782ac6 100644 --- a/drivers/usb/misc/legousbtower.c +++ b/drivers/usb/misc/legousbtower.c @@ -926,6 +926,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device USB_MAJOR, dev->minor); exit: + kfree(get_version_reply); return retval; error: -- 2.9.3
Re: [PATCH v3] usb: misc: legousbtower: Fix buffers on stack
> > @@ -913,6 +929,7 @@ static int tower_probe (struct usb_interface > > *interface, const struct usb_device > > Don't you need to free get_version_reply here? > > > return retval; > > > > error: > > + kfree(get_version_reply); > > tower_delete(dev); > > return retval; > > } Thank you very much, Heikki! I was so focused on failure cases, that missed memory leak in the case when all calls succeeded. I'll prepare a patch shortly to fix this. Regards, Maksim.
Re: [PATCH v3] usb: misc: legousbtower: Fix buffers on stack
> >* removed Tested-by: Alfredo Rafael Vicente Boix ; > > I added this back, as it matters, and your change from the previous > version was trivial. > > >* removed Cc: sta...@vger.kernel.org > > since this patch doesn't apply against v4.10.12 > > I added this back as well :) Thanks, Greg! I was not sure about how strict are the rules about these tags. Maksim.
[PATCH v3] usb: misc: legousbtower: Fix buffers on stack
Allocate buffers on HEAP instead of STACK for local structures
that are to be received using usb_control_msg().
Signed-off-by: Maksim Salau
---
Changes in v3:
* rebased against usb-next;
* removed Tested-by: Alfredo Rafael Vicente Boix ;
* removed Cc: sta...@vger.kernel.org
since this patch doesn't apply against v4.10.12
Changes in v2:
* made checkpatch happy with the format string passed to dev_info
in tower_probe() (merged two parts into a single string literal);
* changed commit message to better reflect location of the module;
was: USB: legousbtower: Fix buffers on stack;
* added Tested-by: Alfredo Rafael Vicente Boix
and Cc: sta...@vger.kernel.org
drivers/usb/misc/legousbtower.c | 37 +++--
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 201c9c3..aa3c280 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -317,9 +317,16 @@ static int tower_open (struct inode *inode, struct file
*file)
int subminor;
int retval = 0;
struct usb_interface *interface;
- struct tower_reset_reply reset_reply;
+ struct tower_reset_reply *reset_reply;
int result;
+ reset_reply = kmalloc(sizeof(*reset_reply), GFP_KERNEL);
+
+ if (!reset_reply) {
+ retval = -ENOMEM;
+ goto exit;
+ }
+
nonseekable_open(inode, file);
subminor = iminor(inode);
@@ -364,8 +371,8 @@ static int tower_open (struct inode *inode, struct file
*file)
USB_TYPE_VENDOR | USB_DIR_IN |
USB_RECIP_DEVICE,
0,
0,
- &reset_reply,
- sizeof(reset_reply),
+ reset_reply,
+ sizeof(*reset_reply),
1000);
if (result < 0) {
dev_err(&dev->udev->dev,
@@ -406,6 +413,7 @@ static int tower_open (struct inode *inode, struct file
*file)
mutex_unlock(&dev->lock);
exit:
+ kfree(reset_reply);
return retval;
}
@@ -806,7 +814,7 @@ static int tower_probe (struct usb_interface *interface,
const struct usb_device
struct device *idev = &interface->dev;
struct usb_device *udev = interface_to_usbdev(interface);
struct lego_usb_tower *dev = NULL;
- struct tower_get_version_reply get_version_reply;
+ struct tower_get_version_reply *get_version_reply = NULL;
int retval = -ENOMEM;
int result;
@@ -871,6 +879,13 @@ static int tower_probe (struct usb_interface *interface,
const struct usb_device
dev->interrupt_in_interval = interrupt_in_interval ?
interrupt_in_interval : dev->interrupt_in_endpoint->bInterval;
dev->interrupt_out_interval = interrupt_out_interval ?
interrupt_out_interval : dev->interrupt_out_endpoint->bInterval;
+ get_version_reply = kmalloc(sizeof(*get_version_reply), GFP_KERNEL);
+
+ if (!get_version_reply) {
+ retval = -ENOMEM;
+ goto error;
+ }
+
/* get the firmware version and log it */
result = usb_control_msg (udev,
usb_rcvctrlpipe(udev, 0),
@@ -878,18 +893,19 @@ static int tower_probe (struct usb_interface *interface,
const struct usb_device
USB_TYPE_VENDOR | USB_DIR_IN |
USB_RECIP_DEVICE,
0,
0,
- &get_version_reply,
- sizeof(get_version_reply),
+ get_version_reply,
+ sizeof(*get_version_reply),
1000);
if (result < 0) {
dev_err(idev, "LEGO USB Tower get version control request
failed\n");
retval = result;
goto error;
}
- dev_info(&interface->dev, "LEGO USB Tower firmware version is %d.%d "
-"build %d\n", get_version_reply.major,
-get_version_reply.minor,
-le16_to_cpu(get_version_reply.build_no));
+ dev_info(&interface->dev,
+"LEGO USB Tower firmware version is %d.%d build %d\n",
+get_version_reply->major,
+get_version_reply->minor,
+le16_to_cpu(get_version_reply->build_no));
/* we can register the device now, as it is ready */
usb_set_intfdata (interface, dev);
@@ -913,6 +929,7 @@ static int tower_probe (struct usb_interface *interface,
const struct usb_device
return retval;
error:
+ kfree(get_version_reply);
tower_delete(dev);
return retval;
}
--
2.9.3
Re: [PATCH v2] usb: core: Warn if an URB's transfer_buffer is on stack
> + } else if (object_is_on_stack(urb->transfer_buffer)) {
> + WARN_ONCE(1, "transfer buffer is on stack\n");
> + ret = -EAGAIN;
> } else {
Hi,
Has anyone considered a fail-safe mode? I.e.: if a buffer is on stack,
kmemdup it and continue with a warning. This will give us both: functional
drivers (with possibly decreased efficiency in speed and memory footprint)
and warnings for developers that a particular driver requires attention.
This mode will not affect drivers which obey the rules, but will make
offenders at least functional. My main concern is that not every user is able
to detect and report a problem, which prevents drivers from functioning.
Especially this is a problem for not wide spread devices.
Due to this users a seeing unusable equipment, but developers are not
aware of those, even if fixes are trivial.
Such mode has a also a negative effect: if a developer has a device
with an offending driver, he can miss the warning message, since the driver
just works.
Regards,
Maksim.
[PATCH] usb: serial: upd78f0730: Make constants static
Some local constants don't change from call to call and are good
candidates to become static. This will prevent copying of these
constants to stack during runtime.
Signed-off-by: Maksim Salau
---
drivers/usb/serial/upd78f0730.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/upd78f0730.c b/drivers/usb/serial/upd78f0730.c
index a028dd2..6819a34 100644
--- a/drivers/usb/serial/upd78f0730.c
+++ b/drivers/usb/serial/upd78f0730.c
@@ -288,7 +288,7 @@ static void upd78f0730_dtr_rts(struct usb_serial_port
*port, int on)
static speed_t upd78f0730_get_baud_rate(struct tty_struct *tty)
{
const speed_t baud_rate = tty_get_baud_rate(tty);
- const speed_t supported[] = {
+ static const speed_t supported[] = {
0, 2400, 4800, 9600, 19200, 38400, 57600, 115200, 153600
};
int i;
@@ -384,7 +384,7 @@ static void upd78f0730_set_termios(struct tty_struct *tty,
static int upd78f0730_open(struct tty_struct *tty, struct usb_serial_port
*port)
{
- struct upd78f0730_open_close request = {
+ static const struct upd78f0730_open_close request = {
.opcode = UPD78F0730_CMD_OPEN_CLOSE,
.state = UPD78F0730_PORT_OPEN
};
@@ -402,7 +402,7 @@ static int upd78f0730_open(struct tty_struct *tty, struct
usb_serial_port *port)
static void upd78f0730_close(struct usb_serial_port *port)
{
- struct upd78f0730_open_close request = {
+ static const struct upd78f0730_open_close request = {
.opcode = UPD78F0730_CMD_OPEN_CLOSE,
.state = UPD78F0730_PORT_CLOSE
};
--
2.9.3
