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

* Re: [PATCH] keyspan: init termios properly
  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
  1 sibling, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2007-11-16  6:24 UTC (permalink / raw)
  To: Lucy McCoy; +Cc: Greg KH, linux-kernel, Andrew Morton

On Thu, Nov 15, 2007 at 01:10:16PM -0800, Lucy McCoy wrote:

Hi Lucy,
> 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?
will test the patch tomorrow and get back to you with results.
 
> Does cflag need to be set to anything other than CS8?  
i'm not that familiar with serial port settings either, but IIRC the cflag is
used to set the requested byte size of the transferred chars (cf
http://en.wikipedia.org/wiki/Serial_port#Data_Bits). Thus, according to the
above CS8 is used for setting up transfer of any kind of data over the serial connection.

> Is keyspan_open_port() always followed by a keyspan_set_termios()?
err, you mean keyspan_open() ...
..and yes, almost all of the usb to serial converter drivers call their own
*_set_termios-routine so that they could "synchronize" the line discipline user
settings with those of the converter.
-- 
Regards/Gruß,
    Boris.

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

* Re: [PATCH] keyspan: init termios properly
  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
  1 sibling, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2007-11-18 13:11 UTC (permalink / raw)
  To: Lucy McCoy; +Cc: Greg KH, linux-kernel, Andrew Morton

On Thu, Nov 15, 2007 at 01:10:16PM -0800, Lucy McCoy wrote:
 
> 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);
> }
> 

yes, after testing this i can confirm that this one fixes the NULL ptr
problem here so you might want to submit a proper patch to Greg.

-- 
Regards/Gruß,
    Boris.

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

* Re: [PATCH] keyspan: init termios properly
  2007-11-18 13:11 ` Borislav Petkov
@ 2007-11-26 22:18   ` Andrew Morton
  2007-11-30  5:45     ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2007-11-26 22:18 UTC (permalink / raw)
  To: bbpetkov; +Cc: lucy, greg, linux-kernel

On Sun, 18 Nov 2007 14:11:30 +0100
Borislav Petkov <bbpetkov@yahoo.de> wrote:

> On Thu, Nov 15, 2007 at 01:10:16PM -0800, Lucy McCoy wrote:
>  
> > 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);
> > }
> > 
> 
> yes, after testing this i can confirm that this one fixes the NULL ptr
> problem here so you might want to submit a proper patch to Greg.

I'll merge revert-keyspan-init-termios-properly.patch soon, but afaik we
are still awaiting the real fix for this problem?  

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

* Re: [PATCH] keyspan: init termios properly
  2007-11-26 22:18   ` Andrew Morton
@ 2007-11-30  5:45     ` Borislav Petkov
  2007-11-30 17:23       ` Lucy McCoy
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2007-11-30  5:45 UTC (permalink / raw)
  To: Andrew Morton; +Cc: lucy, greg, linux-kernel

On Mon, Nov 26, 2007 at 02:18:52PM -0800, Andrew Morton wrote:
> On Sun, 18 Nov 2007 14:11:30 +0100
> Borislav Petkov <bbpetkov@yahoo.de> wrote:
> 
> > On Thu, Nov 15, 2007 at 01:10:16PM -0800, Lucy McCoy wrote:

...

> > yes, after testing this i can confirm that this one fixes the NULL ptr
> > problem here so you might want to submit a proper patch to Greg.
> 
> I'll merge revert-keyspan-init-termios-properly.patch soon, but afaik we
> are still awaiting the real fix for this problem?

Hi Andrew,
sorry for the late reply - i was away from the country and couldn't read mail.
Yes, we are still awaiting the real fix afaik but the code fragment above
removes the NULL ptr deref so we should at least merge that. Will prepare a
patch for this later today...

-- 
Regards/Gruß,
    Boris.

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

* Re: [PATCH] keyspan: init termios properly
  2007-11-30  5:45     ` Borislav Petkov
