2014-06-02 14:25:46

by Mike Remski

[permalink] [raw]
Subject: ftdi_sio BUG: NULL pointer dereference

Please CC me as not subscribed to list.
Third party device, with FTDI chip on it. Get this when plugging device
in. Discovered in kernel 2.6.32, comparing ftdi_sio.c in 2.6.32 to
latest 3.14 code is fundamentally unchanged. Crash looks to be
occurring in ftdi_set_max_packet_size, at the point where
priv->max_packet_size is being set off of ep_desc (about line 1503). It
looks like ep_desc is probably NULL since priv should be nonzero (set in
ftdi_sio_port_probe() which calls ftdi_set_max_packet_size()) The
num_endpoints detected for this device is 0; is this valid? Tracing
use of priv->max_packet_size in the driver it it looks like doing
something like:

priv->max_packet_size = (ep_desc) ?
le16_to_cpu(ep_desc->wMaxPacketSize) : 0x40;

would alleviate the crash, I'd like to verify that this would be valid
to do.

thanks

Jun 2 08:22:23 evoc-rnic45 kernel: ftdi_sio 2-1:1.2: FTDI USB Serial
Device converter detected
Jun 2 08:22:23 evoc-rnic45 kernel: usb 2-1: Detected FT2232C
Jun 2 08:22:23 evoc-rnic45 kernel: usb 2-1: Number of endpoints 0
Jun 2 08:22:23 evoc-rnic45 kernel: BUG: unable to handle kernel NULL
pointer dereference at 00000030
Jun 2 08:22:23 evoc-rnic45 kernel: IP: [<f8abd4a7>]
ftdi_sio_port_probe+0x267/0x730 [ftdi_sio]
Jun 2 08:22:23 evoc-rnic45 kernel: *pdpt = 0000000035e4a001 *pde =
000000003dd29067
Jun 2 08:22:23 evoc-rnic45 kernel: Oops: 0000 [#1] SMP
Jun 2 08:22:23 evoc-rnic45 kernel: last sysfs file:
/sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/manufacturer
Jun 2 08:22:23 evoc-rnic45 kernel: Modules linked in: ftdi_sio
usbserial mlSysLeds(U) coretemp hwmon sunrpc cpufreq_ondemand
acpi_cpufreq ipv6 ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_state
nf_conntrack iptable_filter ip_tables snd_usb_audio snd_seq snd_pcm
snd_timer snd_page_alloc snd_usb_lib snd_rawmidi snd_seq_device
snd_hwdep snd soundcore serio_raw i2c_i801 sg iTCO_wdt
iTCO_vendor_support r8169 mii ext4 jbd2 ext3 jbd mbcache sd_mod
crc_t10dif pata_acpi ata_generic ata_piix i915 drm_kms_helper drm
i2c_algo_bit i2c_core video output dm_mod [last unloaded: scsi_wait_scan]
Jun 2 08:22:23 evoc-rnic45 kernel:
Jun 2 08:22:23 evoc-rnic45 kernel: Pid: 29, comm: khubd Not tainted
(2.6.32-71.29.1.el6.i686 #1) To Be Filled By O.E.M.
Jun 2 08:22:23 evoc-rnic45 kernel: EIP: 0060:[<f8abd4a7>] EFLAGS:
00010246 CPU: 1
Jun 2 08:22:23 evoc-rnic45 kernel: EIP is at
ftdi_sio_port_probe+0x267/0x730 [ftdi_sio]
Jun 2 08:22:23 evoc-rnic45 kernel: EAX: 0000002c EBX: f5dbb400 ECX:
f5df5864 EDX: f5c1f440
Jun 2 08:22:23 evoc-rnic45 kernel: ESI: f5df5800 EDI: f5dbb4f0 EBP:
00000000 ESP: f716da58
Jun 2 08:22:23 evoc-rnic45 kernel: DS: 007b ES: 007b FS: 00d8 GS: 00e0
SS: 0068
Jun 2 08:22:23 evoc-rnic45 kernel: Process khubd (pid: 29, ti=f716c000
task=f70f9030 task.ti=f716c000)
Jun 2 08:22:23 evoc-rnic45 kernel: Stack:
Jun 2 08:22:23 evoc-rnic45 kernel: f8abef40 c0976b2a f61e1d00 00000000
00000004 c057a904 f716da9c f6206c00
Jun 2 08:22:23 evoc-rnic45 kernel: <0> f6206c00 f5df5864 0000002c
00000000 f5c1f440 f6205458 f5de4ed4 00000000
Jun 2 08:22:23 evoc-rnic45 kernel: <0> c057b236 00000000 f5dbb4f0
f5dbb400 f8ac1ea0 f8a93c7c f5dbb4f8 00000000
Jun 2 08:22:23 evoc-rnic45 kernel: Call Trace:
Jun 2 08:22:23 evoc-rnic45 kernel: [<c057a904>] ?
sysfs_addrm_finish+0x14/0x200
Jun 2 08:22:23 evoc-rnic45 kernel: [<c057b236>] ?
sysfs_do_create_link+0xa6/0x120
Jun 2 08:22:23 evoc-rnic45 kernel: [<f8a93c7c>] ?
usb_serial_device_probe+0x5c/0xd0 [usbserial]
Jun 2 08:22:23 evoc-rnic45 kernel: [<c057b2df>] ?
sysfs_create_link+0xf/0x20
Jun 2 08:22:23 evoc-rnic45 kernel: [<c06a1c77>] ?
driver_probe_device+0x87/0x290
Jun 2 08:22:23 evoc-rnic45 kernel: [<c07f7e62>] ? klist_next+0x62/0xd0
Jun 2 08:22:23 evoc-rnic45 kernel: [<c06a1f00>] ? __device_attach+0x0/0x50
Jun 2 08:22:23 evoc-rnic45 kernel: [<c06a0db2>] ?
bus_for_each_drv+0x52/0x80
Jun 2 08:22:23 evoc-rnic45 kernel: [<c06a1ff2>] ? device_attach+0x72/0x90
Jun 2 08:22:23 evoc-rnic45 kernel: [<c06a1f00>] ? __device_attach+0x0/0x50
Jun 2 08:22:23 evoc-rnic45 kernel: [<c06a0b7d>] ?
bus_probe_device+0x1d/0x40
Jun 2 08:22:23 evoc-rnic45 kernel: [<c069f336>] ? device_add+0

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953


2014-06-02 14:34:20

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Mon, Jun 02, 2014 at 10:25:39AM -0400, Mike Remski wrote:
> Please CC me as not subscribed to list.
> Third party device, with FTDI chip on it. Get this when plugging device
> in. Discovered in kernel 2.6.32

Wow, that kernel is ancient. Please upgrade to a more recent kernel such
as v3.14.5, or you need to get support from whatever vendor is forcing
you to use such an old kernel.

Good luck,
Johan

2014-06-02 15:16:17

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

Thanks Johan, that's why I looked at the cross references for ftdi_sio.c
over on
http://lxr.free-electrons.com/source/drivers/usb/serial/ftdi_sio.c,
latest version it's showing is 3.14. It appears that the ftdi_sio code
is largely the same as in 2.6.32, particularly in the
ftdi_set_max_packet_size() function.

I'm trying to verify if the "number of endpoints is 0" is a valid
situation.

Just a typical day of "fun with kernels and devices".


On 06/02/2014 10:33 AM, Johan Hovold wrote:
> On Mon, Jun 02, 2014 at 10:25:39AM -0400, Mike Remski wrote:
>> Please CC me as not subscribed to list.
>> Third party device, with FTDI chip on it. Get this when plugging device
>> in. Discovered in kernel 2.6.32
> Wow, that kernel is ancient. Please upgrade to a more recent kernel such
> as v3.14.5, or you need to get support from whatever vendor is forcing
> you to use such an old kernel.
>
> Good luck,
> Johan


--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-02 15:41:07

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

[ Please avoid top-posting. ]

On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
> Thanks Johan, that's why I looked at the cross references for ftdi_sio.c
> over on
> http://lxr.free-electrons.com/source/drivers/usb/serial/ftdi_sio.c,
> latest version it's showing is 3.14. It appears that the ftdi_sio code
> is largely the same as in 2.6.32, particularly in the
> ftdi_set_max_packet_size() function.

Lots of things have changed since v2.6.32 and not just in the driver
itself but in all the infrastructure it relies on.

> I'm trying to verify if the "number of endpoints is 0" is a valid
> situation.

No, that is not normal, but it should not crash the driver if it's a
hardware issue. What is the lsusb -v output of your device (make sure
the ftdi_sio driver isn't loaded when connecting the device).

And what happens if you plug it into a machine running a recent kernel?

Thanks,
Johan

2014-06-02 16:02:45

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/02/2014 11:40 AM, Johan Hovold wrote:
> [ Please avoid top-posting. ]
>
> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
>> Thanks Johan, that's why I looked at the cross references for ftdi_sio.c
>> over on
>> http://lxr.free-electrons.com/source/drivers/usb/serial/ftdi_sio.c,
>> latest version it's showing is 3.14. It appears that the ftdi_sio code
>> is largely the same as in 2.6.32, particularly in the
>> ftdi_set_max_packet_size() function.
> Lots of things have changed since v2.6.32 and not just in the driver
> itself but in all the infrastructure it relies on.
>
>> I'm trying to verify if the "number of endpoints is 0" is a valid
>> situation.
> No, that is not normal, but it should not crash the driver if it's a
> hardware issue. What is the lsusb -v output of your device (make sure
> the ftdi_sio driver isn't loaded when connecting the device).
>
> And what happens if you plug it into a machine running a recent kernel?
>
> Thanks,
> Johan
Thanks Johan; sorry for top posting.
An even more ancient kernel (2.6.18) has v1.4.3 of the ftdi_sio.c and I
do not see this problem. In v1.4.3,
ftdi_sio_attach() is doing the work that is now in
ftdi_sio_port_probe(), but 1.4.3 does not have the
ftdi_set_max_packet_size() code. The device works fine under 2.6.18. I
have to see if I can scrounge up
a machine running a later kernel (other than my desktop).
Should have included this earlier but its unmodified CentOs 6.0 and 6.4,
kernel is:

Linux nicA91A84 2.6.32-71.29.1.el6.i686 #1 SMP Tue May 10 17:35:05 CDT
2011 i686 i686 i386 GNU/Linux


lsusb -v cut for the device in question.

Bus 005 Device 003: ID 1fdf:1001
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 239 Miscellaneous Device
bDeviceSubClass 2 ?
bDeviceProtocol 1 Interface Association
bMaxPacketSize0 64
idVendor 0x1fdf
idProduct 0x1001
bcdDevice 0.03
iManufacturer 1 Sepura
iProduct 2 Colour Console
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 134
bNumInterfaces 4
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xc0
Self Powered
MaxPower 100mA
Interface Association:
bLength 8
bDescriptorType 11
bFirstInterface 0
bInterfaceCount 2
bFunctionClass 2 Communications
bFunctionSubClass 2 Abstract (modem)
bFunctionProtocol 0 None
iFunction 0
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 2 Communications
bInterfaceSubClass 2 Abstract (modem)
bInterfaceProtocol 0 None
iInterface 0
CDC Header:
bcdCDC 1.10
CDC Call Management:
bmCapabilities 0x01
call management
bDataInterface 1
CDC ACM:
bmCapabilities 0x02
line coding and serial state
CDC Union:
bMasterInterface 0
bSlaveInterface 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 10
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 10 CDC Data
bInterfaceSubClass 0 Unused
bInterfaceProtocol 0
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01 EP 1 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Interface Association:
bLength 8
bDescriptorType 11
bFirstInterface 2
bInterfaceCount 2
bFunctionClass 2 Communications
bFunctionSubClass 2 Abstract (modem)
bFunctionProtocol 0 None
iFunction 0
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 2
bAlternateSetting 0
bNumEndpoints 0
bInterfaceClass 2 Communications
bInterfaceSubClass 2 Abstract (modem)
bInterfaceProtocol 0 None
iInterface 0
CDC Header:
bcdCDC 1.10
CDC Call Management:
bmCapabilities 0x01
call management
bDataInterface 3
CDC ACM:
bmCapabilities 0x02
line coding and serial state
CDC Union:
bMasterInterface 2
bSlaveInterface 3
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 3
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 10 CDC Data
bInterfaceSubClass 0 Unused
bInterfaceProtocol 0
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x04 EP 4 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x85 EP 5 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Device Status: 0x0001
Self Powered

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-02 16:09:40

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/02/2014 11:40 AM, Johan Hovold wrote:
> [ Please avoid top-posting. ]
>
> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
>> Thanks Johan, that's why I looked at the cross references for ftdi_sio.c
>> over on
>> http://lxr.free-electrons.com/source/drivers/usb/serial/ftdi_sio.c,
>> latest version it's showing is 3.14. It appears that the ftdi_sio code
>> is largely the same as in 2.6.32, particularly in the
>> ftdi_set_max_packet_size() function.
> Lots of things have changed since v2.6.32 and not just in the driver
> itself but in all the infrastructure it relies on.
>
>> I'm trying to verify if the "number of endpoints is 0" is a valid
>> situation.
> No, that is not normal, but it should not crash the driver if it's a
> hardware issue. What is the lsusb -v output of your device (make sure
> the ftdi_sio driver isn't loaded when connecting the device).
>
> And what happens if you plug it into a machine running a recent kernel?
>
> Thanks,
> Johan
Plugging device into a system running Redhat, kernel 3.3.4 crashes the
system.

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-02 16:20:15

by Greg KH

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Mon, Jun 02, 2014 at 12:09:36PM -0400, Mike Remski wrote:
> On 06/02/2014 11:40 AM, Johan Hovold wrote:
> >[ Please avoid top-posting. ]
> >
> >On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
> >>Thanks Johan, that's why I looked at the cross references for ftdi_sio.c
> >>over on
> >>http://lxr.free-electrons.com/source/drivers/usb/serial/ftdi_sio.c,
> >>latest version it's showing is 3.14. It appears that the ftdi_sio code
> >>is largely the same as in 2.6.32, particularly in the
> >>ftdi_set_max_packet_size() function.
> >Lots of things have changed since v2.6.32 and not just in the driver
> >itself but in all the infrastructure it relies on.
> >
> >>I'm trying to verify if the "number of endpoints is 0" is a valid
> >>situation.
> >No, that is not normal, but it should not crash the driver if it's a
> >hardware issue. What is the lsusb -v output of your device (make sure
> >the ftdi_sio driver isn't loaded when connecting the device).
> >
> >And what happens if you plug it into a machine running a recent kernel?
> >
> >Thanks,
> >Johan
> Plugging device into a system running Redhat, kernel 3.3.4 crashes the
> system.

3.3.4 is still _many_ years old. Please try something from 2014 at the
latest...

2014-06-02 16:20:58

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
> On 06/02/2014 11:40 AM, Johan Hovold wrote:
> > [ Please avoid top-posting. ]
> >
> > On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
> >> Thanks Johan, that's why I looked at the cross references for ftdi_sio.c
> >> over on
> >> http://lxr.free-electrons.com/source/drivers/usb/serial/ftdi_sio.c,
> >> latest version it's showing is 3.14. It appears that the ftdi_sio code
> >> is largely the same as in 2.6.32, particularly in the
> >> ftdi_set_max_packet_size() function.
> > Lots of things have changed since v2.6.32 and not just in the driver
> > itself but in all the infrastructure it relies on.
> >
> >> I'm trying to verify if the "number of endpoints is 0" is a valid
> >> situation.
> > No, that is not normal, but it should not crash the driver if it's a
> > hardware issue. What is the lsusb -v output of your device (make sure
> > the ftdi_sio driver isn't loaded when connecting the device).
> >
> > And what happens if you plug it into a machine running a recent kernel?
> >
> > Thanks,
> > Johan
> Thanks Johan; sorry for top posting.
> An even more ancient kernel (2.6.18) has v1.4.3 of the ftdi_sio.c and I
> do not see this problem. In v1.4.3,
> ftdi_sio_attach() is doing the work that is now in
> ftdi_sio_port_probe(), but 1.4.3 does not have the
> ftdi_set_max_packet_size() code. The device works fine under 2.6.18. I
> have to see if I can scrounge up
> a machine running a later kernel (other than my desktop).

Your desktop should be just fine for testing.

> Should have included this earlier but its unmodified CentOs 6.0 and 6.4,
> kernel is:
>
> Linux nicA91A84 2.6.32-71.29.1.el6.i686 #1 SMP Tue May 10 17:35:05 CDT
> 2011 i686 i686 i386 GNU/Linux
>
>
> lsusb -v cut for the device in question.
>
> Bus 005 Device 003: ID 1fdf:1001
> Device Descriptor:
> bLength 18
> bDescriptorType 1
> bcdUSB 2.00
> bDeviceClass 239 Miscellaneous Device
> bDeviceSubClass 2 ?
> bDeviceProtocol 1 Interface Association
> bMaxPacketSize0 64
> idVendor 0x1fdf
> idProduct 0x1001
> bcdDevice 0.03
> iManufacturer 1 Sepura
> iProduct 2 Colour Console
> iSerial 0
> bNumConfigurations 1
> Configuration Descriptor:
> bLength 9
> bDescriptorType 2
> wTotalLength 134
> bNumInterfaces 4
> bConfigurationValue 1
> iConfiguration 0
> bmAttributes 0xc0
> Self Powered
> MaxPower 100mA
> Interface Association:
> bLength 8
> bDescriptorType 11
> bFirstInterface 0
> bInterfaceCount 2
> bFunctionClass 2 Communications
> bFunctionSubClass 2 Abstract (modem)

Is this indeed the right device? Then you seem to be using the wrong
driver as this is no FTDI device, but a CDC-ACM modem-class device which
should be driven by the cdc-acm driver.

> bFunctionProtocol 0 None
> iFunction 0
> Interface Descriptor:
> bLength 9
> bDescriptorType 4
> bInterfaceNumber 0
> bAlternateSetting 0
> bNumEndpoints 1
> bInterfaceClass 2 Communications
> bInterfaceSubClass 2 Abstract (modem)
> bInterfaceProtocol 0 None
> iInterface 0
> CDC Header:
> bcdCDC 1.10
> CDC Call Management:
> bmCapabilities 0x01
> call management
> bDataInterface 1
> CDC ACM:
> bmCapabilities 0x02
> line coding and serial state
> CDC Union:
> bMasterInterface 0
> bSlaveInterface 1
> Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x83 EP 3 IN
> bmAttributes 3
> Transfer Type Interrupt
> Synch Type None
> Usage Type Data
> wMaxPacketSize 0x0040 1x 64 bytes
> bInterval 10
> Interface Descriptor:
> bLength 9
> bDescriptorType 4
> bInterfaceNumber 1
> bAlternateSetting 0
> bNumEndpoints 2
> bInterfaceClass 10 CDC Data
> bInterfaceSubClass 0 Unused
> bInterfaceProtocol 0
> iInterface 0
> Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x01 EP 1 OUT
> bmAttributes 2
> Transfer Type Bulk
> Synch Type None
> Usage Type Data
> wMaxPacketSize 0x0040 1x 64 bytes
> bInterval 0
> Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x82 EP 2 IN
> bmAttributes 2
> Transfer Type Bulk
> Synch Type None
> Usage Type Data
> wMaxPacketSize 0x0040 1x 64 bytes
> bInterval 0
> Interface Association:
> bLength 8
> bDescriptorType 11
> bFirstInterface 2
> bInterfaceCount 2
> bFunctionClass 2 Communications
> bFunctionSubClass 2 Abstract (modem)
> bFunctionProtocol 0 None
> iFunction 0
> Interface Descriptor:
> bLength 9
> bDescriptorType 4
> bInterfaceNumber 2
> bAlternateSetting 0
> bNumEndpoints 0

The third interface lacks endpoints and crashes the ftdi_sio driver.
This shouldn't happen (even if you're forcing the wrong driver to bind),
so I'll fix it up if still broken in v3.15-rc.

Thanks,
Johan

2014-06-02 16:24:51

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/02/2014 12:20 PM, Johan Hovold wrote:
> On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
>> On 06/02/2014 11:40 AM, Johan Hovold wrote:
>>> [ Please avoid top-posting. ]
>>>
>>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
>>>> Thanks Johan, that's why I looked at the cross references for ftdi_sio.c
>>>> over on
>>>> http://lxr.free-electrons.com/source/drivers/usb/serial/ftdi_sio.c,
>>>> latest version it's showing is 3.14. It appears that the ftdi_sio code
>>>> is largely the same as in 2.6.32, particularly in the
>>>> ftdi_set_max_packet_size() function.
>>> Lots of things have changed since v2.6.32 and not just in the driver
>>> itself but in all the infrastructure it relies on.
>>>
>>>> I'm trying to verify if the "number of endpoints is 0" is a valid
>>>> situation.
>>> No, that is not normal, but it should not crash the driver if it's a
>>> hardware issue. What is the lsusb -v output of your device (make sure
>>> the ftdi_sio driver isn't loaded when connecting the device).
>>>
>>> And what happens if you plug it into a machine running a recent kernel?
>>>
>>> Thanks,
>>> Johan
>> Thanks Johan; sorry for top posting.
>> An even more ancient kernel (2.6.18) has v1.4.3 of the ftdi_sio.c and I
>> do not see this problem. In v1.4.3,
>> ftdi_sio_attach() is doing the work that is now in
>> ftdi_sio_port_probe(), but 1.4.3 does not have the
>> ftdi_set_max_packet_size() code. The device works fine under 2.6.18. I
>> have to see if I can scrounge up
>> a machine running a later kernel (other than my desktop).
> Your desktop should be just fine for testing.
>
>> Should have included this earlier but its unmodified CentOs 6.0 and 6.4,
>> kernel is:
>>
>> Linux nicA91A84 2.6.32-71.29.1.el6.i686 #1 SMP Tue May 10 17:35:05 CDT
>> 2011 i686 i686 i386 GNU/Linux
>>
>>
>> lsusb -v cut for the device in question.
>>
>> Bus 005 Device 003: ID 1fdf:1001
>> Device Descriptor:
>> bLength 18
>> bDescriptorType 1
>> bcdUSB 2.00
>> bDeviceClass 239 Miscellaneous Device
>> bDeviceSubClass 2 ?
>> bDeviceProtocol 1 Interface Association
>> bMaxPacketSize0 64
>> idVendor 0x1fdf
>> idProduct 0x1001
>> bcdDevice 0.03
>> iManufacturer 1 Sepura
>> iProduct 2 Colour Console
>> iSerial 0
>> bNumConfigurations 1
>> Configuration Descriptor:
>> bLength 9
>> bDescriptorType 2
>> wTotalLength 134
>> bNumInterfaces 4
>> bConfigurationValue 1
>> iConfiguration 0
>> bmAttributes 0xc0
>> Self Powered
>> MaxPower 100mA
>> Interface Association:
>> bLength 8
>> bDescriptorType 11
>> bFirstInterface 0
>> bInterfaceCount 2
>> bFunctionClass 2 Communications
>> bFunctionSubClass 2 Abstract (modem)
> Is this indeed the right device? Then you seem to be using the wrong
> driver as this is no FTDI device, but a CDC-ACM modem-class device which
> should be driven by the cdc-acm driver.
>
>> bFunctionProtocol 0 None
>> iFunction 0
>> Interface Descriptor:
>> bLength 9
>> bDescriptorType 4
>> bInterfaceNumber 0
>> bAlternateSetting 0
>> bNumEndpoints 1
>> bInterfaceClass 2 Communications
>> bInterfaceSubClass 2 Abstract (modem)
>> bInterfaceProtocol 0 None
>> iInterface 0
>> CDC Header:
>> bcdCDC 1.10
>> CDC Call Management:
>> bmCapabilities 0x01
>> call management
>> bDataInterface 1
>> CDC ACM:
>> bmCapabilities 0x02
>> line coding and serial state
>> CDC Union:
>> bMasterInterface 0
>> bSlaveInterface 1
>> Endpoint Descriptor:
>> bLength 7
>> bDescriptorType 5
>> bEndpointAddress 0x83 EP 3 IN
>> bmAttributes 3
>> Transfer Type Interrupt
>> Synch Type None
>> Usage Type Data
>> wMaxPacketSize 0x0040 1x 64 bytes
>> bInterval 10
>> Interface Descriptor:
>> bLength 9
>> bDescriptorType 4
>> bInterfaceNumber 1
>> bAlternateSetting 0
>> bNumEndpoints 2
>> bInterfaceClass 10 CDC Data
>> bInterfaceSubClass 0 Unused
>> bInterfaceProtocol 0
>> iInterface 0
>> Endpoint Descriptor:
>> bLength 7
>> bDescriptorType 5
>> bEndpointAddress 0x01 EP 1 OUT
>> bmAttributes 2
>> Transfer Type Bulk
>> Synch Type None
>> Usage Type Data
>> wMaxPacketSize 0x0040 1x 64 bytes
>> bInterval 0
>> Endpoint Descriptor:
>> bLength 7
>> bDescriptorType 5
>> bEndpointAddress 0x82 EP 2 IN
>> bmAttributes 2
>> Transfer Type Bulk
>> Synch Type None
>> Usage Type Data
>> wMaxPacketSize 0x0040 1x 64 bytes
>> bInterval 0
>> Interface Association:
>> bLength 8
>> bDescriptorType 11
>> bFirstInterface 2
>> bInterfaceCount 2
>> bFunctionClass 2 Communications
>> bFunctionSubClass 2 Abstract (modem)
>> bFunctionProtocol 0 None
>> iFunction 0
>> Interface Descriptor:
>> bLength 9
>> bDescriptorType 4
>> bInterfaceNumber 2
>> bAlternateSetting 0
>> bNumEndpoints 0
> The third interface lacks endpoints and crashes the ftdi_sio driver.
> This shouldn't happen (even if you're forcing the wrong driver to bind),
> so I'll fix it up if still broken in v3.15-rc.
>
> Thanks,
> Johan
Johan,
Thanks again. Yes, the device does indeed have an FTDI embedded in it;
they've programmed in their own ids. They supply a Windows driver for
it, but that doesn't do me any good. :)

I will work on getting a later version set up for testing. May not
happen today but as soon as possible.


--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-02 16:26:09

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/02/2014 12:23 PM, Greg KH wrote:
> On Mon, Jun 02, 2014 at 12:09:36PM -0400, Mike Remski wrote:
>> On 06/02/2014 11:40 AM, Johan Hovold wrote:
>>> [ Please avoid top-posting. ]
>>>
>>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
>>>> Thanks Johan, that's why I looked at the cross references for ftdi_sio.c
>>>> over on
>>>> http://lxr.free-electrons.com/source/drivers/usb/serial/ftdi_sio.c,
>>>> latest version it's showing is 3.14. It appears that the ftdi_sio code
>>>> is largely the same as in 2.6.32, particularly in the
>>>> ftdi_set_max_packet_size() function.
>>> Lots of things have changed since v2.6.32 and not just in the driver
>>> itself but in all the infrastructure it relies on.
>>>
>>>> I'm trying to verify if the "number of endpoints is 0" is a valid
>>>> situation.
>>> No, that is not normal, but it should not crash the driver if it's a
>>> hardware issue. What is the lsusb -v output of your device (make sure
>>> the ftdi_sio driver isn't loaded when connecting the device).
>>>
>>> And what happens if you plug it into a machine running a recent kernel?
>>>
>>> Thanks,
>>> Johan
>> Plugging device into a system running Redhat, kernel 3.3.4 crashes the
>> system.
> 3.3.4 is still _many_ years old. Please try something from 2014 at the
> latest...

Understood Greg; it was the best I had immediately available. I am
working on setting up/finding a later kernel at the moment.

thanks

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-02 16:50:30

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Mon, Jun 02, 2014 at 12:24:44PM -0400, Mike Remski wrote:
> On 06/02/2014 12:20 PM, Johan Hovold wrote:
> > On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
> >> On 06/02/2014 11:40 AM, Johan Hovold wrote:
> >>> [ Please avoid top-posting. ]
> >>>
> >>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:

> >> lsusb -v cut for the device in question.
> >>
> >> Bus 005 Device 003: ID 1fdf:1001
> >> Device Descriptor:
> >> bLength 18
> >> bDescriptorType 1
> >> bcdUSB 2.00
> >> bDeviceClass 239 Miscellaneous Device
> >> bDeviceSubClass 2 ?
> >> bDeviceProtocol 1 Interface Association
> >> bMaxPacketSize0 64
> >> idVendor 0x1fdf
> >> idProduct 0x1001
> >> bcdDevice 0.03
> >> iManufacturer 1 Sepura
> >> iProduct 2 Colour Console
> >> iSerial 0
> >> bNumConfigurations 1
> >> Configuration Descriptor:
> >> bLength 9
> >> bDescriptorType 2
> >> wTotalLength 134
> >> bNumInterfaces 4
> >> bConfigurationValue 1
> >> iConfiguration 0
> >> bmAttributes 0xc0
> >> Self Powered
> >> MaxPower 100mA
> >> Interface Association:
> >> bLength 8
> >> bDescriptorType 11
> >> bFirstInterface 0
> >> bInterfaceCount 2
> >> bFunctionClass 2 Communications
> >> bFunctionSubClass 2 Abstract (modem)
> > Is this indeed the right device? Then you seem to be using the wrong
> > driver as this is no FTDI device, but a CDC-ACM modem-class device which
> > should be driven by the cdc-acm driver.
> >
> >> bFunctionProtocol 0 None
> >> iFunction 0
> >> Interface Descriptor:
> >> bLength 9
> >> bDescriptorType 4
> >> bInterfaceNumber 0
> >> bAlternateSetting 0
> >> bNumEndpoints 1
> >> bInterfaceClass 2 Communications
> >> bInterfaceSubClass 2 Abstract (modem)
> >> bInterfaceProtocol 0 None
> >> iInterface 0
> >> CDC Header:
> >> bcdCDC 1.10
> >> CDC Call Management:
> >> bmCapabilities 0x01
> >> call management
> >> bDataInterface 1
> >> CDC ACM:
> >> bmCapabilities 0x02
> >> line coding and serial state
> >> CDC Union:
> >> bMasterInterface 0
> >> bSlaveInterface 1
> >> Endpoint Descriptor:
> >> bLength 7
> >> bDescriptorType 5
> >> bEndpointAddress 0x83 EP 3 IN
> >> bmAttributes 3
> >> Transfer Type Interrupt
> >> Synch Type None
> >> Usage Type Data
> >> wMaxPacketSize 0x0040 1x 64 bytes
> >> bInterval 10
> >> Interface Descriptor:
> >> bLength 9
> >> bDescriptorType 4
> >> bInterfaceNumber 1
> >> bAlternateSetting 0
> >> bNumEndpoints 2
> >> bInterfaceClass 10 CDC Data
> >> bInterfaceSubClass 0 Unused
> >> bInterfaceProtocol 0
> >> iInterface 0
> >> Endpoint Descriptor:
> >> bLength 7
> >> bDescriptorType 5
> >> bEndpointAddress 0x01 EP 1 OUT
> >> bmAttributes 2
> >> Transfer Type Bulk
> >> Synch Type None
> >> Usage Type Data
> >> wMaxPacketSize 0x0040 1x 64 bytes
> >> bInterval 0
> >> Endpoint Descriptor:
> >> bLength 7
> >> bDescriptorType 5
> >> bEndpointAddress 0x82 EP 2 IN
> >> bmAttributes 2
> >> Transfer Type Bulk
> >> Synch Type None
> >> Usage Type Data
> >> wMaxPacketSize 0x0040 1x 64 bytes
> >> bInterval 0
> >> Interface Association:
> >> bLength 8
> >> bDescriptorType 11
> >> bFirstInterface 2
> >> bInterfaceCount 2
> >> bFunctionClass 2 Communications
> >> bFunctionSubClass 2 Abstract (modem)
> >> bFunctionProtocol 0 None
> >> iFunction 0
> >> Interface Descriptor:
> >> bLength 9
> >> bDescriptorType 4
> >> bInterfaceNumber 2
> >> bAlternateSetting 0
> >> bNumEndpoints 0
> > The third interface lacks endpoints and crashes the ftdi_sio driver.
> > This shouldn't happen (even if you're forcing the wrong driver to bind),
> > so I'll fix it up if still broken in v3.15-rc.
> >
> > Thanks,
> > Johan
> Johan,
> Thanks again. Yes, the device does indeed have an FTDI embedded in it;
> they've programmed in their own ids. They supply a Windows driver for
> it, but that doesn't do me any good. :)

Not just their own ID's it seems.

Have you tried just using the cdc-acm driver? The ports should up as
/dev/ttyACMx instead of ttyUSBx.

Johan

2014-06-02 17:11:42

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/02/2014 12:49 PM, Johan Hovold wrote:
> On Mon, Jun 02, 2014 at 12:24:44PM -0400, Mike Remski wrote:
>> On 06/02/2014 12:20 PM, Johan Hovold wrote:
>>> On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
>>>> On 06/02/2014 11:40 AM, Johan Hovold wrote:
>>>>> [ Please avoid top-posting. ]
>>>>>
>>>>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
>>>> lsusb -v cut for the device in question.
>>>>
>>>> Bus 005 Device 003: ID 1fdf:1001
>>>> Device Descriptor:
>>>> bLength 18
>>>> bDescriptorType 1
>>>> bcdUSB 2.00
>>>> bDeviceClass 239 Miscellaneous Device
>>>> bDeviceSubClass 2 ?
>>>> bDeviceProtocol 1 Interface Association
>>>> bMaxPacketSize0 64
>>>> idVendor 0x1fdf
>>>> idProduct 0x1001
>>>> bcdDevice 0.03
>>>> iManufacturer 1 Sepura
>>>> iProduct 2 Colour Console
>>>> iSerial 0
>>>> bNumConfigurations 1
>>>> Configuration Descriptor:
>>>> bLength 9
>>>> bDescriptorType 2
>>>> wTotalLength 134
>>>> bNumInterfaces 4
>>>> bConfigurationValue 1
>>>> iConfiguration 0
>>>> bmAttributes 0xc0
>>>> Self Powered
>>>> MaxPower 100mA
>>>> Interface Association:
>>>> bLength 8
>>>> bDescriptorType 11
>>>> bFirstInterface 0
>>>> bInterfaceCount 2
>>>> bFunctionClass 2 Communications
>>>> bFunctionSubClass 2 Abstract (modem)
>>> Is this indeed the right device? Then you seem to be using the wrong
>>> driver as this is no FTDI device, but a CDC-ACM modem-class device which
>>> should be driven by the cdc-acm driver.
>>>
>>>> bFunctionProtocol 0 None
>>>> iFunction 0
>>>> Interface Descriptor:
>>>> bLength 9
>>>> bDescriptorType 4
>>>> bInterfaceNumber 0
>>>> bAlternateSetting 0
>>>> bNumEndpoints 1
>>>> bInterfaceClass 2 Communications
>>>> bInterfaceSubClass 2 Abstract (modem)
>>>> bInterfaceProtocol 0 None
>>>> iInterface 0
>>>> CDC Header:
>>>> bcdCDC 1.10
>>>> CDC Call Management:
>>>> bmCapabilities 0x01
>>>> call management
>>>> bDataInterface 1
>>>> CDC ACM:
>>>> bmCapabilities 0x02
>>>> line coding and serial state
>>>> CDC Union:
>>>> bMasterInterface 0
>>>> bSlaveInterface 1
>>>> Endpoint Descriptor:
>>>> bLength 7
>>>> bDescriptorType 5
>>>> bEndpointAddress 0x83 EP 3 IN
>>>> bmAttributes 3
>>>> Transfer Type Interrupt
>>>> Synch Type None
>>>> Usage Type Data
>>>> wMaxPacketSize 0x0040 1x 64 bytes
>>>> bInterval 10
>>>> Interface Descriptor:
>>>> bLength 9
>>>> bDescriptorType 4
>>>> bInterfaceNumber 1
>>>> bAlternateSetting 0
>>>> bNumEndpoints 2
>>>> bInterfaceClass 10 CDC Data
>>>> bInterfaceSubClass 0 Unused
>>>> bInterfaceProtocol 0
>>>> iInterface 0
>>>> Endpoint Descriptor:
>>>> bLength 7
>>>> bDescriptorType 5
>>>> bEndpointAddress 0x01 EP 1 OUT
>>>> bmAttributes 2
>>>> Transfer Type Bulk
>>>> Synch Type None
>>>> Usage Type Data
>>>> wMaxPacketSize 0x0040 1x 64 bytes
>>>> bInterval 0
>>>> Endpoint Descriptor:
>>>> bLength 7
>>>> bDescriptorType 5
>>>> bEndpointAddress 0x82 EP 2 IN
>>>> bmAttributes 2
>>>> Transfer Type Bulk
>>>> Synch Type None
>>>> Usage Type Data
>>>> wMaxPacketSize 0x0040 1x 64 bytes
>>>> bInterval 0
>>>> Interface Association:
>>>> bLength 8
>>>> bDescriptorType 11
>>>> bFirstInterface 2
>>>> bInterfaceCount 2
>>>> bFunctionClass 2 Communications
>>>> bFunctionSubClass 2 Abstract (modem)
>>>> bFunctionProtocol 0 None
>>>> iFunction 0
>>>> Interface Descriptor:
>>>> bLength 9
>>>> bDescriptorType 4
>>>> bInterfaceNumber 2
>>>> bAlternateSetting 0
>>>> bNumEndpoints 0
>>> The third interface lacks endpoints and crashes the ftdi_sio driver.
>>> This shouldn't happen (even if you're forcing the wrong driver to bind),
>>> so I'll fix it up if still broken in v3.15-rc.
>>>
>>> Thanks,
>>> Johan
>> Johan,
>> Thanks again. Yes, the device does indeed have an FTDI embedded in it;
>> they've programmed in their own ids. They supply a Windows driver for
>> it, but that doesn't do me any good. :)
> Not just their own ID's it seems.
>
> Have you tried just using the cdc-acm driver? The ports should up as
> /dev/ttyACMx instead of ttyUSBx.
>
> Johan
Not yet, next on the list.

I'm suspecting that bNumEndpoints == 0 is causing endpoint[1].desc to
stay at NULL (line 1567 in 3.1.4.5 source), so by the time it gets used
later on, I'm hitting the NULL dereference.



Thanks.

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-02 17:47:01

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Mon, Jun 02, 2014 at 01:11:37PM -0400, Mike Remski wrote:
> On 06/02/2014 12:49 PM, Johan Hovold wrote:
> > On Mon, Jun 02, 2014 at 12:24:44PM -0400, Mike Remski wrote:
> >> On 06/02/2014 12:20 PM, Johan Hovold wrote:
> >>> On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
> >>>> On 06/02/2014 11:40 AM, Johan Hovold wrote:
> >>>>> [ Please avoid top-posting. ]
> >>>>>
> >>>>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:

> >>> The third interface lacks endpoints and crashes the ftdi_sio driver.
> >>> This shouldn't happen (even if you're forcing the wrong driver to bind),
> >>> so I'll fix it up if still broken in v3.15-rc.
> >>>
> >> Johan,
> >> Thanks again. Yes, the device does indeed have an FTDI embedded in it;
> >> they've programmed in their own ids. They supply a Windows driver for
> >> it, but that doesn't do me any good. :)
> > Not just their own ID's it seems.
> >
> > Have you tried just using the cdc-acm driver? The ports should up as
> > /dev/ttyACMx instead of ttyUSBx.
> >
> Not yet, next on the list.

You really should try this before anything else. :)

> I'm suspecting that bNumEndpoints == 0 is causing endpoint[1].desc to
> stay at NULL (line 1567 in 3.1.4.5 source), so by the time it gets used
> later on, I'm hitting the NULL dereference.

Yeah, the code is obviously broken (also in v3.15-rc). It should
probably work to just return from ftdi_set_max_packet_size if
num_endpoints is 0 if you want to try that (or you can use your ?:
construct), but I should be able to fix this up properly on Wednesday.

Thanks,
Johan

2014-06-02 17:50:44

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/02/2014 01:46 PM, Johan Hovold wrote:
> On Mon, Jun 02, 2014 at 01:11:37PM -0400, Mike Remski wrote:
>> On 06/02/2014 12:49 PM, Johan Hovold wrote:
>>> On Mon, Jun 02, 2014 at 12:24:44PM -0400, Mike Remski wrote:
>>>> On 06/02/2014 12:20 PM, Johan Hovold wrote:
>>>>> On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
>>>>>> On 06/02/2014 11:40 AM, Johan Hovold wrote:
>>>>>>> [ Please avoid top-posting. ]
>>>>>>>
>>>>>>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
>>>>> The third interface lacks endpoints and crashes the ftdi_sio driver.
>>>>> This shouldn't happen (even if you're forcing the wrong driver to bind),
>>>>> so I'll fix it up if still broken in v3.15-rc.
>>>>>
>>>> Johan,
>>>> Thanks again. Yes, the device does indeed have an FTDI embedded in it;
>>>> they've programmed in their own ids. They supply a Windows driver for
>>>> it, but that doesn't do me any good. :)
>>> Not just their own ID's it seems.
>>>
>>> Have you tried just using the cdc-acm driver? The ports should up as
>>> /dev/ttyACMx instead of ttyUSBx.
>>>
>> Not yet, next on the list.
> You really should try this before anything else. :)
>
>> I'm suspecting that bNumEndpoints == 0 is causing endpoint[1].desc to
>> stay at NULL (line 1567 in 3.1.4.5 source), so by the time it gets used
>> later on, I'm hitting the NULL dereference.
> Yeah, the code is obviously broken (also in v3.15-rc). It should
> probably work to just return from ftdi_set_max_packet_size if
> num_endpoints is 0 if you want to try that (or you can use your ?:
> construct), but I should be able to fix this up properly on Wednesday.
>
> Thanks,
> Johan

Yep, trying to get the modalias correct so the cdc_acm driver recognizes
and loads for the device in question.

Appreciate your help.

m

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-03 10:17:37

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/02/2014 01:46 PM, Johan Hovold wrote:
> On Mon, Jun 02, 2014 at 01:11:37PM -0400, Mike Remski wrote:
>> On 06/02/2014 12:49 PM, Johan Hovold wrote:
>>> On Mon, Jun 02, 2014 at 12:24:44PM -0400, Mike Remski wrote:
>>>> On 06/02/2014 12:20 PM, Johan Hovold wrote:
>>>>> On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
>>>>>> On 06/02/2014 11:40 AM, Johan Hovold wrote:
>>>>>>> [ Please avoid top-posting. ]
>>>>>>>
>>>>>>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
>>>>> The third interface lacks endpoints and crashes the ftdi_sio driver.
>>>>> This shouldn't happen (even if you're forcing the wrong driver to bind),
>>>>> so I'll fix it up if still broken in v3.15-rc.
>>>>>
>>>> Johan,
>>>> Thanks again. Yes, the device does indeed have an FTDI embedded in it;
>>>> they've programmed in their own ids. They supply a Windows driver for
>>>> it, but that doesn't do me any good. :)
>>> Not just their own ID's it seems.
>>>
>>> Have you tried just using the cdc-acm driver? The ports should up as
>>> /dev/ttyACMx instead of ttyUSBx.
>>>
>> Not yet, next on the list.
> You really should try this before anything else. :)
>
>> I'm suspecting that bNumEndpoints == 0 is causing endpoint[1].desc to
>> stay at NULL (line 1567 in 3.1.4.5 source), so by the time it gets used
>> later on, I'm hitting the NULL dereference.
> Yeah, the code is obviously broken (also in v3.15-rc). It should
> probably work to just return from ftdi_set_max_packet_size if
> num_endpoints is 0 if you want to try that (or you can use your ?:
> construct), but I should be able to fix this up properly on Wednesday.
>
> Thanks,
> Johan
Johan,
I had a chance to play around with code over in ftdi_sio.c; adding this:

