Bug 13675 - sierra_close sometimes crashes upon device removal
Summary: sierra_close sometimes crashes upon device removal
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: USB (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Greg Kroah-Hartman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-29 16:15 UTC by Peter Naulls
Modified: 2009-07-16 18:35 UTC (History)
1 user (show)

See Also:
Kernel Version: 2.6.31-rc1-git5
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments

Description Peter Naulls 2009-06-29 16:15:01 UTC
First time submitting a kernel.org bug.  Apologies for anything I've missed.

I think was this introduced in 2.6.29.  I do not see this in 2.6.28.x kernels.

I am seeing this on both AMD Geode and Via C7 platforms.

In this system there are other processes which may be attempting to read related /sys files or the device node itself.  In this case, I am removing an AT&T Mercury USB Modem:

usb 1-3: USB disconnect, address 3
sierra ttyUSB4: resubmit read urb failed.(-19)
sierra ttyUSB4: resubmit read urb failed.(-19)
sierra ttyUSB0: Sierra USB modem converter now disconnected from ttyUSB0
sierra 1-3:1.0: device disconnected
sierra ttyUSB1: Sierra USB modem converter now disconnected from ttyUSB1
sierra 1-3:1.1: device disconnected
sierra ttyUSB2: Sierra USB modem converter now disconnected from ttyUSB2
sierra 1-3:1.2: device disconnected
sierra ttyUSB3: Sierra USB modem converter now disconnected from ttyUSB3
sierra 1-3:1.3: device disconnected
sierra ttyUSB4: Sierra USB modem converter now disconnected from ttyUSB4
sierra 1-3:1.4: device disconnected
sierra ttyUSB5: Sierra USB modem converter now disconnected from ttyUSB5
sierra 1-3:1.5: device disconnected
sierra ttyUSB6: Sierra USB modem converter now disconnected from ttyUSB6
sierra 1-3:1.6: device disconnected
BUG: unable to handle kernel NULL pointer dereference at 00000040
IP: [<c02fe3f4>] sierra_close+0x14/0xc0
*pde = 00000000
Oops: 0002 [#1]
last sysfs file: /sys/devices/pci0000:00/0000:00:0e.0/usb1/1-3/idProduct
Modules linked in: coop_gateways(P) sctp

Pid: 4, comm: events/0 Tainted: P           (2.6.31-rc1-git5 #11)
EIP: 0060:[<c02fe3f4>] EFLAGS: 00010282 CPU: 0
EIP is at sierra_close+0x14/0xc0
EAX: cfa62c00 EBX: ced278c0 ECX: 00000000 EDX: c02fe3e0
ESI: c055b6c0 EDI: 00000000 EBP: cf83bf04 ESP: cf83bef4
 DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
Process events/0 (pid: 4, ti=cf83a000 task=cf8391b0 task.ti=cf83a000)
Stack:
 cfa62c00 cfa62c00 c055b6c0 cfa62ce8 cf83bf18 c02f95cd cfa62c00 ffffffff
<0> ced9c000 cf83bf24 c02faee1 00000296 cf83bf50 c0267606 ced9c098 00000000
<0> 00000000 00000001 00000000 ced9c10a ced9c1e4 ced9c1e0 cf803ce0 cf83bfa0
Call Trace:
 [<c02f95cd>] ? serial_do_down+0x4d/0x60
 [<c02faee1>] ? serial_hangup+0x11/0x30
 [<c0267606>] ? do_tty_hangup+0xc6/0x330
 [<c012dc0c>] ? worker_thread+0x13c/0x240
 [<c012dbaa>] ? worker_thread+0xda/0x240
 [<c0267540>] ? do_tty_hangup+0x0/0x330
 [<c0131510>] ? autoremove_wake_function+0x0/0x50
 [<c012dad0>] ? worker_thread+0x0/0x240
 [<c01312c4>] ? kthread+0x74/0x80
 [<c0131250>] ? kthread+0x0/0x80
 [<c010345b>] ? kernel_thread_helper+0x7/0x1c
Code: 8b 1c 24 8b 74 24 04 89 ec 5d c3 8d b6 00 00 00 00 8d bf 00 00 00 00 55 89 e5 57 56 53 83 ec 04 89 45 f0 8b 18 8b b8 20 02 00 00 <c7> 47 40 00 00 00 00 c7 47 44 00 00 00 00 8b 0b 85 c9 74 77 8d
EIP: [<c02fe3f4>] sierra_close+0x14/0xc0 SS:ESP 0068:cf83bef4
CR2: 0000000000000040
---[ end trace af1048799077b9ee ]---
Comment 1 Andrew Morton 2009-06-29 19:05:39 UTC
(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Mon, 29 Jun 2009 16:15:02 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=13675
> 
>            Summary: sierra_close sometimes crashes upon device removal
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: 2.6.31-rc1-git5
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: USB
>         AssignedTo: greg@kroah.com
>         ReportedBy: peter@mushroomnetworks.com
>         Regression: Yes

2.6.28->2.6.29 regression.  Might be usb-serial, might be serial/tty layer?

> 
> First time submitting a kernel.org bug.  Apologies for anything I've missed.
> 
> I think was this introduced in 2.6.29.  I do not see this in 2.6.28.x
> kernels.
> 
> I am seeing this on both AMD Geode and Via C7 platforms.
> 
> In this system there are other processes which may be attempting to read
> related /sys files or the device node itself.  In this case, I am removing an
> AT&T Mercury USB Modem:
> 
> usb 1-3: USB disconnect, address 3
> sierra ttyUSB4: resubmit read urb failed.(-19)
> sierra ttyUSB4: resubmit read urb failed.(-19)
> sierra ttyUSB0: Sierra USB modem converter now disconnected from ttyUSB0
> sierra 1-3:1.0: device disconnected
> sierra ttyUSB1: Sierra USB modem converter now disconnected from ttyUSB1
> sierra 1-3:1.1: device disconnected
> sierra ttyUSB2: Sierra USB modem converter now disconnected from ttyUSB2
> sierra 1-3:1.2: device disconnected
> sierra ttyUSB3: Sierra USB modem converter now disconnected from ttyUSB3
> sierra 1-3:1.3: device disconnected
> sierra ttyUSB4: Sierra USB modem converter now disconnected from ttyUSB4
> sierra 1-3:1.4: device disconnected
> sierra ttyUSB5: Sierra USB modem converter now disconnected from ttyUSB5
> sierra 1-3:1.5: device disconnected
> sierra ttyUSB6: Sierra USB modem converter now disconnected from ttyUSB6
> sierra 1-3:1.6: device disconnected
> BUG: unable to handle kernel NULL pointer dereference at 00000040
> IP: [<c02fe3f4>] sierra_close+0x14/0xc0
> *pde = 00000000
> Oops: 0002 [#1]
> last sysfs file: /sys/devices/pci0000:00/0000:00:0e.0/usb1/1-3/idProduct
> Modules linked in: coop_gateways(P) sctp
> 
> Pid: 4, comm: events/0 Tainted: P           (2.6.31-rc1-git5 #11)
> EIP: 0060:[<c02fe3f4>] EFLAGS: 00010282 CPU: 0
> EIP is at sierra_close+0x14/0xc0
> EAX: cfa62c00 EBX: ced278c0 ECX: 00000000 EDX: c02fe3e0
> ESI: c055b6c0 EDI: 00000000 EBP: cf83bf04 ESP: cf83bef4
>  DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
> Process events/0 (pid: 4, ti=cf83a000 task=cf8391b0 task.ti=cf83a000)
> Stack:
>  cfa62c00 cfa62c00 c055b6c0 cfa62ce8 cf83bf18 c02f95cd cfa62c00 ffffffff
> <0> ced9c000 cf83bf24 c02faee1 00000296 cf83bf50 c0267606 ced9c098 00000000
> <0> 00000000 00000001 00000000 ced9c10a ced9c1e4 ced9c1e0 cf803ce0 cf83bfa0
> Call Trace:
>  [<c02f95cd>] ? serial_do_down+0x4d/0x60
>  [<c02faee1>] ? serial_hangup+0x11/0x30
>  [<c0267606>] ? do_tty_hangup+0xc6/0x330
>  [<c012dc0c>] ? worker_thread+0x13c/0x240
>  [<c012dbaa>] ? worker_thread+0xda/0x240
>  [<c0267540>] ? do_tty_hangup+0x0/0x330
>  [<c0131510>] ? autoremove_wake_function+0x0/0x50
>  [<c012dad0>] ? worker_thread+0x0/0x240
>  [<c01312c4>] ? kthread+0x74/0x80
>  [<c0131250>] ? kthread+0x0/0x80
>  [<c010345b>] ? kernel_thread_helper+0x7/0x1c
> Code: 8b 1c 24 8b 74 24 04 89 ec 5d c3 8d b6 00 00 00 00 8d bf 00 00 00 00 55
> 89 e5 57 56 53 83 ec 04 89 45 f0 8b 18 8b b8 20 02 00 00 <c7> 47 40 00 00 00
> 00
> c7 47 44 00 00 00 00 8b 0b 85 c9 74 77 8d
> EIP: [<c02fe3f4>] sierra_close+0x14/0xc0 SS:ESP 0068:cf83bef4
> CR2: 0000000000000040
> ---[ end trace af1048799077b9ee ]---
Comment 2 Alan Stern 2009-06-29 19:31:42 UTC
On Mon, 29 Jun 2009, Andrew Morton wrote:

> > http://bugzilla.kernel.org/show_bug.cgi?id=13675
> > 
> >            Summary: sierra_close sometimes crashes upon device removal

> 2.6.28->2.6.29 regression.  Might be usb-serial, might be serial/tty layer?

This is almost certainly a duplicate of Bugs #13516 and #13524
(different type of USB serial device but the same problem).  See those
reports for more information.

Alan Stern
Comment 3 Peter Naulls 2009-06-29 23:25:08 UTC
On Mon, Jun 29, 2009 at 12:31 PM, Alan Stern<stern@rowland.harvard.edu> wrote:
> On Mon, 29 Jun 2009, Andrew Morton wrote:
>
>> > http://bugzilla.kernel.org/show_bug.cgi?id=13675
>> >
>> >            Summary: sierra_close sometimes crashes upon device removal
>
>> 2.6.28->2.6.29 regression.  Might be usb-serial, might be serial/tty layer?
>
> This is almost certainly a duplicate of Bugs #13516 and #13524
> (different type of USB serial device but the same problem).  See those
> reports for more information.
>
> Alan Stern

Sorry, additional information I should have included, but it wasn't obvious
to me if those were ad-hoc patches, or under serious consideration for
inclusion, and didn't have the patch information to hand when I filed the
bug

I am actually also using the patches mentioned in  #13516 for 2.6.30, which
definitely resolves this immediate issue, and I can plug and unplug
several devices many times, so consider that testing for those patches.

I didn't try them against 2.6.31-rc1 et al, since they don't apply cleanly -
there seems to have been a lot of churn since 2.6.30.

Thanks.

-- 
- Peter Naulls
Comment 4 Alan Stern 2009-06-30 14:27:33 UTC
On Mon, 29 Jun 2009, Peter Naulls wrote:

> Sorry, additional information I should have included, but it wasn't obvious
> to me if those were ad-hoc patches, or under serious consideration for
> inclusion, and didn't have the patch information to hand when I filed the
> bug
> 
> I am actually also using the patches mentioned in  #13516 for 2.6.30, which
> definitely resolves this immediate issue, and I can plug and unplug
> several devices many times, so consider that testing for those patches.
> 
> I didn't try them against 2.6.31-rc1 et al, since they don't apply cleanly -
> there seems to have been a lot of churn since 2.6.30.

The patches are already included in 2.6.31-rc1.  That's why you can't 
apply them.  :-)

Greg, more and more people are running up against this problem, and 
there haven't been any reports of problems caused by the patch.  Is it 
okay now to submit it for the stable trees?

Alan Stern
Comment 5 Greg Kroah-Hartman 2009-06-30 14:42:12 UTC
On Tue, Jun 30, 2009 at 10:27:31AM -0400, Alan Stern wrote:
> On Mon, 29 Jun 2009, Peter Naulls wrote:
> 
> > Sorry, additional information I should have included, but it wasn't obvious
> > to me if those were ad-hoc patches, or under serious consideration for
> > inclusion, and didn't have the patch information to hand when I filed the
> > bug
> > 
> > I am actually also using the patches mentioned in  #13516 for 2.6.30, which
> > definitely resolves this immediate issue, and I can plug and unplug
> > several devices many times, so consider that testing for those patches.
> > 
> > I didn't try them against 2.6.31-rc1 et al, since they don't apply cleanly
> -
> > there seems to have been a lot of churn since 2.6.30.
> 
> The patches are already included in 2.6.31-rc1.  That's why you can't 
> apply them.  :-)
> 
> Greg, more and more people are running up against this problem, and 
> there haven't been any reports of problems caused by the patch.  Is it 
> okay now to submit it for the stable trees?

Yes, please do so.  I want to cut a new .30 release soon, and it would
be good to get this fix in there for the next release.

thanks,

greg k-h
Comment 6 Peter Naulls 2009-06-30 14:46:39 UTC
On Tue, Jun 30, 2009 at 7:27 AM, Alan Stern<stern@rowland.harvard.edu> wrote:
> On Mon, 29 Jun 2009, Peter Naulls wrote:
>
>> Sorry, additional information I should have included, but it wasn't obvious
>> to me if those were ad-hoc patches, or under serious consideration for
>> inclusion, and didn't have the patch information to hand when I filed the
>> bug
>>
>> I am actually also using the patches mentioned in  #13516 for 2.6.30, which
>> definitely resolves this immediate issue, and I can plug and unplug
>> several devices many times, so consider that testing for those patches.
>>
>> I didn't try them against 2.6.31-rc1 et al, since they don't apply cleanly -
>> there seems to have been a lot of churn since 2.6.30.
>
> The patches are already included in 2.6.31-rc1.  That's why you can't
> apply them.  :-)
>

Right, but then there's definitely still a problem - this bug report is against
2.6.31-rc1-git5.



-- 
- Peter Naulls
Comment 7 Alan Stern 2009-06-30 16:02:19 UTC
On Tue, 30 Jun 2009, Peter Naulls wrote:

> > The patches are already included in 2.6.31-rc1.  That's why you can't
> > apply them.  :-)
> >
> 
> Right, but then there's definitely still a problem - this bug report is
> against
> 2.6.31-rc1-git5.

Oho!  It turns out my patch was mangled because other concurrent 
development on the sierra driver was taking place.  This patch should 
put things right.

Alan Stern



Index: usb-2.6/drivers/usb/serial/sierra.c
===================================================================
--- usb-2.6.orig/drivers/usb/serial/sierra.c
+++ usb-2.6/drivers/usb/serial/sierra.c
@@ -814,7 +814,7 @@ static int sierra_startup(struct usb_ser
 	return 0;
 }
 
-static void sierra_disconnect(struct usb_serial *serial)
+static void sierra_release(struct usb_serial *serial)
 {
 	int i;
 	struct usb_serial_port *port;
@@ -830,7 +830,6 @@ static void sierra_disconnect(struct usb
 		if (!portdata)
 			continue;
 		kfree(portdata);
-		usb_set_serial_port_data(port, NULL);
 	}
 }
 
@@ -853,7 +852,7 @@ static struct usb_serial_driver sierra_d
 	.tiocmget          = sierra_tiocmget,
 	.tiocmset          = sierra_tiocmset,
 	.attach            = sierra_startup,
-	.disconnect        = sierra_disconnect,
+	.release           = sierra_release,
 	.read_int_callback = sierra_instat_callback,
 };
Comment 8 Peter Naulls 2009-07-06 16:44:25 UTC
On Tue, Jun 30, 2009 at 9:02 AM, Alan Stern<stern@rowland.harvard.edu> wrote:
> On Tue, 30 Jun 2009, Peter Naulls wrote:
>
>> > The patches are already included in 2.6.31-rc1.  That's why you can't
>> > apply them.  :-)
>> >
>>
>> Right, but then there's definitely still a problem - this bug report is
>> against
>> 2.6.31-rc1-git5.
>
> Oho!  It turns out my patch was mangled because other concurrent
> development on the sierra driver was taking place.  This patch should
> put things right.

