linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver
@ 2016-03-15 23:42 Xose Vazquez Perez
  2016-03-16 13:58 ` Jes Sorensen
  0 siblings, 1 reply; 8+ messages in thread
From: Xose Vazquez Perez @ 2016-03-15 23:42 UTC (permalink / raw)
  To: Jes Sorensen, linux-wireless

Hi,

If I do:
# echo "0bda 8176" > /sys/bus/usb/drivers/rtl8xxxu/new_id

I get:
---dmesg---
usbcore: registered new interface driver rtl8xxxu
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffffa054bbf0>] rtl8xxxu_probe+0x810/0x22a0 [rtl8xxxu]
PGD 0
Oops: 0000 [#1] SMP
Modules linked in: rtl8xxxu ccm snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_pcm_oss snd_mixer_oss arc4 intel_rapl iosf_mbi x86_pkg_temp_thermal coretemp kvm_intel kvm ath9k ath9k_common ath9k_hw
irqbypass ath crct10dif_pclmul i915 crc32_pclmul crc32c_intel iTCO_wdt iTCO_vendor_support mac80211 snd_hda_codec_realtek cfg80211 snd_hda_codec_generic snd_usb_audio i2c_i801 snd_hda_intel
snd_hda_codec snd_hda_core snd_usbmidi_lib snd_rawmidi snd_hwdep snd_seq ses video i2c_algo_bit snd_seq_device drm_kms_helper enclosure lpc_ich drm snd_pcm rfkill snd_timer mei_me snd mei soundcore
shpchp tpm_tis tpm binfmt_misc hid_logitech_hidpp hid_logitech_dj serio_raw r8169 mii fjes uas usb_storage [last unloaded: rtlwifi]
CPU: 0 PID: 1233 Comm: bash Not tainted 4.4.5-300.fc23.x86_64 #1
Hardware name: Hewlett-Packard p6-2004es/2ABF, BIOS 7.16 03/23/2012
task: ffff88022d340000 ti: ffff8800b5c10000 task.ti: ffff8800b5c10000
RIP: 0010:[<ffffffffa054bbf0>] [<ffffffffa054bbf0>] rtl8xxxu_probe+0x810/0x22a0 [rtl8xxxu]
RSP: 0018:ffff8800b5c13bc0 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 000000000000006a RCX: 0000000000005a32
RDX: 0000000000005a31 RSI: 0000000000000000 RDI: ffff8802346bb4c0
RBP: ffff8800b5c13c70 R08: 000000000001a1e0 R09: ffffffff81586bbb
R10: ffffea0002d65380 R11: 0000000000000000 R12: 00000000000000bf
R13: 00000000000000ff R14: ffff8802346bb4c0 R15: 00000000000000be
FS: 00007fe567227700(0000) GS:ffff88023f400000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000000b5d9a000 CR4: 00000000000406f0
Stack:
ffff88022d340000 ffff8800b5c13be8 ffffffff812abf5a 00000000ffffffea
0000000000000000 ffff8800b5c13c18 0000000084b2960b ffff880232cfa090
ffff8802346ba700 ffff880233631400 ffff8802346bb53e 0000006300000246
Call Trace:
[<ffffffff812abf5a>] ? kernfs_activate+0x7a/0xe0
[<ffffffff8158b03d>] usb_probe_interface+0x1bd/0x300
[<ffffffff814ef252>] driver_probe_device+0x222/0x490
[<ffffffff814ef544>] __driver_attach+0x84/0x90
[<ffffffff814ef4c0>] ? driver_probe_device+0x490/0x490
[<ffffffff814ecd2c>] bus_for_each_dev+0x6c/0xc0
[<ffffffff814eea0e>] driver_attach+0x1e/0x20
[<ffffffff8158a33b>] usb_store_new_id+0xeb/0x1c0
[<ffffffff8158a432>] new_id_store+0x22/0x30
[<ffffffff814eca85>] drv_attr_store+0x25/0x30
[<ffffffff812ada77>] sysfs_kf_write+0x37/0x40
[<ffffffff812ad03d>] kernfs_fop_write+0x11d/0x170
[<ffffffff8122dbb7>] __vfs_write+0x37/0x110
[<ffffffff8124b3b3>] ? __fd_install+0x33/0xe0
[<ffffffff810ec982>] ? percpu_down_read+0x12/0x50
[<ffffffff8122e2c9>] vfs_write+0xa9/0x1a0
[<ffffffff8122ef85>] SyS_write+0x55/0xc0
[<ffffffff817a052e>] entry_SYSCALL_64_fastpath+0x12/0x71
Code: 44 89 f0 4d 89 fe 41 89 c7 66 41 81 ff ff 01 0f 86 6f ff ff ff 31 d2 be cf 00 00 00 4c 89 f7 e8 07 a5 ff ff 49 8b 46 10 4c 89 f7 <ff> 10 85 c0 41 89 c0 0f 85 89 03 00 00 49 8b 46 08 48 c7 c1 5b
RIP [<ffffffffa054bbf0>] rtl8xxxu_probe+0x810/0x22a0 [rtl8xxxu]
RSP <ffff8800b5c13bc0>
CR2: 0000000000000000
---[ end trace 0675ed7e0a2d84ed ]---

--rtl8192cu dmesg--
rtl8192cu: Chip version 0x10
rtl8192cu: MAC address: 00:e0:4c:07:dd:45
rtl8192cu: Board Type 0
rtl_usb: rx_max_size 15360, rx_urb_num 8, in_ep 1
rtl8192cu: Loading firmware rtlwifi/rtl8192cufw_TMSC.bin
ieee80211 phy1: Selected rate control algorithm 'rtl_rc'
usbcore: registered new interface driver rtl8192cu
--end--

--lsusb-v--
Bus 002 Device 004: ID 0bda:8176 Realtek Semiconductor Corp. RTL8188CUS 802.11n WLAN Adapter
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  idVendor           0x0bda Realtek Semiconductor Corp.
  idProduct          0x8176 RTL8188CUS 802.11n WLAN Adapter
  bcdDevice            2.00
  iManufacturer           1 Realtek
  iProduct                2 802.11n WLAN Adapter
  iSerial                 3 00e04c000001
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           46
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           4
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x84  EP 4 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
Device Qualifier (for other device speed):
  bLength                10
  bDescriptorType         6
  bcdUSB               2.00
  bDeviceClass            0
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  bNumConfigurations      1
Device Status:     0x0000
  (Bus Powered)
--end--

Thanks.

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

* Re: rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver
  2016-03-15 23:42 rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver Xose Vazquez Perez
@ 2016-03-16 13:58 ` Jes Sorensen
  2016-03-17 18:02   ` Jes Sorensen
  0 siblings, 1 reply; 8+ messages in thread
From: Jes Sorensen @ 2016-03-16 13:58 UTC (permalink / raw)
  To: Xose Vazquez Perez; +Cc: linux-wireless

Xose Vazquez Perez <xose.vazquez@gmail.com> writes:
> Hi,
>
> If I do:
> # echo "0bda 8176" > /sys/bus/usb/drivers/rtl8xxxu/new_id

Hi Xose,

Yes please don't do that. The rtl8xxxu driver relies on the .driver_info
field in struct use_device_id to carry information for the different
types of devices. If you hot add a device like above, the driver will
fail because that field now contains a NULL pointer.

I should probably add a check for it in the probe function, but it will
simply be there to spit out a warning that it doesn't work to hot add a
device like this.

If you build it with CONFIG_RTL8XXXU_UNTESTED the 0bda:8176 should be
included in the device list.

Cheers,
Jes


>
> I get:
> ---dmesg---
> usbcore: registered new interface driver rtl8xxxu
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffffa054bbf0>] rtl8xxxu_probe+0x810/0x22a0 [rtl8xxxu]
> PGD 0
> Oops: 0000 [#1] SMP
> Modules linked in: rtl8xxxu ccm snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_pcm_oss snd_mixer_oss arc4 intel_rapl iosf_mbi x86_pkg_temp_thermal coretemp kvm_intel kvm ath9k ath9k_common ath9k_hw
> irqbypass ath crct10dif_pclmul i915 crc32_pclmul crc32c_intel iTCO_wdt iTCO_vendor_support mac80211 snd_hda_codec_realtek cfg80211 snd_hda_codec_generic snd_usb_audio i2c_i801 snd_hda_intel
> snd_hda_codec snd_hda_core snd_usbmidi_lib snd_rawmidi snd_hwdep snd_seq ses video i2c_algo_bit snd_seq_device drm_kms_helper enclosure lpc_ich drm snd_pcm rfkill snd_timer mei_me snd mei soundcore
> shpchp tpm_tis tpm binfmt_misc hid_logitech_hidpp hid_logitech_dj serio_raw r8169 mii fjes uas usb_storage [last unloaded: rtlwifi]
> CPU: 0 PID: 1233 Comm: bash Not tainted 4.4.5-300.fc23.x86_64 #1
> Hardware name: Hewlett-Packard p6-2004es/2ABF, BIOS 7.16 03/23/2012
> task: ffff88022d340000 ti: ffff8800b5c10000 task.ti: ffff8800b5c10000
> RIP: 0010:[<ffffffffa054bbf0>] [<ffffffffa054bbf0>] rtl8xxxu_probe+0x810/0x22a0 [rtl8xxxu]
> RSP: 0018:ffff8800b5c13bc0 EFLAGS: 00010286
> RAX: 0000000000000000 RBX: 000000000000006a RCX: 0000000000005a32
> RDX: 0000000000005a31 RSI: 0000000000000000 RDI: ffff8802346bb4c0
> RBP: ffff8800b5c13c70 R08: 000000000001a1e0 R09: ffffffff81586bbb
> R10: ffffea0002d65380 R11: 0000000000000000 R12: 00000000000000bf
> R13: 00000000000000ff R14: ffff8802346bb4c0 R15: 00000000000000be
> FS: 00007fe567227700(0000) GS:ffff88023f400000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 00000000b5d9a000 CR4: 00000000000406f0
> Stack:
> ffff88022d340000 ffff8800b5c13be8 ffffffff812abf5a 00000000ffffffea
> 0000000000000000 ffff8800b5c13c18 0000000084b2960b ffff880232cfa090
> ffff8802346ba700 ffff880233631400 ffff8802346bb53e 0000006300000246
> Call Trace:
> [<ffffffff812abf5a>] ? kernfs_activate+0x7a/0xe0
> [<ffffffff8158b03d>] usb_probe_interface+0x1bd/0x300
> [<ffffffff814ef252>] driver_probe_device+0x222/0x490
> [<ffffffff814ef544>] __driver_attach+0x84/0x90
> [<ffffffff814ef4c0>] ? driver_probe_device+0x490/0x490
> [<ffffffff814ecd2c>] bus_for_each_dev+0x6c/0xc0
> [<ffffffff814eea0e>] driver_attach+0x1e/0x20
> [<ffffffff8158a33b>] usb_store_new_id+0xeb/0x1c0
> [<ffffffff8158a432>] new_id_store+0x22/0x30
> [<ffffffff814eca85>] drv_attr_store+0x25/0x30
> [<ffffffff812ada77>] sysfs_kf_write+0x37/0x40
> [<ffffffff812ad03d>] kernfs_fop_write+0x11d/0x170
> [<ffffffff8122dbb7>] __vfs_write+0x37/0x110
> [<ffffffff8124b3b3>] ? __fd_install+0x33/0xe0
> [<ffffffff810ec982>] ? percpu_down_read+0x12/0x50
> [<ffffffff8122e2c9>] vfs_write+0xa9/0x1a0
> [<ffffffff8122ef85>] SyS_write+0x55/0xc0
> [<ffffffff817a052e>] entry_SYSCALL_64_fastpath+0x12/0x71
> Code: 44 89 f0 4d 89 fe 41 89 c7 66 41 81 ff ff 01 0f 86 6f ff ff ff 31 d2 be cf 00 00 00 4c 89 f7 e8 07 a5 ff ff 49 8b 46 10 4c 89 f7 <ff> 10 85 c0 41 89 c0 0f 85 89 03 00 00 49 8b 46 08 48 c7 c1 5b
> RIP [<ffffffffa054bbf0>] rtl8xxxu_probe+0x810/0x22a0 [rtl8xxxu]
> RSP <ffff8800b5c13bc0>
> CR2: 0000000000000000
> ---[ end trace 0675ed7e0a2d84ed ]---
>
> --rtl8192cu dmesg--
> rtl8192cu: Chip version 0x10
> rtl8192cu: MAC address: 00:e0:4c:07:dd:45
> rtl8192cu: Board Type 0
> rtl_usb: rx_max_size 15360, rx_urb_num 8, in_ep 1
> rtl8192cu: Loading firmware rtlwifi/rtl8192cufw_TMSC.bin
> ieee80211 phy1: Selected rate control algorithm 'rtl_rc'
> usbcore: registered new interface driver rtl8192cu
> --end--
>
> --lsusb-v--
> Bus 002 Device 004: ID 0bda:8176 Realtek Semiconductor Corp. RTL8188CUS 802.11n WLAN Adapter
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               2.00
>   bDeviceClass            0
>   bDeviceSubClass         0
>   bDeviceProtocol         0
>   bMaxPacketSize0        64
>   idVendor           0x0bda Realtek Semiconductor Corp.
>   idProduct          0x8176 RTL8188CUS 802.11n WLAN Adapter
>   bcdDevice            2.00
>   iManufacturer           1 Realtek
>   iProduct                2 802.11n WLAN Adapter
>   iSerial                 3 00e04c000001
>   bNumConfigurations      1
>   Configuration Descriptor:
>     bLength                 9
>     bDescriptorType         2
>     wTotalLength           46
>     bNumInterfaces          1
>     bConfigurationValue     1
>     iConfiguration          0
>     bmAttributes         0x80
>       (Bus Powered)
>     MaxPower              500mA
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        0
>       bAlternateSetting       0
>       bNumEndpoints           4
>       bInterfaceClass       255 Vendor Specific Class
>       bInterfaceSubClass    255 Vendor Specific Subclass
>       bInterfaceProtocol    255 Vendor Specific Protocol
>       iInterface              0
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x81  EP 1 IN
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval               0
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x02  EP 2 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval               0
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x03  EP 3 OUT
>         bmAttributes            2
>           Transfer Type            Bulk
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0200  1x 512 bytes
>         bInterval               0
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x84  EP 4 IN
>         bmAttributes            3
>           Transfer Type            Interrupt
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0040  1x 64 bytes
>         bInterval               1
> Device Qualifier (for other device speed):
>   bLength                10
>   bDescriptorType         6
>   bcdUSB               2.00
>   bDeviceClass            0
>   bDeviceSubClass         0
>   bDeviceProtocol         0
>   bMaxPacketSize0        64
>   bNumConfigurations      1
> Device Status:     0x0000
>   (Bus Powered)
> --end--
>
> Thanks.

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

* Re: rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver
  2016-03-16 13:58 ` Jes Sorensen