@ 2007-11-30 17:23       ` Lucy McCoy
  2007-12-02  8:03         ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: Lucy McCoy @ 2007-11-30 17:23 UTC (permalink / raw)
  To: bbpetkov; +Cc: Andrew Morton, greg, linux-kernel

Hi All,

I've been too busy to get to this but i'd rather not use the code 
fragment i sent Boris to try.  It would be better to go ahead with the 
tty setup if the pointer is not NULL, otherwise use the defaults and not 
reference the NULL pointer. I'll see if I can get to this today.

Lucy


Borislav Petkov wrote:
> On Mon, Nov 26, 2007 at 02:18:52PM -0800, Andrew Morton wrote:
>   
>> On Sun, 18 Nov 2007 14:11:30 +0100
>> Borislav Petkov <bbpetkov@yahoo.de> wrote:
>>
>>     
>>> On Thu, Nov 15, 2007 at 01:10:16PM -0800, Lucy McCoy wrote:
>>>       
>
> ...
>
>   
>>> yes, after testing this i can confirm that this one fixes the NULL ptr
>>> problem here so you might want to submit a proper patch to Greg.
>>>       
>> I'll merge revert-keyspan-init-termios-properly.patch soon, but afaik we
>> are still awaiting the real fix for this problem?
>>     
>
> Hi Andrew,
> sorry for the late reply - i was away from the country and couldn't read mail.
> Yes, we are still awaiting the real fix afaik but the code fragment above
> removes the NULL ptr deref so we should at least merge that. Will prepare a
> patch for this later today...
>
>   

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

* Re: [PATCH] keyspan: init termios properly
  2007-11-30 17:23       ` Lucy McCoy
@ 2007-12-02  8:03         ` Borislav Petkov
  2007-12-02 13:57           ` Alan Cox
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2007-12-02  8:03 UTC (permalink / raw)
  To: Lucy McCoy; +Cc: Andrew Morton, greg, linux-kernel

On Fri, Nov 30, 2007 at 09:23:43AM -0800, Lucy McCoy wrote:
> Hi All,
>
> I've been too busy to get to this but i'd rather not use the code fragment 
> i sent Boris to try.  It would be better to go ahead with the tty setup if 
> the pointer is not NULL, otherwise use the defaults and not reference the 
> NULL pointer. I'll see if I can get to this today.
you might also want to take a look at usa90_indat_callback() - there is
port->tty accessed again and this time it kills the machine completely when
something is sent over the serial line from the other end (through minicom, for
example). Oops is at

http://www.screenshots.cc/view_image/ab9a1837/cimg00772.jpg
-- 
Regards/Gruß,
    Boris.

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

* Re: [PATCH] keyspan: init termios properly
  2007-12-02  8:03         ` Borislav Petkov
@ 2007-12-02 13:57           ` Alan Cox
  2007-12-02 17:40             ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Cox @ 2007-12-02 13:57 UTC (permalink / raw)
  To: bbpetkov; +Cc: yme, Lucy McCoy, Andrew Morton, greg, linux-kernel

On Sun, 2 Dec 2007 09:03:40 +0100
Borislav Petkov <yme@gollum.tnic> wrote:

> On Fri, Nov 30, 2007 at 09:23:43AM -0800, Lucy McCoy wrote:
> > Hi All,
> >
> > I've been too busy to get to this but i'd rather not use the code fragment 
> > i sent Boris to try.  It would be better to go ahead with the tty setup if 
> > the pointer is not NULL, otherwise use the defaults and not reference the 
> > NULL pointer. I'll see if I can get to this today.
> you might also want to take a look at usa90_indat_callback() - there is
> port->tty accessed again and this time it kills the machine completely when
> something is sent over the serial line from the other end (through minicom, for
> example). Oops is at
> 
> http://www.screenshots.cc/view_image/ab9a1837/cimg00772.jpg

Change it to if (tty && urb->actual_length) in that function and it'll
probably cure it

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