[snip]

Ok, using that patch against 2.6.31-rc2, but look at this after reboot:


sierra 1-3:1.0: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB0
sierra 1-3:1.1: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB1
sierra 1-3:1.2: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB2
sierra 1-3:1.3: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB3
sierra 1-3:1.4: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB4
sierra 1-3:1.5: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB5
sierra 1-3:1.6: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB6

[ Unplug, devices vanish ]

sierra 1-3:1.0: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB0
sierra 1-3:1.1: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB1
sierra 1-3:1.2: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB2
sierra 1-3:1.3: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB4
sierra 1-3:1.4: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB5
sierra 1-3:1.5: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB6
sierra 1-3:1.6: Sierra USB modem converter detected
usb 1-3: Sierra USB modem converter now attached to ttyUSB7

ttyUSB3 is now missing, which was the device I was trying to use.

To clarify, the actual correct device to use was ttyUSB4, but on a
previous run I changed my code to look at 3 instead, because upon
reinsertion ttyUSB4 vanished and ttyUSB3 acted like ttyUSB4.


-- 
- Peter Naulls
Comment 9 Alan Stern 2009-07-06 18:01:22 UTC
On Mon, 6 Jul 2009, Peter Naulls wrote:

> Ok, using that patch against 2.6.31-rc2, but look at this after reboot:
> 
> 
> sierra 1-3:1.0: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB0
> sierra 1-3:1.1: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB1
> sierra 1-3:1.2: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB2
> sierra 1-3:1.3: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB3
> sierra 1-3:1.4: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB4
> sierra 1-3:1.5: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB5
> sierra 1-3:1.6: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB6
> 
> [ Unplug, devices vanish ]
> 
> sierra 1-3:1.0: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB0
> sierra 1-3:1.1: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB1
> sierra 1-3:1.2: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB2
> sierra 1-3:1.3: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB4
> sierra 1-3:1.4: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB5
> sierra 1-3:1.5: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB6
> sierra 1-3:1.6: Sierra USB modem converter detected
> usb 1-3: Sierra USB modem converter now attached to ttyUSB7
> 
> ttyUSB3 is now missing, which was the device I was trying to use.

