Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1208659 > unrolled thread

[PATCH 0/2] USB: symbolserial: Fix OOPS (regression) and data length

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] USB: symbolserial: Fix OOPS (regression) and data length Philipp Hachtmann <hachti@hachti.de> - 2015-08-17 17:50 +0200
    [PATCH 2/2] USB: symbolserial: Correct transferred data size Philipp Hachtmann <hachti@hachti.de> - 2015-08-17 17:50 +0200

#1208659 — [PATCH 0/2] USB: symbolserial: Fix OOPS (regression) and data length

I bought a Symbol DS3508 2D imager barcode scanner, configured it to
behave as "simple COM port", and my machine OOPSed immediately.

The result are the two attached patches. The first one is actually needed
to get the whole thing again without that nasty NULL pointer dereference.

The second patch makes the driver exclude useless zero bytes from the payload
data. I assume this could lead to discussions because I found (and removed)
a comment that says that the driver will send all data from the urb buffer to
the tty layer without taking the communicated length (from the scanner, in
the first byte of a buffer) into account.

For your amusement here's what happened to my machine before writing the first
patch:

[ 3801.712295] usb 6-2: new full-speed USB device number 12 using uhci_hcd
[ 3801.883708] usb 6-2: New USB device found, idVendor=05e0, idProduct=0600
[ 3801.883712] usb 6-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 3801.883714] usb 6-2: Product: Symbol Bar Code Scanner
[ 3801.883716] usb 6-2: Manufacturer: Symbol Technologies, Inc, 2008
[ 3801.883718] usb 6-2: SerialNumber: S/N:5520B838ED22C34CBC619E74103732B8 Rev:PAAALS00-002-R01-
[ 3801.889804] symbolserial 6-2:1.0: symbol converter detected
[ 3801.889962] usb 6-2: symbol converter now attached to ttyUSB0
[ 3801.896738] BUG: unable to handle kernel NULL pointer dereference at           (null)
[ 3801.896741] IP: [<ffffffff817ef2ab>] _raw_spin_lock_irqsave+0xb/0x30
[ 3801.896746] PGD 61fb5b067 PUD 6208dd067 PMD 0 
[ 3801.896748] Oops: 0002 [#1] SMP 
[ 3801.896749] Modules linked in: nvidia(O)
[ 3801.896751] CPU: 7 PID: 8640 Comm: ModemManager Tainted: G           O    4.2.0-rc7 #27
[ 3801.896752] Hardware name: System manufacturer System Product Name/P6T WS PRO, BIOS 1204    09/16/2010
[ 3801.896753] task: ffff88061daaba80 ti: ffff8800ba608000 task.ti: ffff8800ba608000
[ 3801.896754] RIP: 0010:[<ffffffff817ef2ab>]  [<ffffffff817ef2ab>] _raw_spin_lock_irqsave+0xb/0x30
[ 3801.896756] RSP: 0018:ffff8800ba60bba8  EFLAGS: 00010046
[ 3801.896757] RAX: 0000000000000000 RBX: 0000000000000296 RCX: ffff8806220b4800
[ 3801.896757] RDX: 0000000000000001 RSI: ffff8806220b4800 RDI: 0000000000000000
[ 3801.896758] RBP: ffff8806220b4800 R08: 0000000000000001 R09: ffffc900004d6000
[ 3801.896759] R10: 00003ffffffff000 R11: ffffc900004d5fff R12: ffff880623832780
[ 3801.896759] R13: ffff880591b09000 R14: ffff880591b09000 R15: ffff88061daaba80
[ 3801.896760] FS:  00007fc35e569840(0000) GS:ffff88063fce0000(0000) knlGS:0000000000000000
[ 3801.896761] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3801.896762] CR2: 0000000000000000 CR3: 000000061df01000 CR4: 00000000000006e0
[ 3801.896762] Stack:
[ 3801.896763]  0000000000000000 ffffffff8153267b ffff8806243dd0a8 ffff8806220b4808
[ 3801.896764]  ffff8806238327e8 ffffffff81511cc6 ffff880591b09000 ffffffff813c4b2e
[ 3801.896765]  ffff880591b09000 ffff8806220b4808 ffff880591b09000 ffff8806220b4918
[ 3801.896766] Call Trace:
[ 3801.896769]  [<ffffffff8153267b>] ? symbol_open+0x1b/0x70
[ 3801.896772]  [<ffffffff81511cc6>] ? serial_port_activate+0x56/0x90
[ 3801.896775]  [<ffffffff813c4b2e>] ? tty_port_tty_set+0x2e/0x90
[ 3801.896776]  [<ffffffff813c50c6>] ? tty_port_open+0x76/0xc0
[ 3801.896778]  [<ffffffff813bd94f>] ? tty_init_dev+0x8f/0x1a0
[ 3801.896779]  [<ffffffff813bdb55>] ? tty_open+0xf5/0x5a0
[ 3801.896782]  [<ffffffff8118c2f0>] ? mount_fs+0x160/0x160
[ 3801.896784]  [<ffffffff8118cabc>] ? chrdev_open+0x9c/0x170
[ 3801.896785]  [<ffffffff8118ca20>] ? cdev_put+0x20/0x20
[ 3801.896786]  [<ffffffff81186d00>] ? do_dentry_open+0x1c0/0x2f0
[ 3801.896788]  [<ffffffff81194bd0>] ? path_openat+0x380/0x1030
[ 3801.896789]  [<ffffffff81191982>] ? terminate_walk+0xa2/0xb0
[ 3801.896790]  [<ffffffff81196a60>] ? do_filp_open+0x70/0xd0
[ 3801.896792]  [<ffffffff811a1a97>] ? __alloc_fd+0x37/0x100
[ 3801.896794]  [<ffffffff8118808e>] ? do_sys_open+0x11e/0x200
[ 3801.896795]  [<ffffffff817ef617>] ? entry_SYSCALL_64_fastpath+0x12/0x6a
[ 3801.896795] Code: 31 c0 ba 01 00 00 00 f0 0f b1 17 85 c0 75 02 f3 c3 89 c6 e9 68 6d 91 ff 0f 1f 84 00 00 00 00 00 53 9c 5b fa 31 c0 ba 01 00 00 00 <f0> 0f b1 17 85 c0 75 05 48 89 d8 5b c3 89 c6 e8 41 6d 91 ff 48 
[ 3801.896809] RIP  [<ffffffff817ef2ab>] _raw_spin_lock_irqsave+0xb/0x30
[ 3801.896811]  RSP <ffff8800ba60bba8>
[ 3801.896812] CR2: 0000000000000000
[ 3801.896813] ---[ end trace 196415182e1cd411 ]---


