linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] keyspan: init termios properly
@ 2007-11-15 21:10 Lucy McCoy
  2007-11-16  6:24 ` Borislav Petkov
  2007-11-18 13:11 ` Borislav Petkov
  0 siblings, 2 replies; 14+ messages in thread
From: Lucy McCoy @ 2007-11-15 21:10 UTC (permalink / raw)
  To: bbpetkov; +Cc: Greg KH, linux-kernel, Andrew Morton

Hi Boris,

Actually the 19HS is the usa90 so it is included in the switch but
that isn't a problem.

I'm not familiar with the termios stuff on Linux so can you take a look
at the following modified code to see if this solves your NULL ptr problem?

Also,....

Does cflag need to be set to anything other than CS8?  
Is keyspan_open_port() always followed by a keyspan_set_termios()?

Thanks,

Lucy


static int keyspan_open (struct usb_serial_port *port, struct file *filp)
{
	struct keyspan_port_private 	*p_priv;
	struct keyspan_serial_private 	*s_priv;
	struct usb_serial 		*serial = port->serial;
	const struct keyspan_device_details	*d_details;
	int				i, err;
	struct urb			*urb;
	
	s_priv = usb_get_serial_data(serial);
	p_priv = usb_get_serial_port_data(port);
	d_details = p_priv->device_details;
	
	dbg("%s - port%d.", __FUNCTION__, port->number); 

	/* Set some sane defaults */
	p_priv->rts_state = 1;
	p_priv->dtr_state = 1;
	p_priv->baud = 9600;

	/* set CTS/RTS handshake etc. */
	p_priv->cflag = CS8;   /* 8/N/1  */
	p_priv->flow_control = flow_none;

	/* force baud and lcr to be set on open */
	p_priv->old_baud = 0;
	p_priv->old_cflag = 0;

	p_priv->out_flip = 0;
	p_priv->in_flip = 0;

	/* Reset low level data toggle and start reading from endpoints */
	for (i = 0; i < 2; i++) {
		if ((urb = p_priv->in_urbs[i]) == NULL)
			continue;
		urb->dev = serial->dev;

		/* make sure endpoint data toggle is synchronized with the device */
		
		usb_clear_halt(urb->dev, urb->pipe);

		if ((err = usb_submit_urb(urb, GFP_KERNEL)) != 0) {
			dbg("%s - submit urb %d failed (%d)", __FUNCTION__, i, err);
		}
	}

	/* Reset low level data toggle on out endpoints */
	for (i = 0; i < 2; i++) {
		if ((urb = p_priv->out_urbs[i]) == NULL)
			continue;
		urb->dev = serial->dev;
		/* usb_settoggle(urb->dev, usb_pipeendpoint(urb->pipe), usb_pipeout(urb->pipe), 0); */
	}

	keyspan_send_setup(port, 1);
	//mdelay(100);
	//keyspan_set_termios(port, NULL);

	return (0);
}


Lucy McCoy
lucy@keyspan.com

-----Original Message-----
From: Borislav Petkov <bbpetkov@yahoo.de>
Sent: Thursday, November 15, 2007 12:40pm
To: Lucy McCoy <lucy@keyspan.com>
Cc: Greg KH <greg@kroah.com>, linux-kernel@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] keyspan: init termios properly

On Thu, Nov 15, 2007 at 09:49:22AM -0800, Lucy McCoy wrote:
> Greg & Boris,
>
> I've been out of town and just saw this.
>
> This patch will work with the 19HS but WILL BREAK all other Keyspan
> adapters.  It will take me a few days to get to looking at a correct fix but
> that keyspan_send_setup(port, 1) (and the '1' is the important part) must 
> happen once
> when the port is first opened.  The cflag can just be set to whatever the 
> normal default
> is for your serial environment.  What are the defaults?

Hi Lucy,

sorry but i don't have the other usb-to-serial models to test them too, i guess
you're in the position to do that :) Well, from what i've unterstood from
reading the code, you can still do the keyspan_send_setup() call since the
switch-case-statement is not handling the 19s series so while this call won't have any
influence on them, it'll still init the others properly, no? Anyway, you can send me your
fix for testing.