This is correct behavior, isn't it?  Your program is still running and
still holding ttyUSB3 open, so that same minor number can't be used for
a new device node.

Alan Stern
Comment 10 Peter Naulls 2009-07-06 18:08:12 UTC
On Mon, Jul 6, 2009 at 11:01 AM, Alan Stern<stern@rowland.harvard.edu> wrote:
> On Mon, 6 Jul 2009, Peter Naulls wrote:
>

>>
>> ttyUSB3 is now missing, which was the device I was trying to use.
>
> This is correct behavior, isn't it?  Your program is still running and
> still holding ttyUSB3 open, so that same minor number can't be used for
> a new device node.
>

Dubious.  It's true that I am still holding it open, but it will eventually be
let go by pppd et al.  Apart from that, it's a departure from behaviour in
previous kernel versions.   And how do I determine which interface
is the correct control interface the 3rd, 5th, 10th time it's inserted?

I don't pretend to be an expert on device allocation etc, in the kernel,
so please correct me here, but this doesn't seem the best.

-- 
- Peter Naulls
Comment 11 Alan Stern 2009-07-07 01:37:49 UTC
On Mon, 6 Jul 2009, Peter Naulls wrote:

> On Mon, Jul 6, 2009 at 11:01 AM, Alan Stern<stern@rowland.harvard.edu> wrote:
> > On Mon, 6 Jul 2009, Peter Naulls wrote:
> >
> 
> >>
> >> ttyUSB3 is now missing, which was the device I was trying to use.
> >
> > This is correct behavior, isn't it?  Your program is still running and
> > still holding ttyUSB3 open, so that same minor number can't be used for
> > a new device node.
> >
> 
> Dubious.  It's true that I am still holding it open, but it will eventually
> be
> let go by pppd et al.  Apart from that, it's a departure from behaviour in
> previous kernel versions.

