Skip to contentSkip to conversation

: USB: cdc_acm: Fix oops when Droids MuIn LCD is connected

PATCH In review Erik Slagter 3 participants 5 messages May 11, 2011 3 messages not in the archive

Subsystems (2)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 20110510074421.GA2755@maxin

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw

PATCH 1/1 : USB: cdc_acm: Fix oops when Droids MuIn LCD is connected

From: Erik Slagter <[email protected]>

The Droids MuIn LCD operates like a serial remote terminal. Data received are displayed directly on the LCD. This patch fixes the kernel null pointer oops when it is plugged in.

Add NO_DATA_INTERFACE quirk to tell the driver that "control" and "data" interfaces are not separated for this device, which prevents dereferencing a null pointer in the device probe code.

---
This patch applies to vanilla kernel 2.6.38.2, but I am quite confident it will apply to any recent kernel. It was created by Maxin John after I located the problem, slightly modified by me and tested by me, so blame me if it breaks anything.

drivers/usb/class/cdc-acm.c +11 -2
index e057e53..caa2535 100644
@@ -946,7 +946,7 @@ static int acm_probe(struct usb_interface *intf,
 	u8 ac_management_function = 0;
 	u8 call_management_function = 0;
 	int call_interface_num = -1;
-	int data_interface_num;
+	int data_interface_num = -1;
 	unsigned long quirks;
 	int num_rx_buf;
 	int i;
@@ -1030,7 +1030,11 @@ next_desc:
 	if (!union_header) {
 		if (call_interface_num > 0) {
 			dev_dbg(&intf->dev, "No union descriptor, using call management descriptor\n");
-			data_interface = usb_ifnum_to_if(usb_dev, (data_interface_num = call_interface_num));
+			/* quirks for Droids MuIn LCD */
+			if (quirks & NO_DATA_INTERFACE)
+				data_interface = usb_ifnum_to_if(usb_dev, 0);
+			else
+				data_interface = usb_ifnum_to_if(usb_dev, (data_interface_num = call_interface_num));
 			control_interface = intf;
 		} else {
 			if (intf->cur_altsetting->desc.bNumEndpoints != 3) {
@@ -1622,6 +1626,11 @@ static const struct usb_device_id acm_ids[] = {
 	.driver_info = NOT_A_MODEM,
 	},
 
+	/* Support for Droids MuIn LCD */
+	{ USB_DEVICE(0x04d8, 0x000b),
+	.driver_info = NO_DATA_INTERFACE,
+	},
+
 	/* control interfaces without any protocol set */
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_ACM,
 		USB_CDC_PROTO_NONE) },
drivers/usb/class/cdc-acm.h +1 -0
index b4ea54d..683104a 100644
@@ -137,3 +137,4 @@ struct acm {
 #define SINGLE_RX_URB			2
 #define NO_CAP_LINE			4
 #define NOT_A_MODEM			8
+#define NO_DATA_INTERFACE		16
#4 lore raw
Quoted 10 lines · Erik Slagter

From: Erik Slagter <[email protected]>

The Droids MuIn LCD operates like a serial remote terminal. Data received are displayed directly on the LCD. This patch fixes the kernel null pointer oops when it is plugged in.

Add NO_DATA_INTERFACE quirk to tell the driver that "control" and "data" interfaces are not separated for this device, which prevents dereferencing a null pointer in the device probe code.

Quoted 8 lines

---
This patch applies to vanilla kernel 2.6.38.2, but I am quite confident it will apply to any recent kernel. It was created by Maxin John after I located the problem, slightly modified by me and tested by me, so blame me if it breaks anything.

If Maxin created it, it should have his "From:" on the top, right?

And did Maxin really sign-off-by: on the patch? You can not add other people's names to that without them really doing it, that's not allowed at all.

Care to redo it?

thanks,

greg k-h

#5 lore raw

Hi Greg,

Quoted 1 line

If Maxin created it, it should have his "From:" on the top, right?

It was a combined effort. I think, Erik has done the core part of it.

Quoted 3 lines

And did Maxin really sign-off-by: on the patch?  You can not add other people's names to that without them really doing it, that's not allowed at all.

Yes. I have added the "sign-off-by:" by myself. While we were developing and testing the patch, we have agreed to share the fame/blame together.

Quoted 1 line

Care to redo it?

Should we really re-do it for the "sign-off-by" issue ? If it is really required,
we are ready to redo it.

Best Regards,
Maxin

#6 lore raw
Quoted 5 lines · Maxin B John

Hi Greg,

Quoted 1 line

If Maxin created it, it should have his "From:" on the top, right?

It was a combined effort. I think, Erik has done the core part of it.

Ok, and from an email I just got from Erik, that's fine.

Quoted 6 lines
Quoted 3 lines

And did Maxin really sign-off-by: on the patch?  You can not add other people's names to that without them really doing it, that's not allowed at all.

Yes. I have added the "sign-off-by:" by myself. While we were developing and testing the patch, we have agreed to share the fame/blame together.

Ok, I wasn't aware of this.

Quoted 4 lines
Quoted 1 line

Care to redo it?

Should we really re-do it for the "sign-off-by" issue ? If it is really required,
we are ready to redo it.

No, that's fine, I was not aware that both of you agreed to this. I'll add it to my "to-apply" queue and you will get an automated email when it is applied to the tree.

thanks,

greg k-h

#7 lore raw
Quoted 3 lines

No, that's fine, I was not aware that both of you agreed to this. I'll add it to my "to-apply" queue and you will get an automated email when it is applied to the tree.

Great, thanks!