if (!num_endpoints) {
return;
}
after the "Number of endpoints" message gets rid of the crash,
everything looks to be working correctly.

Thanks again

m

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-04 14:20:12

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Tue, Jun 03, 2014 at 06:17:33AM -0400, Mike Remski wrote:
> On 06/02/2014 01:46 PM, Johan Hovold wrote:
> > On Mon, Jun 02, 2014 at 01:11:37PM -0400, Mike Remski wrote:
> >> On 06/02/2014 12:49 PM, Johan Hovold wrote:
> >>> On Mon, Jun 02, 2014 at 12:24:44PM -0400, Mike Remski wrote:
> >>>> On 06/02/2014 12:20 PM, Johan Hovold wrote:
> >>>>> On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
> >>>>>> On 06/02/2014 11:40 AM, Johan Hovold wrote:
> >>>>>>> [ Please avoid top-posting. ]
> >>>>>>>
> >>>>>>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
> >>>>> The third interface lacks endpoints and crashes the ftdi_sio driver.
> >>>>> This shouldn't happen (even if you're forcing the wrong driver to bind),
> >>>>> so I'll fix it up if still broken in v3.15-rc.
> >>>>>
> >>>> Johan,
> >>>> Thanks again. Yes, the device does indeed have an FTDI embedded in it;
> >>>> they've programmed in their own ids. They supply a Windows driver for
> >>>> it, but that doesn't do me any good. :)
> >>> Not just their own ID's it seems.
> >>>
> >>> Have you tried just using the cdc-acm driver? The ports should up as
> >>> /dev/ttyACMx instead of ttyUSBx.
> >>>
> >> Not yet, next on the list.
> > You really should try this before anything else. :)