Not at all; the kernel has always behaved this way.  Maybe it's not a 
good way, but it hasn't changed.

>   And how do I determine which interface
> is the correct control interface the 3rd, 5th, 10th time it's inserted?

I don't know.  In fact, I don't even know how you can determine the 
correct interface the first time.  After all, if you had another USB 
serial device plugged in earlier then all the ttyUSB numbers would get 
bumped up correspondingly.

The information may be available in sysfs.  It certainly is available 
in the system log, but digging it out isn't easy.

> I don't pretend to be an expert on device allocation etc, in the kernel,
> so please correct me here, but this doesn't seem the best.

What would you prefer?  And what would you do if you had 2 of these 
devices?

Alan Stern
Comment 12 Peter Naulls 2009-07-07 03:11:55 UTC
On Mon, Jul 6, 2009 at 6:37 PM, Alan Stern<stern@rowland.harvard.edu> wrote:
> On Mon, 6 Jul 2009, Peter Naulls wrote:
>
>> On Mon, Jul 6, 2009 at 11:01 AM, Alan Stern<stern@rowland.harvard.edu>
>> wrote:
>> > On Mon, 6 Jul 2009, Peter Naulls wrote:
>> >
>>
>> >>
>> >> ttyUSB3 is now missing, which was the device I was trying to use.
>> >
>> > This is correct behavior, isn't it?  Your program is still running and
>> > still holding ttyUSB3 open, so that same minor number can't be used for
>> > a new device node.
>> >
>>
>> Dubious.  It's true that I am still holding it open, but it will eventually
>> be
>> let go by pppd et al.  Apart from that, it's a departure from behaviour in
>> previous kernel versions.
>
> Not at all; the kernel has always behaved this way.  Maybe it's not a
> good way, but it hasn't changed.

