[PATCH 1/2] usb-serial : fix the nousb oops

[PATCH 1/2] usb-serial : fix the nousb oops

[Dave Young]

In case with "nousb" booting, serial drivers will trigger kernel oops.

Here add usb_disabled() check in usb_serial_init and usb_serial_register

Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
---
drivers/usb/serial/usb-serial.c |    6 ++++++
1 file changed, 6 insertions(+)

diff -uprN a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
--- a/drivers/usb/serial/usb-serial.c	2009-02-01 13:11:11.000000000 +0800
+++ b/drivers/usb/serial/usb-serial.c	2009-02-01 13:24:32.000000000 +0800
@@ -1113,6 +1113,9 @@ static int __init usb_serial_init(void)
 	int i;
 	int result;
 
+	if (usb_disabled())
+		return -ENODEV;
+
 	usb_serial_tty_driver = alloc_tty_driver(SERIAL_TTY_MINORS);
 	if (!usb_serial_tty_driver)
 		return -ENOMEM;
@@ -1230,6 +1233,9 @@ int usb_serial_register(struct usb_seria
 	/* must be called with BKL held */
 	int retval;
 
+	if (usb_disabled())
+		return -ENODEV;
+
 	fixup_generic(driver);
 
 	if (!driver->description)

Re: [PATCH 1/2] usb-serial : fix the nousb oops

[Greg KH]

On Sun, Feb 01, 2009 at 06:53:23PM +0800, Dave Young wrote:
> 
> In case with "nousb" booting, serial drivers will trigger kernel oops.

They will?  What is the oops message?

> Here add usb_disabled() check in usb_serial_init and usb_serial_register

This is odd, is it something new?  Why is usb-serial the only driver
that needs such a thing?

confused,

greg k-h

Re: [PATCH 1/2] usb-serial : fix the nousb oops

[Dave Young]

On Sun, Feb 1, 2009 at 11:51 PM, Greg KH <greg@kroah.com> wrote:
> On Sun, Feb 01, 2009 at 06:53:23PM +0800, Dave Young wrote:
>>
>> In case with "nousb" booting, serial drivers will trigger kernel oops.
>
> They will?  What is the oops message?

Yes, they will, at lease I saw usb_serial/aircable/ftdi_sio oops. I
will manage to copy the oops message later.

>
>> Here add usb_disabled() check in usb_serial_init and usb_serial_register
>
> This is odd, is it something new?  Why is usb-serial the only driver
> that needs such a thing?

No, it is not new thing.  usb serial drivers normally call
usb_serial_register first, then usb_register, usb_register will fail
due to "nousb" checking, but usb_serial_register no.

>
> confused,
>
> greg k-h
>



-- 
Regards
dave

Re: [PATCH 1/2] usb-serial : fix the nousb oops

[Dave Young]

On Mon, Feb 2, 2009 at 9:43 AM, Dave Young <hidave.darkstar@gmail.com> wrote:
> On Sun, Feb 1, 2009 at 11:51 PM, Greg KH <greg@kroah.com> wrote:
>> On Sun, Feb 01, 2009 at 06:53:23PM +0800, Dave Young wrote:
>>>
>>> In case with "nousb" booting, serial drivers will trigger kernel oops.
>>
>> They will?  What is the oops message?
>
> Yes, they will, at lease I saw usb_serial/aircable/ftdi_sio oops. I
> will manage to copy the oops message later.

I can not copy the oops message, so I just hand copy something here:

BUG: unable to handle kernel NULL pointer dereference at 00000001
IP: [<c0527636>] kset_find_obj+0x3b/0x5c
*pde = 00000000
Oops: 0000 [#1] SMP

[snip...]

EIP is at kset_find_obj+0x3b/0x5c
EAX:00000001

[snip...]

Call Trace:
driver_register
destruct_tty_driver
usb_serial_bus_register
usb_serial_register
aircable_init
_stext
aircable_init
restore_nocheck_notrace
pci_arch_init

[snip...]

Actually, after the usb_serial_init failed, the usb_serial_bus_type
will be unregistered.

So when  serial dirvers call usb_serial_register (then
usb_serial_bus_register, and then driver_register),  due to the bus
kset already unregistered kernel will oops.

>
>>
>>> Here add usb_disabled() check in usb_serial_init and usb_serial_register
>>
>> This is odd, is it something new?  Why is usb-serial the only driver
>> that needs such a thing?
>
> No, it is not new thing.  usb serial drivers normally call
> usb_serial_register first, then usb_register, usb_register will fail
> due to "nousb" checking, but usb_serial_register no.
>
>>
>> confused,
>>
>> greg k-h
>>
>
>
>
> --
> Regards
> dave
>



-- 
Regards
dave

Re: [PATCH 1/2] usb-serial : fix the nousb oops

[Greg KH]

On Mon, Feb 02, 2009 at 09:43:04AM +0800, Dave Young wrote:
> On Sun, Feb 1, 2009 at 11:51 PM, Greg KH <greg@kroah.com> wrote:
> > On Sun, Feb 01, 2009 at 06:53:23PM +0800, Dave Young wrote:
> >>
> >> In case with "nousb" booting, serial drivers will trigger kernel oops.
> >
> > They will?  What is the oops message?
> 
> Yes, they will, at lease I saw usb_serial/aircable/ftdi_sio oops. I
> will manage to copy the oops message later.
> 
> >
> >> Here add usb_disabled() check in usb_serial_init and usb_serial_register
> >
> > This is odd, is it something new?  Why is usb-serial the only driver
> > that needs such a thing?
> 
> No, it is not new thing.  usb serial drivers normally call
> usb_serial_register first, then usb_register, usb_register will fail
> due to "nousb" checking, but usb_serial_register no.

But as usb_register will fail, shouldn't they properly back out and no
oopses happen?  This sounds like a simple fix to the drivers that are
failing here, not changing the usb-serial core.

And this should only be an issue if you build your drivers into the
kernel, right?

thanks,

greg k-h

Re: [PATCH 1/2] usb-serial : fix the nousb oops

[Dave Young]

remove wgreathouse@smva.com in cc-list because this mail will be
rejected by the smva.com email server.

On Wed, Feb 4, 2009 at 7:58 AM, Greg KH <greg@kroah.com> wrote:
> On Mon, Feb 02, 2009 at 09:43:04AM +0800, Dave Young wrote:
>> On Sun, Feb 1, 2009 at 11:51 PM, Greg KH <greg@kroah.com> wrote:
>> > On Sun, Feb 01, 2009 at 06:53:23PM +0800, Dave Young wrote:
>> >>
>> >> In case with "nousb" booting, serial drivers will trigger kernel oops.
>> >
>> > They will?  What is the oops message?
>>
>> Yes, they will, at lease I saw usb_serial/aircable/ftdi_sio oops. I
>> will manage to copy the oops message later.
>>
>> >
>> >> Here add usb_disabled() check in usb_serial_init and usb_serial_register
>> >
>> > This is odd, is it something new?  Why is usb-serial the only driver
>> > that needs such a thing?
>>
>> No, it is not new thing.  usb serial drivers normally call
>> usb_serial_register first, then usb_register, usb_register will fail
>> due to "nousb" checking, but usb_serial_register no.
>
> But as usb_register will fail, shouldn't they properly back out and no
> oopses happen?  This sounds like a simple fix to the drivers that are
> failing here, not changing the usb-serial core.

Hi greg

I think all usb serial drivers which call usb_serial_register will
oops, so IMHO usb core should be fixed.

I think maybe you missed my another reply, copy the content here again.

---
hand copy part of the oops message:

BUG: unable to handle kernel NULL pointer dereference at 00000001
IP: [<c0527636>] kset_find_obj+0x3b/0x5c
*pde = 00000000
Oops: 0000 [#1] SMP

[snip...]

EIP is at kset_find_obj+0x3b/0x5c
EAX:00000001

[snip...]

Call Trace:
driver_register
destruct_tty_driver
usb_serial_bus_register
usb_serial_register
aircable_init
_stext
aircable_init
restore_nocheck_notrace
pci_arch_init

[snip...]

Actually, after the usb_serial_init failed, the usb_serial_bus_type
will be unregistered.

So when  serial dirvers call usb_serial_register (then
usb_serial_bus_register, and then driver_register),  due to the bus
kset already unregistered kernel will oops.
---

>
> And this should only be an issue if you build your drivers into the
> kernel, right?

Yes.

>
> thanks,
>
> greg k-h
>



-- 
Regards
dave

Re: [PATCH 1/2] usb-serial : fix the nousb oops

[Oliver Neukum]

Am Wednesday 04 February 2009 00:58:14 schrieb Greg KH:
> On Mon, Feb 02, 2009 at 09:43:04AM +0800, Dave Young wrote:
 
> > No, it is not new thing.  usb serial drivers normally call
> > usb_serial_register first, then usb_register, usb_register will fail
> > due to "nousb" checking, but usb_serial_register no.
> 
> But as usb_register will fail, shouldn't they properly back out and no
> oopses happen?  This sounds like a simple fix to the drivers that are
> failing here, not changing the usb-serial core.

usb_register() can also fail for other reasons. There's no way you
can save drivers from needing to cleanly fail in that case.

> And this should only be an issue if you build your drivers into the
> kernel, right?

No, you can load drivers by other methods than udev.

	Regards
		Oliver

Re: [PATCH 1/2] usb-serial : fix the nousb oops

[Greg KH]

On Wed, Feb 04, 2009 at 09:10:20AM +0800, Dave Young wrote:
> remove wgreathouse@smva.com in cc-list because this mail will be
> rejected by the smva.com email server.
> 
> On Wed, Feb 4, 2009 at 7:58 AM, Greg KH <greg@kroah.com> wrote:
> > On Mon, Feb 02, 2009 at 09:43:04AM +0800, Dave Young wrote:
> >> On Sun, Feb 1, 2009 at 11:51 PM, Greg KH <greg@kroah.com> wrote:
> >> > On Sun, Feb 01, 2009 at 06:53:23PM +0800, Dave Young wrote:
> >> >>
> >> >> In case with "nousb" booting, serial drivers will trigger kernel oops.
> >> >
> >> > They will?  What is the oops message?
> >>
> >> Yes, they will, at lease I saw usb_serial/aircable/ftdi_sio oops. I
> >> will manage to copy the oops message later.
> >>
> >> >
> >> >> Here add usb_disabled() check in usb_serial_init and usb_serial_register
> >> >
> >> > This is odd, is it something new?  Why is usb-serial the only driver
> >> > that needs such a thing?
> >>
> >> No, it is not new thing.  usb serial drivers normally call
> >> usb_serial_register first, then usb_register, usb_register will fail
> >> due to "nousb" checking, but usb_serial_register no.
> >
> > But as usb_register will fail, shouldn't they properly back out and no
> > oopses happen?  This sounds like a simple fix to the drivers that are
> > failing here, not changing the usb-serial core.
> 
> Hi greg
> 
> I think all usb serial drivers which call usb_serial_register will
> oops, so IMHO usb core should be fixed.
> 
> I think maybe you missed my another reply, copy the content here again.

No, I got it.

I'm more interested in why usb-serial is oopsing, as it should't be.

> ---
> hand copy part of the oops message:

Can you take a picture of the oops?

thanks,

greg k-h

Re: [PATCH 1/2] usb-serial : fix the nousb oops

[Dave Young]

On Thu, Feb 5, 2009 at 7:59 AM, Greg KH <greg@kroah.com> wrote:
> On Wed, Feb 04, 2009 at 09:10:20AM +0800, Dave Young wrote:
>> remove wgreathouse@smva.com in cc-list because this mail will be
>> rejected by the smva.com email server.
>>
>> On Wed, Feb 4, 2009 at 7:58 AM, Greg KH <greg@kroah.com> wrote:
>> > On Mon, Feb 02, 2009 at 09:43:04AM +0800, Dave Young wrote:
>> >> On Sun, Feb 1, 2009 at 11:51 PM, Greg KH <greg@kroah.com> wrote:
>> >> > On Sun, Feb 01, 2009 at 06:53:23PM +0800, Dave Young wrote:
>> >> >>
>> >> >> In case with "nousb" booting, serial drivers will trigger kernel oops.
>> >> >
>> >> > They will?  What is the oops message?
>> >>
>> >> Yes, they will, at lease I saw usb_serial/aircable/ftdi_sio oops. I
>> >> will manage to copy the oops message later.
>> >>
>> >> >
>> >> >> Here add usb_disabled() check in usb_serial_init and usb_serial_register
>> >> >
>> >> > This is odd, is it something new?  Why is usb-serial the only driver
>> >> > that needs such a thing?
>> >>
>> >> No, it is not new thing.  usb serial drivers normally call
>> >> usb_serial_register first, then usb_register, usb_register will fail
>> >> due to "nousb" checking, but usb_serial_register no.
>> >
>> > But as usb_register will fail, shouldn't they properly back out and no
>> > oopses happen?  This sounds like a simple fix to the drivers that are
>> > failing here, not changing the usb-serial core.
>>
>> Hi greg
>>
>> I think all usb serial drivers which call usb_serial_register will
>> oops, so IMHO usb core should be fixed.
>>
>> I think maybe you missed my another reply, copy the content here again.
>
> No, I got it.
>
> I'm more interested in why usb-serial is oopsing, as it should't be.
>
>> ---
>> hand copy part of the oops message:
>
> Can you take a picture of the oops?

Hi, I have no camera at hand, I will take the picture tomorrow.

>
> thanks,
>
> greg k-h
>



-- 
Regards
dave

Re: [PATCH 1/2] usb-serial : fix the nousb oops

[Dave Young]

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]

On Thu, Feb 5, 2009 at 7:59 AM, Greg KH <greg@kroah.com> wrote:
> On Wed, Feb 04, 2009 at 09:10:20AM +0800, Dave Young wrote:
>> remove wgreathouse@smva.com in cc-list because this mail will be
>> rejected by the smva.com email server.
>>
>> On Wed, Feb 4, 2009 at 7:58 AM, Greg KH <greg@kroah.com> wrote:
>> > On Mon, Feb 02, 2009 at 09:43:04AM +0800, Dave Young wrote:
>> >> On Sun, Feb 1, 2009 at 11:51 PM, Greg KH <greg@kroah.com> wrote:
>> >> > On Sun, Feb 01, 2009 at 06:53:23PM +0800, Dave Young wrote:
>> >> >>
>> >> >> In case with "nousb" booting, serial drivers will trigger kernel oops.
>> >> >
>> >> > They will?  What is the oops message?
>> >>
>> >> Yes, they will, at lease I saw usb_serial/aircable/ftdi_sio oops. I
>> >> will manage to copy the oops message later.
>> >>
>> >> >
>> >> >> Here add usb_disabled() check in usb_serial_init and usb_serial_register
>> >> >
>> >> > This is odd, is it something new?  Why is usb-serial the only driver
>> >> > that needs such a thing?
>> >>
>> >> No, it is not new thing.  usb serial drivers normally call
>> >> usb_serial_register first, then usb_register, usb_register will fail
>> >> due to "nousb" checking, but usb_serial_register no.
>> >
>> > But as usb_register will fail, shouldn't they properly back out and no
>> > oopses happen?  This sounds like a simple fix to the drivers that are
>> > failing here, not changing the usb-serial core.
>>
>> Hi greg
>>
>> I think all usb serial drivers which call usb_serial_register will
>> oops, so IMHO usb core should be fixed.
>>
>> I think maybe you missed my another reply, copy the content here again.
>
> No, I got it.
>
> I'm more interested in why usb-serial is oopsing, as it should't be.
>
>> ---
>> hand copy part of the oops message:
>
> Can you take a picture of the oops?

Hi, greg:

Yestoday I found that with two diffrent config there's diffrent EIP,
please see the attachment (configs), you can use the configs to create
the panic.

Due to the image files size I only send them to you in another email.

>
> thanks,
>
> greg k-h
>



-- 
Regards
dave

[-- Attachment #2: panic1.config.gz --]
[-- Type: application/x-gzip, Size: 14838 bytes --]

[-- Attachment #3: panic2.config.gz --]
[-- Type: application/x-gzip, Size: 20667 bytes --]