From: Hayes Wang <hidden> Date: 2021-05-13 03:14:02
syzbot [off-list ref]
Sent: Wednesday, May 12, 2021 5:40 PM
[...]
usb 1-1: New USB device found, idVendor=045e, idProduct=0927, bcdDevice=89.4f
usb 1-1: New USB device strings: Mfr=0, Product=4, SerialNumber=0
usb 1-1: Product: syz
usb 1-1: config 0 descriptor??
The bcdDevice is strange. Could you dump your USB descriptor?
My log is as following.
[root@fc32 r8152_inbox]# dmesg
[ 2174.703974] usb 2-8: new SuperSpeed Gen 1 USB device number 7 using xhci_hcd
[ 2174.716592] usb 2-8: New USB device found, idVendor=045e, idProduct=0927, bcdDevice=31.00
[ 2174.716604] usb 2-8: New USB device strings: Mfr=1, Product=2, SerialNumber=6
[ 2174.716609] usb 2-8: Product: USB 10/100/1000 LAN
[ 2174.716613] usb 2-8: Manufacturer: Realtek
[ 2174.716617] usb 2-8: SerialNumber: 0010010AA
[ 2174.837277] usb 2-8: reset SuperSpeed Gen 1 USB device number 7 using xhci_hcd
[ 2174.869013] r8152 2-8:1.0: load rtl8153b-2 v1 10/23/19 successfully
[ 2174.897836] r8152 2-8:1.0 eth2: v1.12.11
[root@fc32 r8152_inbox]# ethtool -i eth2
driver: r8152
version: v1.12.11
firmware-version: rtl8153b-2 v1 10/23/19
expansion-rom-version:
bus-info: usb-0000:00:14.0-8
supports-statistics: yes
supports-test: no
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: no
[root@fc32 r8152_inbox]# lsusb -vd 045e:0927
Bus 002 Device 007: ID 045e:0927 Microsoft Corp. RTL8153B GigE [Surface Ethernet Adapter]
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 3.00
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 9
idVendor 0x045e Microsoft Corp.
idProduct 0x0927 RTL8153B GigE [Surface Ethernet Adapter]
bcdDevice 31.00
iManufacturer 1 Realtek
iProduct 2 USB 10/100/1000 LAN
iSerial 6 0010010AA
bNumConfigurations 2
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 0x0039
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 288mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 255 Vendor Specific Subclass
bInterfaceProtocol 0
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 0x0400 1x 1024 bytes
bInterval 0
bMaxBurst 3
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0400 1x 1024 bytes
bInterval 0
bMaxBurst 3
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0002 1x 2 bytes
bInterval 8
bMaxBurst 0
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 0x0062
bNumInterfaces 2
bConfigurationValue 2
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 288mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 2 Communications
bInterfaceSubClass 6 Ethernet Networking
bInterfaceProtocol 0
iInterface 5 CDC Communications Control
CDC Header:
bcdCDC 1.10
CDC Union:
bMasterInterface 0
bSlaveInterface 1
CDC Ethernet:
iMacAddress 3 00E04C660016
bmEthernetStatistics 0x00000000
wMaxSegmentSize 1514
wNumberMCFilters 0x0000
bNumberPowerFilters 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0010 1x 16 bytes
bInterval 8
bMaxBurst 0
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 0
bInterfaceClass 10 CDC Data
bInterfaceSubClass 0
bInterfaceProtocol 0
iInterface 0
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 1
bNumEndpoints 2
bInterfaceClass 10 CDC Data
bInterfaceSubClass 0
bInterfaceProtocol 0
iInterface 4 Ethernet Data
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0400 1x 1024 bytes
bInterval 0
bMaxBurst 3
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0400 1x 1024 bytes
bInterval 0
bMaxBurst 3
Binary Object Store Descriptor:
bLength 5
bDescriptorType 15
wTotalLength 0x0016
bNumDeviceCaps 2
USB 2.0 Extension Device Capability:
bLength 7
bDescriptorType 16
bDevCapabilityType 2
bmAttributes 0x00000002
HIRD Link Power Management (LPM) Supported
SuperSpeed USB Device Capability:
bLength 10
bDescriptorType 16
bDevCapabilityType 3
bmAttributes 0x02
Latency Tolerance Messages (LTM) Supported
wSpeedsSupported 0x000e
Device can operate at Full Speed (12Mbps)
Device can operate at High Speed (480Mbps)
Device can operate at SuperSpeed (5Gbps)
bFunctionalitySupport 2
Lowest fully-functional device speed is High Speed (480Mbps)
bU1DevExitLat 10 micro seconds
bU2DevExitLat 2047 micro seconds
can't get debug descriptor: Resource temporarily unavailable
Device Status: 0x0010
(Bus Powered)
Latency Tolerance Messaging (LTM) Enabled
[root@fc32 r8152_inbox]#
Best Regards,
Hayes
From: Alan Stern <stern@rowland.harvard.edu> Date: 2021-05-13 14:25:57
On Thu, May 13, 2021 at 03:13:36AM +0000, Hayes Wang wrote:
syzbot [off-list ref]
quoted
Sent: Wednesday, May 12, 2021 5:40 PM
[...]
quoted
usb 1-1: New USB device found, idVendor=045e, idProduct=0927, bcdDevice=89.4f
usb 1-1: New USB device strings: Mfr=0, Product=4, SerialNumber=0
usb 1-1: Product: syz
usb 1-1: config 0 descriptor??
The bcdDevice is strange. Could you dump your USB descriptor?
Syzbot doesn't test real devices. It tests emulations, and the emulated
devices usually behave very strangely and in very peculiar and
unexpected ways, so as to trigger bugs in the kernel. That's why the
USB devices you see in syzbot logs usually have bizarre descriptors.
Alan Stern
From: Hayes Wang <hidden> Date: 2021-05-14 02:58:26
Alan Stern [off-list ref]
Sent: Thursday, May 13, 2021 10:26 PM
[...]
Syzbot doesn't test real devices. It tests emulations, and the emulated
devices usually behave very strangely and in very peculiar and
unexpected ways, so as to trigger bugs in the kernel. That's why the
USB devices you see in syzbot logs usually have bizarre descriptors.
Do you mean I have to debug for a device which doesn't exist?
I don't understand why I must consider a fake device
which provide unexpected USB descriptor deliberately?
Best Regards,
Hayes
From: Dan Carpenter <hidden> Date: 2021-05-14 06:42:20
On Fri, May 14, 2021 at 02:58:00AM +0000, Hayes Wang wrote:
Alan Stern [off-list ref]
quoted
Sent: Thursday, May 13, 2021 10:26 PM
[...]
quoted
Syzbot doesn't test real devices. It tests emulations, and the emulated
devices usually behave very strangely and in very peculiar and
unexpected ways, so as to trigger bugs in the kernel. That's why the
USB devices you see in syzbot logs usually have bizarre descriptors.
Do you mean I have to debug for a device which doesn't exist?
I don't understand why I must consider a fake device
which provide unexpected USB descriptor deliberately?
On Fri, May 14, 2021 at 02:58:00AM +0000, Hayes Wang wrote:
Alan Stern [off-list ref]
quoted
Sent: Thursday, May 13, 2021 10:26 PM
[...]
quoted
Syzbot doesn't test real devices. It tests emulations, and the emulated
devices usually behave very strangely and in very peculiar and
unexpected ways, so as to trigger bugs in the kernel. That's why the
USB devices you see in syzbot logs usually have bizarre descriptors.
Do you mean I have to debug for a device which doesn't exist?
I don't understand why I must consider a fake device
which provide unexpected USB descriptor deliberately?
Because people can create "bad" devices and plug them into a system
which causes the driver to load and then potentially crash the system or
do other bad things.
USB drivers now need to be able to handle "malicious" devices, it's been
that way for many years now.
thanks,
greg k-h
From: Hayes Wang <hidden> Date: 2021-05-14 07:50:31
Dan Carpenter [off-list ref]
Sent: Friday, May 14, 2021 2:42 PM
[...]
Imagine you are at a conference and two people sit down next to you, one
on either side. The one accidentally spills coffee on your lap. The
other plugs in a USB device to your laptop. Now you are infected with
spyware.
I don't think I could find out such devices by only checking the information
of the hardware. That is, there is no way now to avoid other devices to
use our driver.
Best Regards,
Hayes
From: Hayes Wang <hidden> Date: 2021-05-14 07:50:44
Greg KH [off-list ref]
Sent: Friday, May 14, 2021 2:49 PM
[...]
Because people can create "bad" devices and plug them into a system
which causes the driver to load and then potentially crash the system or
do other bad things.
USB drivers now need to be able to handle "malicious" devices, it's been
that way for many years now.
My question is that even I check whole the USB descriptor, the malicious
devices could duplicate it easily to pass my checks. That is, I could add a
lot of checks, but it still doesn't prevent malicious devices. Is this meaningful?
Best Regards,
Hayes
On Fri, May 14, 2021 at 07:50:19AM +0000, Hayes Wang wrote:
Greg KH [off-list ref]
quoted
Sent: Friday, May 14, 2021 2:49 PM
[...]
quoted
Because people can create "bad" devices and plug them into a system
which causes the driver to load and then potentially crash the system or
do other bad things.
USB drivers now need to be able to handle "malicious" devices, it's been
that way for many years now.
My question is that even I check whole the USB descriptor, the malicious
devices could duplicate it easily to pass my checks. That is, I could add a
lot of checks, but it still doesn't prevent malicious devices. Is this meaningful?
Checking the whole USB decriptor is fine, yes, they can duplicate that.
So that means you need to validate _ALL_ data coming from the device
that it is in an acceptable range of values that the driver can
correctly handle.
thanks,
greg k-h
From: Hayes Wang <hidden> Date: 2021-05-14 10:32:49
Greg KH [off-list ref]
Sent: Friday, May 14, 2021 4:27 PM
[...]
Checking the whole USB decriptor is fine, yes, they can duplicate that.
So that means you need to validate _ALL_ data coming from the device
that it is in an acceptable range of values that the driver can
correctly handle.
From: Alan Stern <stern@rowland.harvard.edu> Date: 2021-05-14 15:33:13
On Fri, May 14, 2021 at 07:50:19AM +0000, Hayes Wang wrote:
Greg KH [off-list ref]
quoted
Sent: Friday, May 14, 2021 2:49 PM
[...]
quoted
Because people can create "bad" devices and plug them into a system
which causes the driver to load and then potentially crash the system or
do other bad things.
USB drivers now need to be able to handle "malicious" devices, it's been
that way for many years now.
My question is that even I check whole the USB descriptor, the malicious
devices could duplicate it easily to pass my checks. That is, I could add a
lot of checks, but it still doesn't prevent malicious devices. Is this meaningful?
The real motivation here, which nobody has mentioned explicitly yet, is
that the driver needs to be careful enough that it won't crash no matter
what bizarre, malfunctioning, or malicious device is attached.
Even if a device isn't malicious, if it is buggy, broken, or
malfunctioning in some way then it can present input that a normal
device would never generate. If the driver isn't prepared to handle
this unusual input, it may crash. That is specifically what we want to
avoid.
So if a peculiar emulated device created by syzbot is capable of
crashing the driver, then somewhere there is a bug which needs to be
fixed. It's true that fixing all these bugs might not protect against a
malicious device which deliberately behaves in an apparently reasonable
manner. But it does reduce the attack surface.
Alan Stern
From: Hayes Wang <hidden> Date: 2021-05-17 01:01:56
Alan Stern [off-list ref]
Sent: Friday, May 14, 2021 11:33 PM
[...]
The real motivation here, which nobody has mentioned explicitly yet, is
that the driver needs to be careful enough that it won't crash no matter
what bizarre, malfunctioning, or malicious device is attached.
Even if a device isn't malicious, if it is buggy, broken, or
malfunctioning in some way then it can present input that a normal
device would never generate. If the driver isn't prepared to handle
this unusual input, it may crash. That is specifically what we want to
avoid.
So if a peculiar emulated device created by syzbot is capable of
crashing the driver, then somewhere there is a bug which needs to be
fixed. It's true that fixing all these bugs might not protect against a
malicious device which deliberately behaves in an apparently reasonable
manner. But it does reduce the attack surface.
Thanks for your response.
I will add some checks.
Best Regards,
Hayes
From: Oliver Neukum <oneukum@suse.com> Date: 2021-05-17 10:00:47
Am Montag, den 17.05.2021, 01:01 +0000 schrieb Hayes Wang:
Alan Stern [off-list ref]
quoted
Sent: Friday, May 14, 2021 11:33 PM
quoted
So if a peculiar emulated device created by syzbot is capable of
crashing the driver, then somewhere there is a bug which needs to
be
fixed. It's true that fixing all these bugs might not protect
against a
malicious device which deliberately behaves in an apparently
reasonable
manner. But it does reduce the attack surface.
Thanks for your response.
I will add some checks.
Hi,
the problem in this particular case is in
static bool rtl_vendor_mode(struct usb_interface *intf)
which accepts any config number. It needs to bail out
if you find config #0 to be what the descriptors say,
treating that as an unrecoverable error.
Regards
Oliver
From: Alan Stern <stern@rowland.harvard.edu> Date: 2021-05-17 13:47:22
On Mon, May 17, 2021 at 12:00:19PM +0200, Oliver Neukum wrote:
Am Montag, den 17.05.2021, 01:01 +0000 schrieb Hayes Wang:
quoted
Alan Stern [off-list ref]
quoted
Sent: Friday, May 14, 2021 11:33 PM
quoted
quoted
So if a peculiar emulated device created by syzbot is capable of
crashing the driver, then somewhere there is a bug which needs to
be
fixed. It's true that fixing all these bugs might not protect
against a
malicious device which deliberately behaves in an apparently
reasonable
manner. But it does reduce the attack surface.
Thanks for your response.
I will add some checks.
Hi,
the problem in this particular case is in
static bool rtl_vendor_mode(struct usb_interface *intf)
which accepts any config number. It needs to bail out
if you find config #0 to be what the descriptors say,
treating that as an unrecoverable error.
No, the problem is that the routine calls WARN_ON_ONCE when it doesn't
find an appropriate configuration. WARN_ON_ONCE means there is a bug or
problem in the kernel. That's not the issue here; the issue is that the
device doesn't have the expected descriptors.
The line should be dev_warn(), not WARN_ON_ONCE.
Alan Stern