How did it go with cdc-acm?

You probably need the following commit (depending how well-supported your
old kernel is):

5b239f0aebd4d ("USB: cdc-acm: Add pseudo modem without AT
command capabilities")

in order for the device to bind.

> >> I'm suspecting that bNumEndpoints == 0 is causing endpoint[1].desc to
> >> stay at NULL (line 1567 in 3.1.4.5 source), so by the time it gets used
> >> later on, I'm hitting the NULL dereference.
> > Yeah, the code is obviously broken (also in v3.15-rc). It should
> > probably work to just return from ftdi_set_max_packet_size if
> > num_endpoints is 0 if you want to try that (or you can use your ?:
> > construct), but I should be able to fix this up properly on Wednesday.
> >
> > Thanks,
> > Johan
> Johan,
> I had a chance to play around with code over in ftdi_sio.c; adding this:
>
> if (!num_endpoints) {
> return;
> }
> after the "Number of endpoints" message gets rid of the crash,
> everything looks to be working correctly.

Good. Care to try the patch below so I can add a formal Tested-by tag as
well?

Thanks,
Johan


>From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
From: Johan Hovold <[email protected]>
Date: Wed, 4 Jun 2014 14:09:43 +0200
Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe

Fix NULL-pointer dereference when probing an interface with no
endpoints.

These devices have two bulk endpoints per interface, but this avoids
crashing the kernel if a user forces a non-FTDI device to be probed.

Note that the iterator variable was made unsigned in order to avoid
a maybe-uninitialized compiler warning for ep_desc after the loop.

Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
calculation")

Reported-by: Mike Remski <[email protected]>
Cc: <[email protected]> # 2.3.61
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/ftdi_sio.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 7c6e1dedeb06..3019141397eb 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct usb_serial_port *port)
struct usb_device *udev = serial->dev;