@ 2016-03-17 18:02   ` Jes Sorensen
  2016-03-17 20:01     ` poma
  0 siblings, 1 reply; 8+ messages in thread
From: Jes Sorensen @ 2016-03-17 18:02 UTC (permalink / raw)
  To: Xose Vazquez Perez; +Cc: linux-wireless

Jes Sorensen <Jes.Sorensen@redhat.com> writes:
> Xose Vazquez Perez <xose.vazquez@gmail.com> writes:
>> Hi,
>>
>> If I do:
>> # echo "0bda 8176" > /sys/bus/usb/drivers/rtl8xxxu/new_id
>
> Hi Xose,
>
> Yes please don't do that. The rtl8xxxu driver relies on the .driver_info
> field in struct use_device_id to carry information for the different
> types of devices. If you hot add a device like above, the driver will
> fail because that field now contains a NULL pointer.
>
> I should probably add a check for it in the probe function, but it will
> simply be there to spit out a warning that it doesn't work to hot add a
> device like this.
>
> If you build it with CONFIG_RTL8XXXU_UNTESTED the 0bda:8176 should be
> included in the device list.
>
> Cheers,
> Jes

Hi Xose,

I added the following patch to my tree to avoid this.

Cheers,
Jes

commit 9202f4947aac1d60084ee79c9b5294eb42ba59dc
Author: Jes Sorensen <Jes.Sorensen@redhat.com>
Date:   Thu Mar 17 13:53:48 2016 -0400

    rtl8xxxu: Fix OOPS if user tries to add device via /sys
    
    This driver relies on driver_info in struct usb_device_id, hence
    adding a device via  /sys/bus/usb/drivers/rtl8xxxu/new_id would result
    in a NULL pointer dereference.
    
    Instead print a message and return -ENODEV
    
    Reported-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
    Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