Thanks.

-- 
Regards/Gruß,
    Boris.



^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH] keyspan: init termios properly
@ 2007-11-03 10:03 Borislav Petkov
  2007-11-15 17:49 ` Lucy McCoy
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2007-11-03 10:03 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, Andrew Morton, lucy

Hi Greg,

   i get the following backtrace when booting the kernel with "console=ttyUSB0
   console=tty0" while using a Keyspan USA-19HS the usb-to-serial converter
   connected to a desktop machine:

<snip>
[   43.782384] usbcore: registered new interface driver usbserial
[   43.782444] drivers/usb/serial/usb-serial.c: USB Serial Driver core
[   43.782543] drivers/usb/serial/usb-serial.c: USB Serial support registered for Keyspan - (without firmware)
[   43.782652] drivers/usb/serial/usb-serial.c: USB Serial support registered for Keyspan 1 port adapter
[   43.782759] drivers/usb/serial/usb-serial.c: USB Serial support registered for Keyspan 2 port adapter
[   43.782866] drivers/usb/serial/usb-serial.c: USB Serial support registered for Keyspan 4 port adapter
[   43.782980] usbcore: registered new interface driver keyspan
[   43.783040] drivers/usb/serial/keyspan.c: v1.1.5:Keyspan USB to Serial Converter Driver
...
[  124.816533] usb 3-1: new full speed USB device using uhci_hcd and address 2
[  125.135811] usb 3-1: configuration #1 chosen from 2 choices
[  125.140709] keyspan 3-1:1.0: Keyspan 1 port adapter converter detected
[  125.141110] usb 3-1: Keyspan 1 port adapter converter now attached to ttyUSB0
[  125.142446] BUG: unable to handle kernel NULL pointer dereference at virtual address 00000084
[  125.142597] printing eip: c02654ca *pde = 00000000 
[  125.142764] BUG: using smp_processor_id() in preemptible [00000001] code: khubd/142
[  125.142861] caller is die+0x59/0x1eb
[  125.142930]  [<c0105026>] show_trace_log_lvl+0x1a/0x2f
[  125.143054]  [<c0105a19>] show_trace+0x12/0x14
[  125.143173]  [<c0105b34>] dump_stack+0x16/0x18
[  125.143293]  [<c01e3c2f>] debug_smp_processor_id+0xa3/0xb8
[  125.143429]  [<c0105310>] die+0x59/0x1eb
[  125.143546]  [<c0119b07>] do_page_fault+0x42c/0x505
[  125.143680]  [<c02f8622>] error_code+0x72/0x78
[  125.143802]  [<c0262a50>] usb_console_setup+0x182/0x282
[  125.143925]  [<c0122676>] register_console+0xe9/0x21c
[  125.144048]  [<c02628a4>] usb_serial_console_init+0x31/0x33
[  125.144171]  [<c026190f>] usb_serial_probe+0xe3c/0xf55
[  125.144293]  [<c0253718>] usb_probe_interface+0xb6/0xe7
[  125.144424]  [<c0233b8c>] driver_probe_device+0xcb/0x14f
[  125.144548]  [<c0233c18>] __device_attach+0x8/0xa
[  125.144678]  [<c0232fb8>] bus_for_each_drv+0x3b/0x63
[  125.144799]  [<c0233ca9>] device_attach+0x70/0x85
[  125.144919]  [<c0232f2f>] bus_attach_device+0x29/0x77
[  125.145040]  [<c0232044>] device_add+0x302/0x514
[  125.145160]  [<c02521ff>] usb_set_configuration+0x418/0x46d
[  125.145283]  [<c0258b67>] generic_probe+0x53/0x94
[  125.145403]  [<c02534a9>] usb_probe_device+0x38/0x3e
[  125.145523]  [<c0233b8c>] driver_probe_device+0xcb/0x14f
[  125.145656]  [<c0233c18>] __device_attach+0x8/0xa
[  125.145776]  [<c0232fb8>] bus_for_each_drv+0x3b/0x63
[  125.146675]  [<c0233ca9>] device_attach+0x70/0x85
[  125.146795]  [<c0232f2f>] bus_attach_device+0x29/0x77
[  125.146916]  [<c0232044>] device_add+0x302/0x514
[  125.147036]  [<c024dc69>] usb_new_device+0x44/0x82
[  125.147160]  [<c024eeaa>] hub_thread+0x65a/0xa13
[  125.147280]  [<c0132daf>] kthread+0x3b/0x64
[  125.147400]  [<c0104c87>] kernel_thread_helper+0x7/0x10
[  125.147521]  =======================
[  125.147600] Oops: 0000 [#1] PREEMPT SMP 
[  125.147805] Modules linked in: usbhid video output tg3 intel_agp uhci_hcd psmouse agpgart rtc evdev
[  125.148403] 
[  125.148466] Pid: 142, comm: khubd Not tainted (2.6.24-rc1-521-g54866f0 #16)
[  125.148546] BUG: using smp_processor_id() in preemptible [00000001] code: khubd/142
[  125.148646] caller is __show_registers+0xad/0x1d8
[  125.148717]  [<c0105026>] show_trace_log_lvl+0x1a/0x2f
[  125.148839]  [<c0105a19>] show_trace+0x12/0x14
[  125.148958]  [<c0105b34>] dump_stack+0x16/0x18
[  125.149077]  [<c01e3c2f>] debug_smp_processor_id+0xa3/0xb8
[  125.149201]  [<c0102344>] __show_registers+0xad/0x1d8
[  125.149321]  [<c01050f7>] show_registers+0x19/0x1d9
[  125.149440]  [<c01053d6>] die+0x11f/0x1eb
[  125.149557]  [<c0119b07>] do_page_fault+0x42c/0x505
[  125.149688]  [<c02f8622>] error_code+0x72/0x78
[  125.149808]  [<c0262a50>] usb_console_setup+0x182/0x282
[  125.149930]  [<c0122676>] register_console+0xe9/0x21c
[  125.150051]  [<c02628a4>] usb_serial_console_init+0x31/0x33
[  125.150175]  [<c026190f>] usb_serial_probe+0xe3c/0xf55
[  125.150296]  [<c0253718>] usb_probe_interface+0xb6/0xe7
[  125.150416]  [<c0233b8c>] driver_probe_device+0xcb/0x14f
[  125.150538]  [<c0233c18>] __device_attach+0x8/0xa
[  125.150669]  [<c0232fb8>] bus_for_each_drv+0x3b/0x63
[  125.150792]  [<c0233ca9>] device_attach+0x70/0x85
[  125.150913]  [<c0232f2f>] bus_attach_device+0x29/0x77
[  125.151036]  [<c0232044>] device_add+0x302/0x514
[  125.151156]  [<c02521ff>] usb_set_configuration+0x418/0x46d
[  125.151281]  [<c0258b67>] generic_probe+0x53/0x94
[  125.151402]  [<c02534a9>] usb_probe_device+0x38/0x3e
[  125.151523]  [<c0233b8c>] driver_probe_device+0xcb/0x14f
[  125.151658]  [<c0233c18>] __device_attach+0x8/0xa
[  125.151780]  [<c0232fb8>] bus_for_each_drv+0x3b/0x63
[  125.151902]  [<c0233ca9>] device_attach+0x70/0x85
[  125.152023]  [<c0232f2f>] bus_attach_device+0x29/0x77
[  125.152146]  [<c0232044>] device_add+0x302/0x514
[  125.152266]  [<c024dc69>] usb_new_device+0x44/0x82
[  125.152388]  [<c024eeaa>] hub_thread+0x65a/0xa13
[  125.152508]  [<c0132daf>] kthread+0x3b/0x64
[  125.152633]  [<c0104c87>] kernel_thread_helper+0x7/0x10
[  125.152754]  =======================
[  125.152822] EIP: 0060:[<c02654ca>] EFLAGS: 00010246 CPU: 0
[  125.152901] EIP is at keyspan_open+0x11c/0x19d
[  125.152971] EAX: 00000000 EBX: c19e7c00 ECX: c19e7c00 EDX: c19e7c00
[  125.153047] ESI: c1bd3e00 EDI: 00000002 EBP: c1886b54 ESP: c1886b1c
[  125.153123]  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
[  125.153197] Process khubd (pid: 142, ti=c1886000 task=c1843080 task.ti=c1886000)
[  125.153274] Stack: 00000246 22222222 22222222 22222222 22222222 c03eeb40 c19e7c00 0000006e 
[  125.153775]        c1aeab40 c030fcec c02f72b5 c19e7c00 0000006e c03eed18 c1886bb0 c0262a50 
[  125.154264]        c02316d6 c1886b68 c0232e33 c1aeab40 c1886b78 c01dd877 00000cbd c1886b80 
[  125.154762] Call Trace:
[  125.154875]  [<c0105026>] show_trace_log_lvl+0x1a/0x2f
[  125.154995]  [<c01050d6>] show_stack_log_lvl+0x9b/0xa3
[  125.155115]  [<c0105182>] show_registers+0xa4/0x1d9
[  125.155235]  [<c01053d6>] die+0x11f/0x1eb
[  125.155352]  [<c0119b07>] do_page_fault+0x42c/0x505
[  125.155472]  [<c02f8622>] error_code+0x72/0x78
[  125.155654]  [<c0262a50>] usb_console_setup+0x182/0x282
[  125.155774]  [<c0122676>] register_console+0xe9/0x21c
[  125.155893]  [<c02628a4>] usb_serial_console_init+0x31/0x33
[  125.156013]  [<c026190f>] usb_serial_probe+0xe3c/0xf55
[  125.156131]  [<c0253718>] usb_probe_interface+0xb6/0xe7
[  125.156251]  [<c0233b8c>] driver_probe_device+0xcb/0x14f
[  125.156370]  [<c0233c18>] __device_attach+0x8/0xa
[  125.156488]  [<c0232fb8>] bus_for_each_drv+0x3b/0x63
[  125.156616]  [<c0233ca9>] device_attach+0x70/0x85
[  125.156734]  [<c0232f2f>] bus_attach_device+0x29/0x77
[  125.156852]  [<c0232044>] device_add+0x302/0x514
[  125.156969]  [<c02521ff>] usb_set_configuration+0x418/0x46d
[  125.157089]  [<c0258b67>] generic_probe+0x53/0x94
[  125.157207]  [<c02534a9>] usb_probe_device+0x38/0x3e
[  125.157325]  [<c0233b8c>] driver_probe_device+0xcb/0x14f
[  125.157445]  [<c0233c18>] __device_attach+0x8/0xa
[  125.157562]  [<c0232fb8>] bus_for_each_drv+0x3b/0x63
[  125.157689]  [<c0233ca9>] device_attach+0x70/0x85
[  125.157806]  [<c0232f2f>] bus_attach_device+0x29/0x77
[  125.157925]  [<c0232044>] device_add+0x302/0x514
[  125.158041]  [<c024dc69>] usb_new_device+0x44/0x82
[  125.158160]  [<c024eeaa>] hub_thread+0x65a/0xa13
[  125.158277]  [<c0132daf>] kthread+0x3b/0x64
[  125.158393]  [<c0104c87>] kernel_thread_helper+0x7/0x10
[  125.158512]  =======================
[  125.158577] Code: 74 08 8b 4d e8 8b 01 89 42 28 8b 96 98 00 00 00 85 d2 74 08 8b 5d e8 8b 03 89 42 28 8b 55 e0 8b 4d e0 8b 5d e0 8b 42 04 8a 49 48 <8b> 90 84 00 00 00 8b 7a 08 88 4d f3 8b 13 8a 52 0c 88 55 e7 e8 
[   50.035324] EIP: [<c02654ca>] keyspan_open+0x11c/0x19d SS:ESP 0068:c1886b1c

</snip>

and this happens, imho, because in usb_console_setup(), port->tty is set to NULL
prior to calling serial->type->open() which is keyspan_open() in this case. In
keyspan_open(), otoh, some premature terminal config is done for the purposes of
the setup message by deref'ing, among others, port->tty->termios->c_flag, which,
as we saw before :) is NULL and BAM! The patch below is against current git
(v2.6.24-rc1-573-g74521c2).

---
From: Borislav Petkov <bbpetkov@yahoo.de>

Remove redundant code leading to NULL ptr deref and let terminal config settings
take place in the proper initialization path in usb_console_setup().

Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de>
--

 drivers/usb/serial/keyspan.c |   38 ++++++--------------------------------
 1 files changed, 6 insertions(+), 32 deletions(-)

diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
index 6bfdba6..1f7ab15 100644
--- a/drivers/usb/serial/keyspan.c
+++ b/drivers/usb/serial/keyspan.c
@@ -1215,20 +1215,18 @@ static int keyspan_chars_in_buffer (struct usb_serial_port *port)
 
 static int keyspan_open (struct usb_serial_port *port, struct file *filp)
 {
-	struct keyspan_port_private 	*p_priv;
-	struct keyspan_serial_private 	*s_priv;
-	struct usb_serial 		*serial = port->serial;
+	struct keyspan_port_private	*p_priv;
+	struct keyspan_serial_private	*s_priv;
+	struct usb_serial		*serial = port->serial;
 	const struct keyspan_device_details	*d_details;
 	int				i, err;
-	int				baud_rate, device_port;
 	struct urb			*urb;
-	unsigned int			cflag;
 
 	s_priv = usb_get_serial_data(serial);
 	p_priv = usb_get_serial_port_data(port);
 	d_details = p_priv->device_details;
-	
-	dbg("%s - port%d.", __FUNCTION__, port->number); 
+
+	dbg("%s - port%d.", __FUNCTION__, port->number);
 
 	/* Set some sane defaults */
 	p_priv->rts_state = 1;
@@ -1249,7 +1247,7 @@ static int keyspan_open (struct usb_serial_port *port, struct file *filp)
 		urb->dev = serial->dev;
 
 		/* make sure endpoint data toggle is synchronized with the device */
-		
+
 		usb_clear_halt(urb->dev, urb->pipe);
 
 		if ((err = usb_submit_urb(urb, GFP_KERNEL)) != 0) {
@@ -1265,30 +1263,6 @@ static int keyspan_open (struct usb_serial_port *port, struct file *filp)
 		/* usb_settoggle(urb->dev, usb_pipeendpoint(urb->pipe), usb_pipeout(urb->pipe), 0); */
 	}
 
-	/* get the terminal config for the setup message now so we don't 
-	 * need to send 2 of them */
-
-	cflag = port->tty->termios->c_cflag;
-	device_port = port->number - port->serial->minor;
-
-	/* Baud rate calculation takes baud rate as an integer
-	   so other rates can be generated if desired. */
-	baud_rate = tty_get_baud_rate(port->tty);
-	/* If no match or invalid, leave as default */		
-	if (baud_rate >= 0
-	    && d_details->calculate_baud_rate(baud_rate, d_details->baudclk,
-				NULL, NULL, NULL, device_port) == KEYSPAN_BAUD_RATE_OK) {
-		p_priv->baud = baud_rate;
-	}
-
-	/* set CTS/RTS handshake etc. */
-	p_priv->cflag = cflag;
-	p_priv->flow_control = (cflag & CRTSCTS)? flow_cts: flow_none;
-
-	keyspan_send_setup(port, 1);
-	//mdelay(100);
-	//keyspan_set_termios(port, NULL);
-
 	return (0);
 }
 
-- 
Regards/Gruß,
    Boris.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2007-12-02 17:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-11-15 21:10 [PATCH] keyspan: init termios properly Lucy McCoy
2007-11-16  6:24 ` Borislav Petkov
2007-11-18 13:11 ` Borislav Petkov
2007-11-26 22:18   ` Andrew Morton
2007-11-30  5:45     ` Borislav Petkov
2007-11-30 17:23       ` Lucy McCoy
2007-12-02  8:03         ` Borislav Petkov
2007-12-02 13:57           ` Alan Cox
2007-12-02 17:40             ` Borislav Petkov
  -- strict thread matches above, loose matches on Subject: below --
2007-11-03 10:03 Borislav Petkov
2007-11-15 17:49 ` Lucy McCoy
2007-11-15 20:09   ` Andrew Morton
2007-11-15 20:28     ` Lucy McCoy
2007-11-15 20:40   ` Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).