struct usb_interface *interface = serial->interface;
- struct usb_endpoint_descriptor *ep_desc = &interface->cur_altsetting->endpoint[1].desc;
+ struct usb_endpoint_descriptor *ep_desc;

unsigned num_endpoints;
- int i;
+ unsigned i;

num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);

+ if (!num_endpoints)
+ return;
+
/* NOTE: some customers have programmed FT232R/FT245R devices
* with an endpoint size of 0 - not good. In this case, we
* want to override the endpoint descriptor setting and use a
--
1.8.5.5

2014-06-04 14:29:41

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference


On 06/04/2014 10:19 AM, Johan Hovold wrote:
> On Tue, Jun 03, 2014 at 06:17:33AM -0400, Mike Remski wrote:
>> On 06/02/2014 01:46 PM, Johan Hovold wrote:
>>> On Mon, Jun 02, 2014 at 01:11:37PM -0400, Mike Remski wrote:
>>>> On 06/02/2014 12:49 PM, Johan Hovold wrote:
>>>>> On Mon, Jun 02, 2014 at 12:24:44PM -0400, Mike Remski wrote:
>>>>>> On 06/02/2014 12:20 PM, Johan Hovold wrote:
>>>>>>> On Mon, Jun 02, 2014 at 12:02:40PM -0400, Mike Remski wrote:
>>>>>>>> On 06/02/2014 11:40 AM, Johan Hovold wrote:
>>>>>>>>> [ Please avoid top-posting. ]
>>>>>>>>>
>>>>>>>>> On Mon, Jun 02, 2014 at 11:16:11AM -0400, Mike Remski wrote:
>>>>>>> The third interface lacks endpoints and crashes the ftdi_sio driver.
>>>>>>> This shouldn't happen (even if you're forcing the wrong driver to bind),
>>>>>>> so I'll fix it up if still broken in v3.15-rc.
>>>>>>>
>>>>>> Johan,
>>>>>> Thanks again. Yes, the device does indeed have an FTDI embedded in it;
>>>>>> they've programmed in their own ids. They supply a Windows driver for
>>>>>> it, but that doesn't do me any good. :)
>>>>> Not just their own ID's it seems.
>>>>>
>>>>> Have you tried just using the cdc-acm driver? The ports should up as
>>>>> /dev/ttyACMx instead of ttyUSBx.
>>>>>
>>>> Not yet, next on the list.
>>> You really should try this before anything else. :)
> How did it go with cdc-acm?
>
> You probably need the following commit (depending how well-supported your
> old kernel is):
>
> 5b239f0aebd4d ("USB: cdc-acm: Add pseudo modem without AT
> command capabilities")
>
> in order for the device to bind.
>
>>>> I'm suspecting that bNumEndpoints == 0 is causing endpoint[1].desc to
>>>> stay at NULL (line 1567 in 3.1.4.5 source), so by the time it gets used
>>>> later on, I'm hitting the NULL dereference.
>>> Yeah, the code is obviously broken (also in v3.15-rc). It should
>>> probably work to just return from ftdi_set_max_packet_size if
>>> num_endpoints is 0 if you want to try that (or you can use your ?:
>>> construct), but I should be able to fix this up properly on Wednesday.
>>>
>>> Thanks,
>>> Johan
>> Johan,
>> I had a chance to play around with code over in ftdi_sio.c; adding this:
>>
>> if (!num_endpoints) {
>> return;
>> }
>> after the "Number of endpoints" message gets rid of the crash,
>> everything looks to be working correctly.
> Good. Care to try the patch below so I can add a formal Tested-by tag as
> well?
>
> Thanks,
> Johan
>
>
> >From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
> From: Johan Hovold <[email protected]>
> Date: Wed, 4 Jun 2014 14:09:43 +0200
> Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
>
> Fix NULL-pointer dereference when probing an interface with no
> endpoints.
>
> These devices have two bulk endpoints per interface, but this avoids
> crashing the kernel if a user forces a non-FTDI device to be probed.
>
> Note that the iterator variable was made unsigned in order to avoid
> a maybe-uninitialized compiler warning for ep_desc after the loop.
>
> Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
> calculation")
>
> Reported-by: Mike Remski <[email protected]>
> Cc: <[email protected]> # 2.3.61
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> drivers/usb/serial/ftdi_sio.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 7c6e1dedeb06..3019141397eb 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct usb_serial_port *port)
> struct usb_device *udev = serial->dev;
>
> struct usb_interface *interface = serial->interface;
> - struct usb_endpoint_descriptor *ep_desc = &interface->cur_altsetting->endpoint[1].desc;
> + struct usb_endpoint_descriptor *ep_desc;
>
> unsigned num_endpoints;
> - int i;
> + unsigned i;
>
> num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
> dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
>
> + if (!num_endpoints)
> + return;
> +
> /* NOTE: some customers have programmed FT232R/FT245R devices
> * with an endpoint size of 0 - not good. In this case, we
> * want to override the endpoint descriptor setting and use a

Thanks Johan. I tried to get the cdc_acm working; did not have much
luck/time (typical overcommit on workload) I will retry with the commit
mentioned.
I will try the patch today and get back to you. Nice on the ep_desc:
looking at the code priv->max_packet_size is attached to the port, your
change would use the last thing off of cur_altsetting->endpoint[], but
I'm wondering if we should actually be setting priv->max_packet_size to
whatever the max is of all endpoint[].desc->wMaxPacketSize?

Thoughts?

m

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-04 14:52:33

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
> On 06/04/2014 10:19 AM, Johan Hovold wrote:

> > >From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
> > From: Johan Hovold <[email protected]>
> > Date: Wed, 4 Jun 2014 14:09:43 +0200
> > Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
> >
> > Fix NULL-pointer dereference when probing an interface with no
> > endpoints.
> >
> > These devices have two bulk endpoints per interface, but this avoids
> > crashing the kernel if a user forces a non-FTDI device to be probed.
> >
> > Note that the iterator variable was made unsigned in order to avoid
> > a maybe-uninitialized compiler warning for ep_desc after the loop.
> >
> > Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
> > calculation")
> >
> > Reported-by: Mike Remski <[email protected]>
> > Cc: <[email protected]> # 2.3.61
> > Signed-off-by: Johan Hovold <[email protected]>
> > ---
> > drivers/usb/serial/ftdi_sio.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> > index 7c6e1dedeb06..3019141397eb 100644
> > --- a/drivers/usb/serial/ftdi_sio.c
> > +++ b/drivers/usb/serial/ftdi_sio.c
> > @@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct usb_serial_port *port)
> > struct usb_device *udev = serial->dev;
> >
> > struct usb_interface *interface = serial->interface;
> > - struct usb_endpoint_descriptor *ep_desc = &interface->cur_altsetting->endpoint[1].desc;
> > + struct usb_endpoint_descriptor *ep_desc;
> >
> > unsigned num_endpoints;
> > - int i;
> > + unsigned i;
> >
> > num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
> > dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
> >
> > + if (!num_endpoints)
> > + return;
> > +
> > /* NOTE: some customers have programmed FT232R/FT245R devices
> > * with an endpoint size of 0 - not good. In this case, we
> > * want to override the endpoint descriptor setting and use a
>
> Thanks Johan. I tried to get the cdc_acm working; did not have much
> luck/time (typical overcommit on workload) I will retry with the commit
> mentioned.
> I will try the patch today and get back to you. Nice on the ep_desc:
> looking at the code priv->max_packet_size is attached to the port, your
> change would use the last thing off of cur_altsetting->endpoint[], but
> I'm wondering if we should actually be setting priv->max_packet_size to
> whatever the max is of all endpoint[].desc->wMaxPacketSize?
>
> Thoughts?

This is the exact same behaviour as the old code (minus the NULL-deref).

These device have two bulk endpoints per interface and they are supposed
to be using the same max packet size (64 or 512 depending on device and
host).

This value is also used during depacketisation of incoming data (and
packetisation of outgoing data for legacy devices). I'm pretty convinced
you're using the wrong driver, something which would lead to corruption
of incoming data when the (non-existing) status bytes are stripped from
the stream.

You really should try cdc-acm.

Johan

2014-06-04 14:54:34

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/04/2014 10:52 AM, Johan Hovold wrote:
> On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
>> On 06/04/2014 10:19 AM, Johan Hovold wrote:
>>> >From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
>>> From: Johan Hovold <[email protected]>
>>> Date: Wed, 4 Jun 2014 14:09:43 +0200
>>> Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
>>>
>>> Fix NULL-pointer dereference when probing an interface with no
>>> endpoints.
>>>
>>> These devices have two bulk endpoints per interface, but this avoids
>>> crashing the kernel if a user forces a non-FTDI device to be probed.
>>>
>>> Note that the iterator variable was made unsigned in order to avoid
>>> a maybe-uninitialized compiler warning for ep_desc after the loop.
>>>
>>> Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
>>> calculation")
>>>
>>> Reported-by: Mike Remski <[email protected]>
>>> Cc: <[email protected]> # 2.3.61
>>> Signed-off-by: Johan Hovold <[email protected]>
>>> ---
>>> drivers/usb/serial/ftdi_sio.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>>> index 7c6e1dedeb06..3019141397eb 100644
>>> --- a/drivers/usb/serial/ftdi_sio.c
>>> +++ b/drivers/usb/serial/ftdi_sio.c
>>> @@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct usb_serial_port *port)
>>> struct usb_device *udev = serial->dev;
>>>
>>> struct usb_interface *interface = serial->interface;
>>> - struct usb_endpoint_descriptor *ep_desc = &interface->cur_altsetting->endpoint[1].desc;
>>> + struct usb_endpoint_descriptor *ep_desc;
>>>
>>> unsigned num_endpoints;
>>> - int i;
>>> + unsigned i;
>>>
>>> num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
>>> dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
>>>
>>> + if (!num_endpoints)
>>> + return;
>>> +
>>> /* NOTE: some customers have programmed FT232R/FT245R devices
>>> * with an endpoint size of 0 - not good. In this case, we
>>> * want to override the endpoint descriptor setting and use a
>> Thanks Johan. I tried to get the cdc_acm working; did not have much
>> luck/time (typical overcommit on workload) I will retry with the commit
>> mentioned.
>> I will try the patch today and get back to you. Nice on the ep_desc:
>> looking at the code priv->max_packet_size is attached to the port, your
>> change would use the last thing off of cur_altsetting->endpoint[], but
>> I'm wondering if we should actually be setting priv->max_packet_size to
>> whatever the max is of all endpoint[].desc->wMaxPacketSize?
>>
>> Thoughts?
> This is the exact same behaviour as the old code (minus the NULL-deref).
>
> These device have two bulk endpoints per interface and they are supposed
> to be using the same max packet size (64 or 512 depending on device and
> host).
>
> This value is also used during depacketisation of incoming data (and
> packetisation of outgoing data for legacy devices). I'm pretty convinced
> you're using the wrong driver, something which would lead to corruption
> of incoming data when the (non-existing) status bytes are stripped from
> the stream.
>
> You really should try cdc-acm.
>
> Johan

Thanks for the explaination. Yes, I am working on trying cdc-acm.

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-04 14:55:35

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/04/2014 10:52 AM, Johan Hovold wrote:
> On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
>> On 06/04/2014 10:19 AM, Johan Hovold wrote:
>>> >From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
>>> From: Johan Hovold <[email protected]>
>>> Date: Wed, 4 Jun 2014 14:09:43 +0200
>>> Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
>>>
>>> Fix NULL-pointer dereference when probing an interface with no
>>> endpoints.
>>>
>>> These devices have two bulk endpoints per interface, but this avoids
>>> crashing the kernel if a user forces a non-FTDI device to be probed.
>>>
>>> Note that the iterator variable was made unsigned in order to avoid
>>> a maybe-uninitialized compiler warning for ep_desc after the loop.
>>>
>>> Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
>>> calculation")
>>>
>>> Reported-by: Mike Remski <[email protected]>
>>> Cc: <[email protected]> # 2.3.61
>>> Signed-off-by: Johan Hovold <[email protected]>
>>> ---
>>> drivers/usb/serial/ftdi_sio.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>>> index 7c6e1dedeb06..3019141397eb 100644
>>> --- a/drivers/usb/serial/ftdi_sio.c
>>> +++ b/drivers/usb/serial/ftdi_sio.c
>>> @@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct usb_serial_port *port)
>>> struct usb_device *udev = serial->dev;
>>>
>>> struct usb_interface *interface = serial->interface;
>>> - struct usb_endpoint_descriptor *ep_desc = &interface->cur_altsetting->endpoint[1].desc;
>>> + struct usb_endpoint_descriptor *ep_desc;
>>>
>>> unsigned num_endpoints;
>>> - int i;
>>> + unsigned i;
>>>
>>> num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
>>> dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
>>>
>>> + if (!num_endpoints)
>>> + return;
>>> +
>>> /* NOTE: some customers have programmed FT232R/FT245R devices
>>> * with an endpoint size of 0 - not good. In this case, we
>>> * want to override the endpoint descriptor setting and use a
>> Thanks Johan. I tried to get the cdc_acm working; did not have much
>> luck/time (typical overcommit on workload) I will retry with the commit
>> mentioned.
>> I will try the patch today and get back to you. Nice on the ep_desc:
>> looking at the code priv->max_packet_size is attached to the port, your
>> change would use the last thing off of cur_altsetting->endpoint[], but
>> I'm wondering if we should actually be setting priv->max_packet_size to
>> whatever the max is of all endpoint[].desc->wMaxPacketSize?
>>
>> Thoughts?
> This is the exact same behaviour as the old code (minus the NULL-deref).
>
> These device have two bulk endpoints per interface and they are supposed
> to be using the same max packet size (64 or 512 depending on device and
> host).
>
> This value is also used during depacketisation of incoming data (and
> packetisation of outgoing data for legacy devices). I'm pretty convinced
> you're using the wrong driver, something which would lead to corruption
> of incoming data when the (non-existing) status bytes are stripped from
> the stream.
>
> You really should try cdc-acm.
>
> Johan
Sorry, forgot to add:
Tested patch and it works as desired.

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-04 15:10:14

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Wed, Jun 04, 2014 at 10:55:28AM -0400, Mike Remski wrote:
> On 06/04/2014 10:52 AM, Johan Hovold wrote:
> > On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
> >> On 06/04/2014 10:19 AM, Johan Hovold wrote:
> >>> >From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
> >>> From: Johan Hovold <[email protected]>
> >>> Date: Wed, 4 Jun 2014 14:09:43 +0200
> >>> Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
> >>>
> >>> Fix NULL-pointer dereference when probing an interface with no
> >>> endpoints.
> >>>
> >>> These devices have two bulk endpoints per interface, but this avoids
> >>> crashing the kernel if a user forces a non-FTDI device to be probed.
> >>>
> >>> Note that the iterator variable was made unsigned in order to avoid
> >>> a maybe-uninitialized compiler warning for ep_desc after the loop.
> >>>
> >>> Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
> >>> calculation")
> >>>
> >>> Reported-by: Mike Remski <[email protected]>
> >>> Cc: <[email protected]> # 2.3.61
> >>> Signed-off-by: Johan Hovold <[email protected]>
> >>> ---
> >>> drivers/usb/serial/ftdi_sio.c | 7 +++++--
> >>> 1 file changed, 5 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> >>> index 7c6e1dedeb06..3019141397eb 100644
> >>> --- a/drivers/usb/serial/ftdi_sio.c
> >>> +++ b/drivers/usb/serial/ftdi_sio.c
> >>> @@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct usb_serial_port *port)
> >>> struct usb_device *udev = serial->dev;
> >>>
> >>> struct usb_interface *interface = serial->interface;
> >>> - struct usb_endpoint_descriptor *ep_desc = &interface->cur_altsetting->endpoint[1].desc;
> >>> + struct usb_endpoint_descriptor *ep_desc;
> >>>
> >>> unsigned num_endpoints;
> >>> - int i;
> >>> + unsigned i;
> >>>
> >>> num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
> >>> dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
> >>>
> >>> + if (!num_endpoints)
> >>> + return;
> >>> +
> >>> /* NOTE: some customers have programmed FT232R/FT245R devices
> >>> * with an endpoint size of 0 - not good. In this case, we
> >>> * want to override the endpoint descriptor setting and use a
> >> Thanks Johan. I tried to get the cdc_acm working; did not have much
> >> luck/time (typical overcommit on workload) I will retry with the commit
> >> mentioned.
> >> I will try the patch today and get back to you. Nice on the ep_desc:
> >> looking at the code priv->max_packet_size is attached to the port, your
> >> change would use the last thing off of cur_altsetting->endpoint[], but
> >> I'm wondering if we should actually be setting priv->max_packet_size to
> >> whatever the max is of all endpoint[].desc->wMaxPacketSize?
> >>
> >> Thoughts?
> > This is the exact same behaviour as the old code (minus the NULL-deref).
> >
> > These device have two bulk endpoints per interface and they are supposed
> > to be using the same max packet size (64 or 512 depending on device and
> > host).
> >
> > This value is also used during depacketisation of incoming data (and
> > packetisation of outgoing data for legacy devices). I'm pretty convinced
> > you're using the wrong driver, something which would lead to corruption
> > of incoming data when the (non-existing) status bytes are stripped from
> > the stream.
> >
> > You really should try cdc-acm.
> >
> > Johan
> Sorry, forgot to add:
> Tested patch and it works as desired.

Thanks, I'll add a Tested-by tag then.

Johan

2014-06-04 15:12:36

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/04/2014 11:09 AM, Johan Hovold wrote:
> On Wed, Jun 04, 2014 at 10:55:28AM -0400, Mike Remski wrote:
>> On 06/04/2014 10:52 AM, Johan Hovold wrote:
>>> On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
>>>> On 06/04/2014 10:19 AM, Johan Hovold wrote:
>>>>> >From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
>>>>> From: Johan Hovold <[email protected]>
>>>>> Date: Wed, 4 Jun 2014 14:09:43 +0200
>>>>> Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
>>>>>
>>>>> Fix NULL-pointer dereference when probing an interface with no
>>>>> endpoints.
>>>>>
>>>>> These devices have two bulk endpoints per interface, but this avoids
>>>>> crashing the kernel if a user forces a non-FTDI device to be probed.
>>>>>
>>>>> Note that the iterator variable was made unsigned in order to avoid
>>>>> a maybe-uninitialized compiler warning for ep_desc after the loop.
>>>>>
>>>>> Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
>>>>> calculation")
>>>>>
>>>>> Reported-by: Mike Remski <[email protected]>
>>>>> Cc: <[email protected]> # 2.3.61
>>>>> Signed-off-by: Johan Hovold <[email protected]>
>>>>> ---
>>>>> drivers/usb/serial/ftdi_sio.c | 7 +++++--
>>>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>>>>> index 7c6e1dedeb06..3019141397eb 100644
>>>>> --- a/drivers/usb/serial/ftdi_sio.c
>>>>> +++ b/drivers/usb/serial/ftdi_sio.c
>>>>> @@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct usb_serial_port *port)
>>>>> struct usb_device *udev = serial->dev;
>>>>>
>>>>> struct usb_interface *interface = serial->interface;
>>>>> - struct usb_endpoint_descriptor *ep_desc = &interface->cur_altsetting->endpoint[1].desc;
>>>>> + struct usb_endpoint_descriptor *ep_desc;
>>>>>
>>>>> unsigned num_endpoints;
>>>>> - int i;
>>>>> + unsigned i;
>>>>>
>>>>> num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
>>>>> dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
>>>>>
>>>>> + if (!num_endpoints)
>>>>> + return;
>>>>> +
>>>>> /* NOTE: some customers have programmed FT232R/FT245R devices
>>>>> * with an endpoint size of 0 - not good. In this case, we
>>>>> * want to override the endpoint descriptor setting and use a
>>>> Thanks Johan. I tried to get the cdc_acm working; did not have much
>>>> luck/time (typical overcommit on workload) I will retry with the commit
>>>> mentioned.
>>>> I will try the patch today and get back to you. Nice on the ep_desc:
>>>> looking at the code priv->max_packet_size is attached to the port, your
>>>> change would use the last thing off of cur_altsetting->endpoint[], but
>>>> I'm wondering if we should actually be setting priv->max_packet_size to
>>>> whatever the max is of all endpoint[].desc->wMaxPacketSize?
>>>>
>>>> Thoughts?
>>> This is the exact same behaviour as the old code (minus the NULL-deref).
>>>
>>> These device have two bulk endpoints per interface and they are supposed
>>> to be using the same max packet size (64 or 512 depending on device and
>>> host).
>>>
>>> This value is also used during depacketisation of incoming data (and
>>> packetisation of outgoing data for legacy devices). I'm pretty convinced
>>> you're using the wrong driver, something which would lead to corruption
>>> of incoming data when the (non-existing) status bytes are stripped from
>>> the stream.
>>>
>>> You really should try cdc-acm.
>>>
>>> Johan
>> Sorry, forgot to add:
>> Tested patch and it works as desired.
> Thanks, I'll add a Tested-by tag then.
>
> Johan
Thanks for quick response. I'll let you know the results of cdc-acm
(had to pull in the commit you mentioned).

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-04 15:41:51

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/04/2014 11:09 AM, Johan Hovold wrote:
> On Wed, Jun 04, 2014 at 10:55:28AM -0400, Mike Remski wrote:
>> On 06/04/2014 10:52 AM, Johan Hovold wrote:
>>> On Wed, Jun 04, 2014 at 10:29:37AM -0400, Mike Remski wrote:
>>>> On 06/04/2014 10:19 AM, Johan Hovold wrote:
>>>>> >From 4ddea3a573b8c15beefb67bc35c440850063d79d Mon Sep 17 00:00:00 2001
>>>>> From: Johan Hovold <[email protected]>
>>>>> Date: Wed, 4 Jun 2014 14:09:43 +0200
>>>>> Subject: [PATCH 1/5] USB: ftdi_sio: fix null deref at port probe
>>>>>
>>>>> Fix NULL-pointer dereference when probing an interface with no
>>>>> endpoints.
>>>>>
>>>>> These devices have two bulk endpoints per interface, but this avoids
>>>>> crashing the kernel if a user forces a non-FTDI device to be probed.
>>>>>
>>>>> Note that the iterator variable was made unsigned in order to avoid
>>>>> a maybe-uninitialized compiler warning for ep_desc after the loop.
>>>>>
>>>>> Fixes: 895f28badce9 ("USB: ftdi_sio: fix hi-speed device packet size
>>>>> calculation")
>>>>>
>>>>> Reported-by: Mike Remski <[email protected]>
>>>>> Cc: <[email protected]> # 2.3.61
>>>>> Signed-off-by: Johan Hovold <[email protected]>
>>>>> ---
>>>>> drivers/usb/serial/ftdi_sio.c | 7 +++++--
>>>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
>>>>> index 7c6e1dedeb06..3019141397eb 100644
>>>>> --- a/drivers/usb/serial/ftdi_sio.c
>>>>> +++ b/drivers/usb/serial/ftdi_sio.c
>>>>> @@ -1564,14 +1564,17 @@ static void ftdi_set_max_packet_size(struct usb_serial_port *port)
>>>>> struct usb_device *udev = serial->dev;
>>>>>
>>>>> struct usb_interface *interface = serial->interface;
>>>>> - struct usb_endpoint_descriptor *ep_desc = &interface->cur_altsetting->endpoint[1].desc;
>>>>> + struct usb_endpoint_descriptor *ep_desc;
>>>>>
>>>>> unsigned num_endpoints;
>>>>> - int i;
>>>>> + unsigned i;
>>>>>
>>>>> num_endpoints = interface->cur_altsetting->desc.bNumEndpoints;
>>>>> dev_info(&udev->dev, "Number of endpoints %d\n", num_endpoints);
>>>>>
>>>>> + if (!num_endpoints)
>>>>> + return;
>>>>> +
>>>>> /* NOTE: some customers have programmed FT232R/FT245R devices
>>>>> * with an endpoint size of 0 - not good. In this case, we
>>>>> * want to override the endpoint descriptor setting and use a
>>>> Thanks Johan. I tried to get the cdc_acm working; did not have much
>>>> luck/time (typical overcommit on workload) I will retry with the commit
>>>> mentioned.
>>>> I will try the patch today and get back to you. Nice on the ep_desc:
>>>> looking at the code priv->max_packet_size is attached to the port, your
>>>> change would use the last thing off of cur_altsetting->endpoint[], but
>>>> I'm wondering if we should actually be setting priv->max_packet_size to
>>>> whatever the max is of all endpoint[].desc->wMaxPacketSize?
>>>>
>>>> Thoughts?
>>> This is the exact same behaviour as the old code (minus the NULL-deref).
>>>
>>> These device have two bulk endpoints per interface and they are supposed
>>> to be using the same max packet size (64 or 512 depending on device and
>>> host).
>>>
>>> This value is also used during depacketisation of incoming data (and
>>> packetisation of outgoing data for legacy devices). I'm pretty convinced
>>> you're using the wrong driver, something which would lead to corruption
>>> of incoming data when the (non-existing) status bytes are stripped from
>>> the stream.
>>>
>>> You really should try cdc-acm.
>>>
>>> Johan
>> Sorry, forgot to add:
>> Tested patch and it works as desired.
> Thanks, I'll add a Tested-by tag then.
>
> Johan

Ok, had a chance to try the cdc-acm; yes it's the ancient one that is
in the base install of CentOS 6.4 (2.6.32-71.29.1 centos patched). I'm
hitting something similar; I noticed there where a few commits for NULL
pointer dereferences in cdc-acm.c.

I'll poke into this a bit (yes, I know try a newer kernel, etc) and let
you know what I find (or perhaps this looks similar to something already
seen?). This device is the same as the original ftdi crash, 1.2 is the
same interface with bNumEndpoints == 0.

Jun 4 11:35:20 nicA91A84 kernel: cdc_acm 5-2:1.2: This device cannot do
calls on its own. It is not a modem.
Jun 4 11:35:20 nicA91A84 kernel: BUG: unable to handle kernel NULL
pointer dereference at 00000004
Jun 4 11:35:20 nicA91A84 kernel: IP: [<f84f9c9e>]
acm_probe+0x44e/0x127c [cdc_acm]
Jun 4 11:35:20 nicA91A84 kernel: *pdpt = 00000000354eb001 *pde =
000000007f003067
Jun 4 11:35:20 nicA91A84 kernel: Oops: 0000 [#1] SMP
Jun 4 11:35:20 nicA91A84 kernel: last sysfs file:
/sys/devices/pci0000:00/0000:00:1d.0/usb5/5-2/product


--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-04 16:00:33

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Wed, Jun 04, 2014 at 11:41:47AM -0400, Mike Remski wrote:
> Ok, had a chance to try the cdc-acm; yes it's the ancient one that is
> in the base install of CentOS 6.4 (2.6.32-71.29.1 centos patched). I'm
> hitting something similar; I noticed there where a few commits for NULL
> pointer dereferences in cdc-acm.c.
>
> I'll poke into this a bit (yes, I know try a newer kernel, etc) and let
> you know what I find (or perhaps this looks similar to something already
> seen?). This device is the same as the original ftdi crash, 1.2 is the
> same interface with bNumEndpoints == 0.
>
> Jun 4 11:35:20 nicA91A84 kernel: cdc_acm 5-2:1.2: This device cannot do
> calls on its own. It is not a modem.
> Jun 4 11:35:20 nicA91A84 kernel: BUG: unable to handle kernel NULL
> pointer dereference at 00000004
> Jun 4 11:35:20 nicA91A84 kernel: IP: [<f84f9c9e>]
> acm_probe+0x44e/0x127c [cdc_acm]
> Jun 4 11:35:20 nicA91A84 kernel: *pdpt = 00000000354eb001 *pde =
> 000000007f003067
> Jun 4 11:35:20 nicA91A84 kernel: Oops: 0000 [#1] SMP
> Jun 4 11:35:20 nicA91A84 kernel: last sysfs file:
> /sys/devices/pci0000:00/0000:00:1d.0/usb5/5-2/product

Yeah, you really need to update your kernel. You'll definitely need at
least commit 99f347caa456 ("USB: CDC ACM: Fix NULL pointer
dereference").

No more reports until you've tried a recent kernel, ok? ;)

Johan

2014-06-04 16:13:10

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/04/2014 12:00 PM, Johan Hovold wrote:
> On Wed, Jun 04, 2014 at 11:41:47AM -0400, Mike Remski wrote:
>> Ok, had a chance to try the cdc-acm; yes it's the ancient one that is
>> in the base install of CentOS 6.4 (2.6.32-71.29.1 centos patched). I'm
>> hitting something similar; I noticed there where a few commits for NULL
>> pointer dereferences in cdc-acm.c.
>>
>> I'll poke into this a bit (yes, I know try a newer kernel, etc) and let
>> you know what I find (or perhaps this looks similar to something already
>> seen?). This device is the same as the original ftdi crash, 1.2 is the
>> same interface with bNumEndpoints == 0.
>>
>> Jun 4 11:35:20 nicA91A84 kernel: cdc_acm 5-2:1.2: This device cannot do
>> calls on its own. It is not a modem.
>> Jun 4 11:35:20 nicA91A84 kernel: BUG: unable to handle kernel NULL
>> pointer dereference at 00000004
>> Jun 4 11:35:20 nicA91A84 kernel: IP: [<f84f9c9e>]
>> acm_probe+0x44e/0x127c [cdc_acm]
>> Jun 4 11:35:20 nicA91A84 kernel: *pdpt = 00000000354eb001 *pde =
>> 000000007f003067
>> Jun 4 11:35:20 nicA91A84 kernel: Oops: 0000 [#1] SMP
>> Jun 4 11:35:20 nicA91A84 kernel: last sysfs file:
>> /sys/devices/pci0000:00/0000:00:1d.0/usb5/5-2/product
> Yeah, you really need to update your kernel. You'll definitely need at
> least commit 99f347caa456 ("USB: CDC ACM: Fix NULL pointer
> dereference").
>
> No more reports until you've tried a recent kernel, ok? ;)
>
> Johan
Agreed. But I still have to understand it so I can backport the
appropriate fixes, no?
I'll be quiet now.

--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-04 17:05:09

by Mike Remski

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On 06/04/2014 12:00 PM, Johan Hovold wrote:
> On Wed, Jun 04, 2014 at 11:41:47AM -0400, Mike Remski wrote:
>> Ok, had a chance to try the cdc-acm; yes it's the ancient one that is
>> in the base install of CentOS 6.4 (2.6.32-71.29.1 centos patched). I'm
>> hitting something similar; I noticed there where a few commits for NULL
>> pointer dereferences in cdc-acm.c.
>>
>> I'll poke into this a bit (yes, I know try a newer kernel, etc) and let
>> you know what I find (or perhaps this looks similar to something already
>> seen?). This device is the same as the original ftdi crash, 1.2 is the
>> same interface with bNumEndpoints == 0.
>>
>> Jun 4 11:35:20 nicA91A84 kernel: cdc_acm 5-2:1.2: This device cannot do
>> calls on its own. It is not a modem.
>> Jun 4 11:35:20 nicA91A84 kernel: BUG: unable to handle kernel NULL
>> pointer dereference at 00000004
>> Jun 4 11:35:20 nicA91A84 kernel: IP: [<f84f9c9e>]
>> acm_probe+0x44e/0x127c [cdc_acm]
>> Jun 4 11:35:20 nicA91A84 kernel: *pdpt = 00000000354eb001 *pde =
>> 000000007f003067
>> Jun 4 11:35:20 nicA91A84 kernel: Oops: 0000 [#1] SMP
>> Jun 4 11:35:20 nicA91A84 kernel: last sysfs file:
>> /sys/devices/pci0000:00/0000:00:1d.0/usb5/5-2/product
> Yeah, you really need to update your kernel. You'll definitely need at
> least commit 99f347caa456 ("USB: CDC ACM: Fix NULL pointer
> dereference").
>
> No more reports until you've tried a recent kernel, ok? ;)
>
> Johan
Device in question does not cause kernel crash when using the cdc-acm
driver on a Fedora 19 system running Fedora kernel 3.9.5-301.

Thanks for all the help, I'll take care of my ancient kernels.



--
Office: (978)401-4032 (x123 internally)
Cell: (603) 759-6953

2014-06-05 07:10:59

by Johan Hovold

[permalink] [raw]
Subject: Re: ftdi_sio BUG: NULL pointer dereference

On Wed, Jun 04, 2014 at 01:05:03PM -0400, Mike Remski wrote:
> On 06/04/2014 12:00 PM, Johan Hovold wrote:
> > On Wed, Jun 04, 2014 at 11:41:47AM -0400, Mike Remski wrote:
> >> Ok, had a chance to try the cdc-acm; yes it's the ancient one that is
> >> in the base install of CentOS 6.4 (2.6.32-71.29.1 centos patched). I'm
> >> hitting something similar; I noticed there where a few commits for NULL
> >> pointer dereferences in cdc-acm.c.
> >>
> >> I'll poke into this a bit (yes, I know try a newer kernel, etc) and let
> >> you know what I find (or perhaps this looks similar to something already
> >> seen?). This device is the same as the original ftdi crash, 1.2 is the
> >> same interface with bNumEndpoints == 0.
> >>
> >> Jun 4 11:35:20 nicA91A84 kernel: cdc_acm 5-2:1.2: This device cannot do
> >> calls on its own. It is not a modem.
> >> Jun 4 11:35:20 nicA91A84 kernel: BUG: unable to handle kernel NULL
> >> pointer dereference at 00000004
> >> Jun 4 11:35:20 nicA91A84 kernel: IP: [<f84f9c9e>]
> >> acm_probe+0x44e/0x127c [cdc_acm]
> >> Jun 4 11:35:20 nicA91A84 kernel: *pdpt = 00000000354eb001 *pde =
> >> 000000007f003067
> >> Jun 4 11:35:20 nicA91A84 kernel: Oops: 0000 [#1] SMP
> >> Jun 4 11:35:20 nicA91A84 kernel: last sysfs file:
> >> /sys/devices/pci0000:00/0000:00:1d.0/usb5/5-2/product
> > Yeah, you really need to update your kernel. You'll definitely need at
> > least commit 99f347caa456 ("USB: CDC ACM: Fix NULL pointer
> > dereference").
> >
> > No more reports until you've tried a recent kernel, ok? ;)
> >
> > Johan
> Device in question does not cause kernel crash when using the cdc-acm
> driver on a Fedora 19 system running Fedora kernel 3.9.5-301.
>
> Thanks for all the help, I'll take care of my ancient kernels.

That's good to hear. Good luck with the backporting.

Johan