I beg to differ.  The software in question has been working for more than
a year's worth of kernels.  There's something different about
2.6.31-rc2 over 2.6.30 (with your patches).  Maybe it's some subtlety
I'm now triggering.

>>   And how do I determine which interface
>> is the correct control interface the 3rd, 5th, 10th time it's inserted?
>
> I don't know.  In fact, I don't even know how you can determine the
> correct interface the first time.  After all, if you had another USB
> serial device plugged in earlier then all the ttyUSB numbers would get
> bumped up correspondingly.

Certainly. See below.

> The information may be available in sysfs.  It certainly is available
> in the system log, but digging it out isn't easy.
>
>> I don't pretend to be an expert on device allocation etc, in the kernel,
>> so please correct me here, but this doesn't seem the best.
>
> What would you prefer?  And what would you do if you had 2 of these
> devices?

Like you said - and what the code I have already does.  Enumerate
values in /sys and then look for unique USB IDs.  I then know for
certain USB IDs that it's the first device (or in the case of various
wacky Sierra devices, the nth device).  Certainly I assume they're
sequential, but that could be changed. But If it's now the 3rd device
instead of the 4th after removal and reinsertion, then that's
definitely odd.

Real world context: this is a 3G router which accepts multiple cards,
on which I expect the customer to do all kinds of crazy plugging and
unplugging.