Philipp Hachtmann (2):
  USB: symbolserial: Use usb_get_serial_port_data
  USB: symbolserial: Correct transferred data size

 drivers/usb/serial/symbolserial.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1208668 — [PATCH 2/2] USB: symbolserial: Correct transferred data size

The scanner (here DS3508) always returns 64 bytes per urb buffer. The first
byte indicates the data length used in the current buffer. There even was
a comment describing this. But the comment also said that we'll send
everything in the buffer to the tty layer. That means sending the actual
barcode data and lots of trailing zeroes. This patch lets the driver only
send the real data.

Signed-off-by: Philipp Hachtmann <hachti@hachti.de>
---
 drivers/usb/serial/symbolserial.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/serial/symbolserial.c b/drivers/usb/serial/symbolserial.c
index 6ed8044..37f3ad1 100644
--- a/drivers/usb/serial/symbolserial.c
+++ b/drivers/usb/serial/symbolserial.c
@@ -60,17 +60,15 @@ static void symbol_int_callback(struct urb *urb)
 
 	usb_serial_debug_data(&port->dev, __func__, urb->actual_length, data);
 
+	/*
+	 * Data from the device comes with a 1 byte header:
+	 *
+	 * <size of data> <data>...
+	 */
 	if (urb->actual_length > 1) {
-		data_length = urb->actual_length - 1;
-
-		/*
-		 * Data from the device comes with a 1 byte header:
-		 *
-		 * <size of data>data...
-		 * 	This is real data to be sent to the tty layer
-		 * we pretty much just ignore the size and send everything
-		 * else to the tty layer.
-		 */
+		data_length = data[0];
+		if (data_length > (urb->actual_length - 1))
+			data_length = urb->actual_length - 1;
 		tty_insert_flip_string(&port->port, &data[1], data_length);
 		tty_flip_buffer_push(&port->port);
 	} else {
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web