Bug 11585

Created attachment 17849 [details]
dmesg output from 2.6.26.1 drivers

Generated with "debug=1" option enabled for usb-serial.ko and mos7840.ko

Created attachment 17850 [details]
failing dmesg output from 2.6.26.3 drivers 

Generated with "debug=1" option enabled for usb-serial.ko and mos7840.ko

Created attachment 17851 [details]
strace of application (working) using 2.6.26.1 drivers

Created using minicom to open the usb-serial port, send a few characters to the tty which are echoed back by the attached serial device, and then close the tty.

Created attachment 17853 [details]
strace of application (failing) using 2.6.26.3 drivers

Created using minicom to open the usb-serial port, send a few characters to the tty which are echoed back by the attached serial device, and then close the tty.

Note the incorrect ioctl results compared to the 2.6.26.1 strace.  Minicom sets the port to 4800 8N1 with no hardware flow control but the driver sends wholly different results to a TIOCMGET.

Reply-To: akpm@linux-foundation.org


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Thu, 18 Sep 2008 03:02:40 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=11585
> 
>            Summary: mos7840 usb-serial driver sends wrong chars, returns
>                     wrong ioctl results, and OOPSes when device unplugged.

Is that all?  Gee you're picky!

>            Product: Drivers
>            Version: 2.5
>      KernelVersion: 2.6.26.3
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: USB
>         AssignedTo: greg@kroah.com
>         ReportedBy: nick@leverton.org
> 
> 
> Latest working kernel version: 2.6.26.2 generic
> Earliest failing kernel version: 2.6.26.3 generic
> Distribution: Debian
> Hardware Environment: Dell Latitude D830,
>    mos7840 USB ExpressCard, labelled "CE N35 4210 11 03401"
> Software Environment: i686 kernel; libusb 0.1.12
> 
> The mos7840 usb-serial driver worked for me in 2.6.26.1.
> 
> Since the "usb-serial: don't release unregistered minors" change to
> usb-serial.c which was committed to 2.6.26.3, the mos7840 driver fails in
> several ways.
> 
> * Whatever character I send to the serial port from my application, it always
> sends the uppercase letter O to the tty.
> 
> * TIOCMGET returns incorrect values
> 
> * When I unplug the device, the kernel OOPSes with a NULL pointer
> dereference.
> 
> Steps to reproduce:
> 
> 1. Plug in mos7840 USB Express usb-serial device - OK
> 
> 2. Type characters and watch the output from the serial port - wrong
> character
> emerges.  Strace your program and see the incorrect ioctl results.
> 
> 3. Unplug the mos7840 device - kernel OOPS
> 
> If I revert usb-serial.c to the version from 2.6.26.1, backing out the
> "unregistered minors" change, but leaving all other kernel source and
> config as at 2.6.26.3, then the mos7840 driver again fully works.
> 
> So I guess that the usb-serial.c change is having unexpected effects
> in conjunction with the mos7840 driver.  I note one of the changes in
> this patch was to remove a check for NULL serial device pointer - looks quite
> relevant in the context of the OOPS.
> 
> [  129.469805] usb 3-1: new high speed USB device using ehci_hcd and address
> 3
> [  129.610533] usb 3-1: configuration #1 chosen from 1 choice
> [  129.625110] usb 3-1: New USB device found, idVendor=9710, idProduct=7840
> [  129.639548] usb 3-1: New USB device strings: Mfr=0, Product=0,
> SerialNumber=0
> [  129.733070] usbserial: USB Serial support registered for Moschip 7840/7820
> USB Serial Driver
> [  129.740656] mos7840: Moschip 7840/7820 USB Serial Driver 1.3.1
> [  129.752701] mos7840 3-1:1.0: Moschip 7840/7820 USB Serial Driver converter
> detected
> [  129.766513] usb 3-1: Moschip 7840/7820 USB Serial Driver converter now
> attached to ttyUSB2
> [  129.777384] usb 3-1: Moschip 7840/7820 USB Serial Driver converter now
> attached to ttyUSB3
> [  129.785435] usb 3-1: Moschip 7840/7820 USB Serial Driver converter now
> attached to ttyUSB4
> [  129.799854] usb 3-1: Moschip 7840/7820 USB Serial Driver converter now
> attached to ttyUSB5
> [  129.810962] usbcore: registered new interface driver mos7840
> [  138.813698] usb 3-1: USB disconnect, address 3
> [  138.826849] BUG: unable to handle kernel NULL pointer dereference at
> 00000078
> [  138.831587] IP: [<c02b9012>] _spin_lock_irqsave+0x1d/0x2f
> [  138.831587] *pde = 00000000
> [  138.831587] Oops: 0002 [#1] SMP
> [  138.831587] Modules linked in: mos7840 rfcomm l2cap ppdev parport_pc lp
> parport nfsd auth_rpcgss exportfs nfs lockd nfs_acl sunrpc ip6table_filter
> ip6_tables iptable_raw xt_comment xt_policy ipt_ULOG ipt_TTL ipt_ttl
> ipt_REJECT
> ipt_RE
> DIRECT ipt_recent ipt_NETMAP ipt_MASQUERADE ipt_LOG ipt_ECN ipt_ecn
> ipt_CLUSTERIP ipt_ah ipt_addrtype nf_nat_tftp nf_nat_snmp_basic nf_nat_sip
> nf_nat_pptp nf_nat_proto_gre nf_nat_irc nf_nat_h323 nf_nat_ftp nf_nat_amanda
> ts_kmp nf_conntrack_amanda nf_conntrack_tftp nf_conntrack_sip
> nf_conntrack_proto_sctp nf_conntrack_pptp nf_conntrack_proto_gre
> nf_conntrack_netlink nf_conntrack_netbios_ns nf_conntrack_irc
> nf_conntrack_h323
> nf_conntrack_ftp xt_tcpmss xt_pkttype xt_physdev xt_owner xt_NFQUEUE xt_NFLOG
> xt_multiport xt_MARK xt_mark xt_mac xt_limit xt_length xt_iprange xt_helper
> xt_hashlimit xt_DSCP xt_dscp xt_dccp xt_conntrack xt_CONNMARK xt_connmark
> xt_CLASSIFY xt_tcpudp xt_state iptable_nat nf_nat nf_conntrack_ipv4
> nf_conntrack iptable_mangle nfnetlink iptable_filter ip_tables x_tables
> bridge
> tun ipv6 nls_utf8 isofs nls_base zlib_inflate fuse ext3 jbd mbcache coretemp
> vboxdrv loop i8k cpufreq_powersave cpufreq_ondemand cpufreq_conservative
> acpi_cpufreq freq_table joydev pcmcia firmware_class psmouse serio_raw
> yenta_socket rsrc_nonstatic pcmcia_core i2c_i801 pcspkr snd_hda_intel
> i2c_core
> ftdi_sio iTCO_wdt usbserial snd_pcm_oss snd_mixer_oss snd_pcm snd_timer
> hci_usb
> bluetooth snd soundcore snd_page_alloc video output bay wmi button ac battery
> intel_agp agpgart evdev dcdbas reiserfs sha256_generic aes_i586 aes_generic
> cbc
> dm_crypt crypto_blkcipher dm_mirror dm_log dm_snapshot dm_mod ide_cd_mod
> cdrom
> sd_mod usbhid hid ff_memless ide_pci_generic piix ide_core ohci1394 ata_piix
> ata_generic ieee1394 libata scsi_mod tg3 ehci_hcd uhci_hcd dock usbcore
> thermal
> processor fan thermal_sys
> [  138.831587]
> [  138.831587] Pid: 722, comm: khubd Not tainted (2.6.26-1-686 #1)
> [  138.831587] EIP: 0060:[<c02b9012>] EFLAGS: 00010093 CPU: 1
> [  138.831587] EIP is at _spin_lock_irqsave+0x1d/0x2f
> [  138.831587] EAX: 00000100 EBX: 00000000 ECX: 00000293 EDX: 00000078
> [  138.831587] ESI: 00000078 EDI: f407cb40 EBP: 00000001 ESP: f78bbea0
> [  138.831587]  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> [  138.831587] Process khubd (pid: 722, ti=f78ba000 task=f78c6ac0
> task.ti=f78ba000)
> [  138.831587] Stack: f9545980 f407cb40 f8dba6d1 00000004 f407cb78 f8dba704
> f407cb74 f8dba6d1
> [  138.831587]        00000004 c01de21a f407cb74 f407cb40 f8dba5e1 f7c5ce00
> f8dbac43 f360ec1c
> [  138.831587]        f360ec00 f360ec1c f954ada0 f364b400 f8b88a03 00000000
> f360ec1c f954add4
> [  138.831587] Call Trace:
> [  138.831587]  [<f9545980>] mos7840_shutdown+0x5e/0xc6 [mos7840]
> [  138.831587]  [<f8dba6d1>] destroy_serial+0x0/0xf0 [usbserial]
> [  138.831587]  [<f8dba704>] destroy_serial+0x33/0xf0 [usbserial]
> [  138.831587]  [<f8dba6d1>] destroy_serial+0x0/0xf0 [usbserial]
> [  138.831587]  [<c01de21a>] kref_put+0x36/0x40
> [  138.831587]  [<f8dba5e1>] usb_serial_put+0x1c/0x27 [usbserial]
> [  138.831587]  [<f8dbac43>] usb_serial_disconnect+0x83/0xa8 [usbserial]
> [  138.831587]  [<f8b88a03>] usb_unbind_interface+0x44/0x85 [usbcore]
> [  138.831587]  [<c023a331>] __device_release_driver+0x58/0x76
> [  138.831587]  [<c023a367>] device_release_driver+0x18/0x21
> [  138.831587]  [<c0239a7a>] bus_remove_device+0x6b/0x7b
> [  138.831587]  [<c0238b72>] device_del+0xc0/0x111
> [  138.831587]  [<f8b8674a>] usb_disable_device+0x5c/0xbb [usbcore]
> [  138.831587]  [<f8b82e2d>] usb_disconnect+0x6f/0x106 [usbcore]
> [  138.831587]  [<f8b83d53>] hub_thread+0x346/0xb04 [usbcore]
> [  138.831587]  [<c013177c>] autoremove_wake_function+0x0/0x2d
> [  138.831587]  [<f8b83a0d>] hub_thread+0x0/0xb04 [usbcore]
> [  138.831587]  [<c01316bb>] kthread+0x38/0x5d
> [  138.831587]  [<c0131683>] kthread+0x0/0x5d
> [  138.831587]  [<c01044f3>] kernel_thread_helper+0x7/0x10
> [  138.831587]  =======================
> [  138.831587] Code: d7 e6 ff f0 fe 00 8b 04 24 e9 18 d7 e6 ff 89 c2 9c 58 0f
> 1f 84 00 00 00 00 00 89 c1 fa 0f 1f 84 00 00 00 00 00 90 b8 00 01 00 00 <f0>
> 66
> 0f c1 02 38 e0 74 06 f3 90 8a 02 eb f6 89 c8 c3 89 c2 fa
> [  138.831587] EIP: [<c02b9012>] _spin_lock_irqsave+0x1d/0x2f SS:ESP
> 0068:f78bbea0
> [  138.831587] ---[ end trace 795fe733cedc32e7 ]---
> 
> 

Incidentally I have contacted the previous mos7840 maintainer, Paul Schroeder, but he is unable to help as he no longer has access to mos7840 hardware.

On Thu, Sep 18, 2008 at 03:20:50AM -0700, Andrew Morton wrote:
> 
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
> 
> On Thu, 18 Sep 2008 03:02:40 -0700 (PDT) bugme-daemon@bugzilla.kernel.org
> wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=11585
> > 
> >            Summary: mos7840 usb-serial driver sends wrong chars, returns
> >                     wrong ioctl results, and OOPSes when device unplugged.
> 
> Is that all?  Gee you're picky!

A bad habit of mine :)

Apols that I already added to Bugzilla before I saw your mail.  There are
futher dmesg logs with debug=1 enabled for usb-serial.c and the mos7840
driver, made during a brief application test.  Also application strace
logs showing the incorrect TIOCMGET results.

I contacted Paul Schroeder who is named as author in the mos7840 sources,
but he no longer works for the same vendor and doesn't have either time
or access to hardware to help.

Nick

On Thu, 18 Sep 2008, Andrew Morton wrote:

> > http://bugzilla.kernel.org/show_bug.cgi?id=11585
> > 
> >            Summary: mos7840 usb-serial driver sends wrong chars, returns
> >                     wrong ioctl results, and OOPSes when device unplugged.

> > The mos7840 usb-serial driver worked for me in 2.6.26.1.
> > 
> > Since the "usb-serial: don't release unregistered minors" change to
> > usb-serial.c which was committed to 2.6.26.3, the mos7840 driver fails in
> > several ways.

> > If I revert usb-serial.c to the version from 2.6.26.1, backing out the
> > "unregistered minors" change, but leaving all other kernel source and
> > config as at 2.6.26.3, then the mos7840 driver again fully works.

This isn't clear.  Are you taking the usb-serial.c source file from
2.6.26.1 and putting it in the 2.6.26.3 tree, or are you reverting that
one patch from the 2.6.26.3 source file?

> > So I guess that the usb-serial.c change is having unexpected effects
> > in conjunction with the mos7840 driver.  I note one of the changes in
> > this patch was to remove a check for NULL serial device pointer - looks
> quite
> > relevant in the context of the OOPS.

The code with that NULL pointer check gets called from only one place, 
and the pointer is dereferenced before the call occurs.  So when the 
check occurs the pointer can never be NULL.  That's why I removed it.

> > [  138.831587] Call Trace:
> > [  138.831587]  [<f9545980>] mos7840_shutdown+0x5e/0xc6 [mos7840]
> > [  138.831587]  [<f8dba6d1>] destroy_serial+0x0/0xf0 [usbserial]
> > [  138.831587]  [<f8dba704>] destroy_serial+0x33/0xf0 [usbserial]

Indeed, this oops happened in the call to mos7840_shutdown(), _before_
the call to return_serial() (which is where the patch made its
changes and where the pointer check was removed).

You should debug the mos7840 routine; it has had its own changes.

Alan Stern

On Thu, 18 Sep 2008, Nick Leverton wrote:

> I contacted Paul Schroeder who is named as author in the mos7840 sources,
> but he no longer works for the same vendor and doesn't have either time
> or access to hardware to help.

You should contact the people who made recent changes to the mos7840 
driver, not the original author.

Alan Stern

Created attachment 20421 [details]
this patch fixes the order of assigning minor and calling attach()

This patch effectively fixes the bug for me. It does so by assigning minor number to device before calling attach() function. The reason for this bug to appear is apparently that mos7840_startup() function uses minor variable, which was unitialized at the moment. The reason for this to appear now, but not earlier, is that mos7840 module was supposedly tested only using just one device, for which case minor was equal to the same zero value before and after its initialization. The TTY_NO_MINOR patch changed initial value, and that broke everything.
I can't check if this patch breaks other devices, because I don't have any. Neither do I have enough experience to figure this out from reading sources.

On Tue, Mar 03, 2009 at 11:14:36PM -0800, bugme-daemon@bugzilla.kernel.org wrote:
> this patch fixes the order of assigning minor and calling attach()

Can you please send this through email as described in
Documentation/SubmittingPatches to me and the linux-usb mailing list?

I confirm the bug described here, and I confirm that patch
  http://bugzilla.kernel.org/attachment.cgi?id=20421
fixes the bug for me.

I tested with a USOPTL4-4 USB<->serial adapter from B&B Electronics.  Without the patch, the device does not operate, and the kernel generally fails as described above.  With the patch, the device operates as it should.

I applied the patch to the 2.6.27.19 kernel, compiled it for X86_64, and tested it on fedora 9.

Should be fixed in 2.6.30.  If not, please let the developers at linux-usb@vger.kernel.org know about it.