* Re: [PATCH] keyspan: init termios properly
  2007-12-02 13:57           ` Alan Cox
@ 2007-12-02 17:40             ` Borislav Petkov
  0 siblings, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2007-12-02 17:40 UTC (permalink / raw)
  To: Alan Cox; +Cc: Lucy McCoy, Andrew Morton, greg, linux-kernel

On Sun, Dec 02, 2007 at 01:57:34PM +0000, Alan Cox wrote:
> On Sun, 2 Dec 2007 09:03:40 +0100
> Borislav Petkov <yme@gollum.tnic> wrote:
> 
> > On Fri, Nov 30, 2007 at 09:23:43AM -0800, Lucy McCoy wrote:
> > > Hi All,
> > >
> > > I've been too busy to get to this but i'd rather not use the code fragment 
> > > i sent Boris to try.  It would be better to go ahead with the tty setup if 
> > > the pointer is not NULL, otherwise use the defaults and not reference the 
> > > NULL pointer. I'll see if I can get to this today.
> > you might also want to take a look at usa90_indat_callback() - there is
> > port->tty accessed again and this time it kills the machine completely when
> > something is sent over the serial line from the other end (through minicom, for
> > example). Oops is at
> > 
> > http://www.screenshots.cc/view_image/ab9a1837/cimg00772.jpg
> 
> Change it to if (tty && urb->actual_length) in that function and it'll
> probably cure it
yep, this does the trick, however, this is just a brown paper bag over the issue, afaict.
Most of the usb serial drivers do something in the likes of

	struct usb_serial_port *port = (struct usb_serial_port *) urb->context;
	...
	tty = port->tty;
	...
	tty_insert_flip_{char,string,..}(tty,..)

in their _indat_callback()s.

In the keyspan case the port->tty is being accessed already in the ->open() function
from usb_console_setup(), before setting the termios settings and later again in the
_indat callback. I'm clearly missing the big picture here so what is the proper way
to initialize termios settings correctly in a driver?

-- 
Regards/Gruß,
    Boris.

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

* Re: [PATCH] keyspan: init termios properly
  2007-11-15 17:49 ` Lucy McCoy
  2007-11-15 20:09   ` Andrew Morton
@ 2007-11-15 20:40   ` Borislav Petkov
  1 sibling, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2007-11-15 20:40 UTC (permalink / raw)
  To: Lucy McCoy; +Cc: Greg KH, linux-kernel, Andrew Morton

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

* Re: [PATCH] keyspan: init termios properly
  2007-11-15 20:09   ` Andrew Morton
@ 2007-11-15 20:28     ` Lucy McCoy
  0 siblings, 0 replies; 14+ messages in thread
From: Lucy McCoy @ 2007-11-15 20:28 UTC (permalink / raw)
  To: Andrew Morton; +Cc: bbpetkov, Greg KH, linux-kernel

Yes, it will break things. I'll try to get to it soon...

Andrew Morton wrote:
> On Thu, 15 Nov 2007 09:49:22 -0800 Lucy McCoy <lucy@keyspan.com> 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?
>>
>>     
>
> drat, this patch just went into mainline.
>
> If you're sure that it'll break things, I'll revert it while you cook up
> something better, OK?
>
>
>   

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

* Re: [PATCH] keyspan: init termios properly
  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
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2007-11-15 20:09 UTC (permalink / raw)
  To: Lucy McCoy; +Cc: bbpetkov, Greg KH, linux-kernel

On Thu, 15 Nov 2007 09:49:22 -0800 Lucy McCoy <lucy@keyspan.com> 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?
> 

drat, this patch just went into mainline.

If you're sure that it'll break things, I'll revert it while you cook up
something better, OK?


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

* Re: [PATCH] keyspan: init termios properly
  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:40   ` Borislav Petkov
  0 siblings, 2 replies; 14+ messages in thread
From: Lucy McCoy @ 2007-11-15 17:49 UTC (permalink / raw)
  To: bbpetkov, Greg KH; +Cc: linux-kernel, Andrew Morton

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?

Lucy

Borislav Petkov wrote:
> 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);
>  }
>  
>   

^ 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).