Maybe I can work around all this, but what I don't have right now
is a good characterization of the problem.    If you wish, I can
get that, but I feel it would be more productive for someone who
knows the code be able to describe what should be happening.


-- 
- Peter Naulls
Comment 13 Alan Stern 2009-07-07 14:39:06 UTC
On Mon, 6 Jul 2009, Peter Naulls wrote:

> >> Dubious.  It's true that I am still holding it open, but it will
> eventually be
> >> let go by pppd et al.  Apart from that, it's a departure from behaviour in
> >> previous kernel versions.
> >
> > Not at all; the kernel has always behaved this way.  Maybe it's not a
> > good way, but it hasn't changed.
> 
> I beg to differ.  The software in question has been working for more than
> a year's worth of kernels.  There's something different about
> 2.6.31-rc2 over 2.6.30 (with your patches).  Maybe it's some subtlety
> I'm now triggering.

Or maybe it's that the usb-serial code is now behaving as intended, 
whereas in 2.6.30 and below it was buggy.  It's hard to tell; I don't 
understand the earlier design and the whole tty layer has recently been 
revamped.

> >>   And how do I determine which interface
> >> is the correct control interface the 3rd, 5th, 10th time it's inserted?
> >
> > I don't know.  In fact, I don't even know how you can determine the
> > correct interface the first time.  After all, if you had another USB
> > serial device plugged in earlier then all the ttyUSB numbers would get
> > bumped up correspondingly.
> 
> Certainly. See below.
> 
> > The information may be available in sysfs.  It certainly is available
> > in the system log, but digging it out isn't easy.
> >
> >> I don't pretend to be an expert on device allocation etc, in the kernel,
> >> so please correct me here, but this doesn't seem the best.
> >
> > What would you prefer?  And what would you do if you had 2 of these
> > devices?
> 
> Like you said - and what the code I have already does.  Enumerate
> values in /sys and then look for unique USB IDs.  I then know for
> certain USB IDs that it's the first device (or in the case of various
> wacky Sierra devices, the nth device).  Certainly I assume they're
> sequential, but that could be changed. But If it's now the 3rd device
> instead of the 4th after removal and reinsertion, then that's
> definitely odd.