index 8d893f4..55fc00e 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
@@ -9671,6 +9671,15 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
 
 	udev = usb_get_dev(interface_to_usbdev(interface));
 
+	if (!id->driver_info) {
+		dev_warn(&udev->dev,
+			 "rtl8xxxu relies on driver_info in struct usb_device_id.\n");
+		dev_warn(&udev->dev,
+			 "Adding a device via /sys/bus/usb/drivers/rtl8xxxu/new_id is not supported!\n");
+		ret = -ENODEV;
+		goto exit;
+	}
+
 	switch (id->idVendor) {
 	case USB_VENDOR_ID_REALTEK:
 		switch(id->idProduct) {

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

* Re: rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver
  2016-03-17 18:02   ` Jes Sorensen
@ 2016-03-17 20:01     ` poma
  2016-03-17 20:11       ` Jes Sorensen
                         ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: poma @ 2016-03-17 20:01 UTC (permalink / raw)
  To: Jes Sorensen, Xose Vazquez Perez
  Cc: linux-wireless, linux-usb@vger.kernel.org

On 17.03.2016 19:02, Jes Sorensen wrote:
> Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>> Xose Vazquez Perez <xose.vazquez@gmail.com> writes:
>>> Hi,
>>>
>>> If I do:
>>> # echo "0bda 8176" > /sys/bus/usb/drivers/rtl8xxxu/new_id
>>
>> Hi Xose,
>>
>> Yes please don't do that. The rtl8xxxu driver relies on the .driver_info
>> field in struct use_device_id to carry information for the different
>> types of devices. If you hot add a device like above, the driver will
>> fail because that field now contains a NULL pointer.
>>
>> I should probably add a check for it in the probe function, but it will
>> simply be there to spit out a warning that it doesn't work to hot add a
>> device like this.
>>
>> If you build it with CONFIG_RTL8XXXU_UNTESTED the 0bda:8176 should be
>> included in the device list.
>>
>> Cheers,
>> Jes
> 
> Hi Xose,
> 
> I added the following patch to my tree to avoid this.
> 
> Cheers,
> Jes
> 
> commit 9202f4947aac1d60084ee79c9b5294eb42ba59dc
> Author: Jes Sorensen <Jes.Sorensen@redhat.com>
> Date:   Thu Mar 17 13:53:48 2016 -0400
> 
>     rtl8xxxu: Fix OOPS if user tries to add device via /sys
>     
>     This driver relies on driver_info in struct usb_device_id, hence
>     adding a device via  /sys/bus/usb/drivers/rtl8xxxu/new_id would result
>     in a NULL pointer dereference.
>     
>     Instead print a message and return -ENODEV
>     
>     Reported-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
>     Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> index 8d893f4..55fc00e 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> @@ -9671,6 +9671,15 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>  
>  	udev = usb_get_dev(interface_to_usbdev(interface));
>  
> +	if (!id->driver_info) {
> +		dev_warn(&udev->dev,
> +			 "rtl8xxxu relies on driver_info in struct usb_device_id.\n");
> +		dev_warn(&udev->dev,
> +			 "Adding a device via /sys/bus/usb/drivers/rtl8xxxu/new_id is not supported!\n");
> +		ret = -ENODEV;
> +		goto exit;
> +	}
> +
>  	switch (id->idVendor) {
>  	case USB_VENDOR_ID_REALTEK:
>  		switch(id->idProduct) {
>


Dynamic adding and removing a new device IDs to a USB device drivers
via /sys/bus/usb/drivers/.../[new_id|remove_id]
https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-bus-usb

is not supported only with rtl8xxxu?



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

* Re: rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver
  2016-03-17 20:01     ` poma
@ 2016-03-17 20:11       ` Jes Sorensen
  2016-03-17 20:42       ` Greg KH
  2016-03-17 20:50       ` Alan Stern
  2 siblings, 0 replies; 8+ messages in thread
From: Jes Sorensen @ 2016-03-17 20:11 UTC (permalink / raw)
  To: poma; +Cc: Xose Vazquez Perez, linux-wireless, linux-usb@vger.kernel.org

poma <pomidorabelisima@gmail.com> writes:
> On 17.03.2016 19:02, Jes Sorensen wrote:
>> Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>>> Xose Vazquez Perez <xose.vazquez@gmail.com> writes:
>>>> Hi,
>>>>
>>>> If I do:
>>>> # echo "0bda 8176" > /sys/bus/usb/drivers/rtl8xxxu/new_id
>>>
>>> Hi Xose,
>>>
>>> Yes please don't do that. The rtl8xxxu driver relies on the .driver_info
>>> field in struct use_device_id to carry information for the different
>>> types of devices. If you hot add a device like above, the driver will
>>> fail because that field now contains a NULL pointer.
>>>
>>> I should probably add a check for it in the probe function, but it will
>>> simply be there to spit out a warning that it doesn't work to hot add a
>>> device like this.
>>>
>>> If you build it with CONFIG_RTL8XXXU_UNTESTED the 0bda:8176 should be
>>> included in the device list.
>>>
>>> Cheers,
>>> Jes
>> 
>> Hi Xose,
>> 
>> I added the following patch to my tree to avoid this.
>> 
>> Cheers,
>> Jes
>> 
>> commit 9202f4947aac1d60084ee79c9b5294eb42ba59dc
>> Author: Jes Sorensen <Jes.Sorensen@redhat.com>
>> Date:   Thu Mar 17 13:53:48 2016 -0400
>> 
>>     rtl8xxxu: Fix OOPS if user tries to add device via /sys
>>     
>>     This driver relies on driver_info in struct usb_device_id, hence
>>     adding a device via  /sys/bus/usb/drivers/rtl8xxxu/new_id would result
>>     in a NULL pointer dereference.
>>     
>>     Instead print a message and return -ENODEV
>>     
>>     Reported-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
>>     Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>> 
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> index 8d893f4..55fc00e 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> @@ -9671,6 +9671,15 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>>  
>>  	udev = usb_get_dev(interface_to_usbdev(interface));
>>  
>> +	if (!id->driver_info) {
>> +		dev_warn(&udev->dev,
>> +			 "rtl8xxxu relies on driver_info in struct usb_device_id.\n");
>> +		dev_warn(&udev->dev,
>> +			 "Adding a device via /sys/bus/usb/drivers/rtl8xxxu/new_id is not supported!\n");
>> +		ret = -ENODEV;
>> +		goto exit;
>> +	}
>> +
>>  	switch (id->idVendor) {
>>  	case USB_VENDOR_ID_REALTEK:
>>  		switch(id->idProduct) {
>
> Dynamic adding and removing a new device IDs to a USB device drivers
> via /sys/bus/usb/drivers/.../[new_id|remove_id]
> https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-bus-usb
>
> is not supported only with rtl8xxxu?

Dynamically adding one will not work. The driver relies on driver_info
in struct usb_device_id() to carry the struct fileops associated with
the device type and you cannot specify this via the USB add operation.

Jes

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

* Re: rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver
  2016-03-17 20:01     ` poma
  2016-03-17 20:11       ` Jes Sorensen
@ 2016-03-17 20:42       ` Greg KH
  2016-03-17 21:47         ` Jes Sorensen
  2016-03-17 20:50       ` Alan Stern
  2 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2016-03-17 20:42 UTC (permalink / raw)
  To: poma
  Cc: Jes Sorensen, Xose Vazquez Perez, linux-wireless,
	linux-usb@vger.kernel.org

On Thu, Mar 17, 2016 at 09:01:25PM +0100, poma wrote:
> On 17.03.2016 19:02, Jes Sorensen wrote:
> > Jes Sorensen <Jes.Sorensen@redhat.com> writes:
> >> Xose Vazquez Perez <xose.vazquez@gmail.com> writes:
> >>> Hi,
> >>>
> >>> If I do:
> >>> # echo "0bda 8176" > /sys/bus/usb/drivers/rtl8xxxu/new_id
> >>
> >> Hi Xose,
> >>
> >> Yes please don't do that. The rtl8xxxu driver relies on the .driver_info
> >> field in struct use_device_id to carry information for the different
> >> types of devices. If you hot add a device like above, the driver will
> >> fail because that field now contains a NULL pointer.
> >>
> >> I should probably add a check for it in the probe function, but it will
> >> simply be there to spit out a warning that it doesn't work to hot add a
> >> device like this.
> >>
> >> If you build it with CONFIG_RTL8XXXU_UNTESTED the 0bda:8176 should be
> >> included in the device list.
> >>
> >> Cheers,
> >> Jes
> > 
> > Hi Xose,
> > 
> > I added the following patch to my tree to avoid this.
> > 
> > Cheers,
> > Jes
> > 
> > commit 9202f4947aac1d60084ee79c9b5294eb42ba59dc
> > Author: Jes Sorensen <Jes.Sorensen@redhat.com>
> > Date:   Thu Mar 17 13:53:48 2016 -0400
> > 
> >     rtl8xxxu: Fix OOPS if user tries to add device via /sys
> >     
> >     This driver relies on driver_info in struct usb_device_id, hence
> >     adding a device via  /sys/bus/usb/drivers/rtl8xxxu/new_id would result
> >     in a NULL pointer dereference.
> >     
> >     Instead print a message and return -ENODEV
> >     
> >     Reported-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> >     Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> > 
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > index 8d893f4..55fc00e 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > @@ -9671,6 +9671,15 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> >  
> >  	udev = usb_get_dev(interface_to_usbdev(interface));
> >  
> > +	if (!id->driver_info) {
> > +		dev_warn(&udev->dev,
> > +			 "rtl8xxxu relies on driver_info in struct usb_device_id.\n");
> > +		dev_warn(&udev->dev,
> > +			 "Adding a device via /sys/bus/usb/drivers/rtl8xxxu/new_id is not supported!\n");

We do have a flag in the USB driver structure to prevent this from
happening, "no_dynamic_id", please just set that and then this should
not happen.

thanks,

gre k-h

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

* Re: rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver
  2016-03-17 20:01     ` poma
  2016-03-17 20:11       ` Jes Sorensen
  2016-03-17 20:42       ` Greg KH
@ 2016-03-17 20:50       ` Alan Stern
  2 siblings, 0 replies; 8+ messages in thread
From: Alan Stern @ 2016-03-17 20:50 UTC (permalink / raw)
  To: poma
  Cc: Jes Sorensen, Xose Vazquez Perez, linux-wireless,
	linux-usb@vger.kernel.org

On Thu, 17 Mar 2016, poma wrote:

> On 17.03.2016 19:02, Jes Sorensen wrote:
> > Jes Sorensen <Jes.Sorensen@redhat.com> writes:
> >> Xose Vazquez Perez <xose.vazquez@gmail.com> writes:
> >>> Hi,
> >>>
> >>> If I do:
> >>> # echo "0bda 8176" > /sys/bus/usb/drivers/rtl8xxxu/new_id
> >>
> >> Hi Xose,
> >>
> >> Yes please don't do that. The rtl8xxxu driver relies on the .driver_info
> >> field in struct use_device_id to carry information for the different
> >> types of devices. If you hot add a device like above, the driver will
> >> fail because that field now contains a NULL pointer.
> >>
> >> I should probably add a check for it in the probe function, but it will
> >> simply be there to spit out a warning that it doesn't work to hot add a
> >> device like this.
> >>
> >> If you build it with CONFIG_RTL8XXXU_UNTESTED the 0bda:8176 should be
> >> included in the device list.
> >>
> >> Cheers,
> >> Jes
> > 
> > Hi Xose,
> > 
> > I added the following patch to my tree to avoid this.
> > 
> > Cheers,
> > Jes
> > 
> > commit 9202f4947aac1d60084ee79c9b5294eb42ba59dc
> > Author: Jes Sorensen <Jes.Sorensen@redhat.com>
> > Date:   Thu Mar 17 13:53:48 2016 -0400
> > 
> >     rtl8xxxu: Fix OOPS if user tries to add device via /sys
> >     
> >     This driver relies on driver_info in struct usb_device_id, hence
> >     adding a device via  /sys/bus/usb/drivers/rtl8xxxu/new_id would result
> >     in a NULL pointer dereference.
> >     
> >     Instead print a message and return -ENODEV
> >     
> >     Reported-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
> >     Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> > 
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > index 8d893f4..55fc00e 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
> > @@ -9671,6 +9671,15 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> >  
> >  	udev = usb_get_dev(interface_to_usbdev(interface));
> >  
> > +	if (!id->driver_info) {
> > +		dev_warn(&udev->dev,
> > +			 "rtl8xxxu relies on driver_info in struct usb_device_id.\n");

You should leave out this line.  It won't mean anything to the general
user.

Alan Stern


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

* Re: rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver
  2016-03-17 20:42       ` Greg KH
@ 2016-03-17 21:47         ` Jes Sorensen
  0 siblings, 0 replies; 8+ messages in thread
From: Jes Sorensen @ 2016-03-17 21:47 UTC (permalink / raw)
  To: Greg KH; +Cc: poma, Xose Vazquez Perez, linux-wireless,
	linux-usb@vger.kernel.org

Greg KH <greg@kroah.com> writes:
> On Thu, Mar 17, 2016 at 09:01:25PM +0100, poma wrote:
>> On 17.03.2016 19:02, Jes Sorensen wrote:
>> > Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>> >> Xose Vazquez Perez <xose.vazquez@gmail.com> writes:
>> >>> Hi,
>> >>>
>> >>> If I do:
>> >>> # echo "0bda 8176" > /sys/bus/usb/drivers/rtl8xxxu/new_id
>> >>
>> >> Hi Xose,
>> >>
>> >> Yes please don't do that. The rtl8xxxu driver relies on the .driver_info
>> >> field in struct use_device_id to carry information for the different
>> >> types of devices. If you hot add a device like above, the driver will
>> >> fail because that field now contains a NULL pointer.
>> >>
>> >> I should probably add a check for it in the probe function, but it will
>> >> simply be there to spit out a warning that it doesn't work to hot add a
>> >> device like this.
>> >>
>> >> If you build it with CONFIG_RTL8XXXU_UNTESTED the 0bda:8176 should be
>> >> included in the device list.
>> >>
>> >> Cheers,
>> >> Jes
>> > 
>> > Hi Xose,
>> > 
>> > I added the following patch to my tree to avoid this.
>> > 
>> > Cheers,
>> > Jes
>> > 
>> > commit 9202f4947aac1d60084ee79c9b5294eb42ba59dc
>> > Author: Jes Sorensen <Jes.Sorensen@redhat.com>
>> > Date:   Thu Mar 17 13:53:48 2016 -0400
>> > 
>> >     rtl8xxxu: Fix OOPS if user tries to add device via /sys
>> >     
>> >     This driver relies on driver_info in struct usb_device_id, hence
>> >     adding a device via  /sys/bus/usb/drivers/rtl8xxxu/new_id would result
>> >     in a NULL pointer dereference.
>> >     
>> >     Instead print a message and return -ENODEV
>> >     
>> >     Reported-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
>> >     Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>> > 
>> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> > b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> > index 8d893f4..55fc00e 100644
>> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.c
>> > @@ -9671,6 +9671,15 @@ static int rtl8xxxu_probe(struct
>> > usb_interface *interface,
>> >  
>> >  	udev = usb_get_dev(interface_to_usbdev(interface));
>> >  
>> > +	if (!id->driver_info) {
>> > +		dev_warn(&udev->dev,
>> > + "rtl8xxxu relies on driver_info in struct usb_device_id.\n");
>> > +		dev_warn(&udev->dev,
>> > + "Adding a device via /sys/bus/usb/drivers/rtl8xxxu/new_id is not
>> > supported!\n");
>
> We do have a flag in the USB driver structure to prevent this from
> happening, "no_dynamic_id", please just set that and then this should
> not happen.

I wasn't aware of this flag - I'll update the patch to do that.

Thanks,
Jes

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

end of thread, other threads:[~2016-03-17 21:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-15 23:42 rtl8xxxu 4.4.5(from f23): I get a panic adding a new device to the driver Xose Vazquez Perez
2016-03-16 13:58 ` Jes Sorensen
2016-03-17 18:02   ` Jes Sorensen
2016-03-17 20:01     ` poma
2016-03-17 20:11       ` Jes Sorensen
2016-03-17 20:42       ` Greg KH
2016-03-17 21:47         ` Jes Sorensen
2016-03-17 20:50       ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).