Okay, I did a little checking.  The information you want _is_ available 
in sysfs.  For example, right now I've got a pl2303-based device 
plugged into port 2 of bus 7.  Look what shows up:

$ ls /sys/bus/usb/devices/7-2:1.0/
bAlternateSetting   bInterfaceSubClass  ep_81@    subsystem@
bInterfaceClass     bNumEndpoints       ep_83@    ttyUSB0/
bInterfaceNumber    driver@             modalias  uevent
bInterfaceProtocol  ep_02@              power/    usb_endpoint/

Notice the "ttyUSB0" directory?  In your case you want the fourth 
interface, so you'd look for pathnames of the form 
/sys/bus/usb/devices/*-*:1.3/.

The information is also available in /proc/tty/driver/usbserial, but
this is less useful because /proc/tty/driver is inaccessible unless you
have superuser privileges.

> Real world context: this is a 3G router which accepts multiple cards,
> on which I expect the customer to do all kinds of crazy plugging and
> unplugging.
> 
> Maybe I can work around all this, but what I don't have right now
> is a good characterization of the problem.    If you wish, I can
> get that, but I feel it would be more productive for someone who
> knows the code be able to describe what should be happening.

It does seem unnecessary to keep the minor number tied up merely 
because the device file is open.  Maybe there's some deeper reason for 
this I'm not aware of.  In any case, below is an experimental patch you 
can try out.  It should cause usb-serial to release the minor number as 
soon as the device is unplugged.  (I haven't tried it myself, so you'll 
be the first!)

Alan Stern


Index: usb-2.6/drivers/usb/serial/usb-serial.c
===================================================================
--- usb-2.6.orig/drivers/usb/serial/usb-serial.c
+++ usb-2.6/drivers/usb/serial/usb-serial.c
@@ -137,10 +137,6 @@ static void destroy_serial(struct kref *
 
 	dbg("%s - %s", __func__, serial->type->description);
 
-	/* return the minor range that this device had */
-	if (serial->minor != SERIAL_TTY_NO_MINOR)
-		return_serial(serial);
-
 	serial->type->release(serial);
 
 	for (i = 0; i < serial->num_ports; ++i) {
@@ -1158,6 +1154,10 @@ void usb_serial_disconnect(struct usb_in
 	}
 	serial->type->disconnect(serial);
 
+	/* return the minor range that this device had */
+	if (serial->minor != SERIAL_TTY_NO_MINOR)
+		return_serial(serial);
+
 	/* let the last holder of this object
 	 * cause it to be cleaned up */
 	usb_serial_put(serial);
Comment 14 Alan Stern 2009-07-07 19:29:19 UTC
On Tue, 7 Jul 2009, Oliver Neukum wrote:

> Am Dienstag, 7. Juli 2009 16:39:03 schrieb Alan Stern:
> > It does seem unnecessary to keep the minor number tied up merely
> > because the device file is open.  Maybe there's some deeper reason for
> > this I'm not aware of.  In any case, below is an experimental patch you
> > can try out.  It should cause usb-serial to release the minor number as
> > soon as the device is unplugged.  (I haven't tried it myself, so you'll
> > be the first!)
> 
> This endangers system security. Somebody might have permissions to
> use the old device, defined by a major:minor pair, who must not be let use
> the new device.

I don't understand.  Permissions aren't associated with major:minor 
paris; they are associated with device nodes in the filesystem.

So yes, it's possible that an old device node allowing global access
might not be replaced sufficiently quickly by a new node having more
restrictive permissions.  Then a user could open the old node and
thereby gain access to the new device.  But that's unrelated to this 
patch.

Alan Stern
Comment 15 Anonymous Emailer 2009-07-07 20:42:22 UTC
Reply-To: oliver@neukum.org

Am Dienstag, 7. Juli 2009 21:29:16 schrieb Alan Stern:
> On Tue, 7 Jul 2009, Oliver Neukum wrote:
> > Am Dienstag, 7. Juli 2009 16:39:03 schrieb Alan Stern:
> > > It does seem unnecessary to keep the minor number tied up merely
> > > because the device file is open.  Maybe there's some deeper reason for
> > > this I'm not aware of.  In any case, below is an experimental patch you
> > > can try out.  It should cause usb-serial to release the minor number as
> > > soon as the device is unplugged.  (I haven't tried it myself, so you'll
> > > be the first!)
> >
> > This endangers system security. Somebody might have permissions to
> > use the old device, defined by a major:minor pair, who must not be let
> > use the new device.
>
> I don't understand.  Permissions aren't associated with major:minor
> paris; they are associated with device nodes in the filesystem.

But the node is merely a scheme to link a major:minor pair to a name
and permissions.

> So yes, it's possible that an old device node allowing global access
> might not be replaced sufficiently quickly by a new node having more
> restrictive permissions.  Then a user could open the old node and
> thereby gain access to the new device.  But that's unrelated to this
> patch.

Does it make it likely that the minor number will be reused next time?

	Regards
		Oliver
Comment 16 Alan Stern 2009-07-07 21:02:13 UTC
On Tue, 7 Jul 2009, Oliver Neukum wrote:

> > So yes, it's possible that an old device node allowing global access
> > might not be replaced sufficiently quickly by a new node having more
> > restrictive permissions.  Then a user could open the old node and
> > thereby gain access to the new device.  But that's unrelated to this
> > patch.
> 
> Does it make it likely that the minor number will be reused next time?

If the device file is not in use then the likelihood is unchanged by 
this patch.  If the device file is held open then the patch makes it 
more likely that the minor number will be reused.

Alan Stern
Comment 17 Greg Kroah-Hartman 2009-07-08 16:08:39 UTC
On Mon, Jul 06, 2009 at 11:08:10AM -0700, Peter Naulls wrote:
> On Mon, Jul 6, 2009 at 11:01 AM, Alan Stern<stern@rowland.harvard.edu> wrote:
> > On Mon, 6 Jul 2009, Peter Naulls wrote:
> >
> 
> >>
> >> ttyUSB3 is now missing, which was the device I was trying to use.
> >
> > This is correct behavior, isn't it?  Your program is still running and
> > still holding ttyUSB3 open, so that same minor number can't be used for
> > a new device node.
> >
> 
> Dubious.  It's true that I am still holding it open, but it will eventually
> be
> let go by pppd et al.  Apart from that, it's a departure from behaviour in
> previous kernel versions.

As Alan Stern noted, this is the same behavior that has been present for
many years now.

Perhaps your userspace program is not paying attention to the proper tty
signals now saying that the device was removed (hangup I think.)  Or
perhaps the tty layer is not sending those messages anymore?

> And how do I determine which interface
> is the correct control interface the 3rd, 5th, 10th time it's inserted?

Use the persistent device nodes in /dev/ for usb to serial devices that
modern versions of udev provides you.  To rely on anything else would be
madness :)

thanks,

greg k-h
Comment 18 Alan Stern 2009-07-14 18:46:59 UTC
The patch has been merged as commit 7bae0a070db4bc2761dd9515f450cdfa3f3f248c.  This bug report can now be closed.

Peter, did you ever try the patch in comment #13?  Does it help?
Comment 19 Peter Naulls 2009-07-16 17:40:12 UTC
I'm afraid I do not immediately have access to the hardware to test it, so haven't been able to try it.  Since the initial bug here has been fixed (now using 2.6.30.1 without any patches), I recommend this bug be closed as fixed.  If I'm able to pursue this matter at a later time, I'll open a new bug on the secondary issue if I think it still exists.

Thanks.
Comment 20 Greg Kroah-Hartman 2009-07-16 18:35:51 UTC
Thanks, am closing out.

Note You need to log in before you can comment on or make changes to this bug.