From: rtm@csail.mit.edu
To: Greg KH <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Subject: Re: USB hub code can dereference NULL hub and hub->ports
Date: Wed, 22 Jan 2025 06:37:45 -0500 [thread overview]
Message-ID: <66581.1737545865@localhost> (raw)
In-Reply-To: Your message of "Tue, 21 Jan 2025 08:01:22 +0100." <2025012150-nervous-john-fb53@gregkh>
[-- Attachment #1: Type: text/plain, Size: 3169 bytes --]
> Great, can you submit patches to fix these issues now that you have a
> reliable test program to verify the problem?
I think the problem is (at least sometimes) not that hub->ports is
legitimately NULL and there's a missing check, but that
usb_hub_to_struct_hub() returns an object of the wrong type so that
hub->ports is junk, and only accidentally NULL in the demo I
previously submitted.
I've attached a new demo which crashes because hub->ports is
0xcccccccccccccccc (on a kernel with red zones). The demo presents a
USB device whose DeviceClass is a hub (9), with two interfaces, but
the Vendor and Product indicate an FTDI serial adaptor.
First, usb_serial_probe() sets the interface zero dev->driver_data to
a struct usb_serial.
Later, when the hub code is trying to set up interface one, it calls
usb_hub_to_struct_hub(hdev):
struct usb_hub *usb_hub_to_struct_hub(struct usb_device *hdev)
{
if (!hdev || !hdev->actconfig || !hdev->maxchild)
return NULL;
return usb_get_intfdata(hdev->actconfig->interface[0]);
}
interface[0], however, has been set up by the serial port code, and
its dev->driver_data is a struct usb_serial, not a struct usb_hub.
struct usb_serial is shorter than usb_hub, and as a result the end of
the usb_hub is beyond the end of the allocated memory, which causes
hub->ports to refer to bytes in the red zone.
I don't understand the code well enough to suggest a patch.
# cc usbser1c.c
# ./a.out
...
hub 1-1:1.0: bad descriptor, ignoring hub
hub 1-1:1.0: probe with driver hub failed with error -5
Oops: general protection fault, probably for non-canonical address 0xcccccccccccccccc: 0000 [#1] SMP DEBUG_PAGEALLOC PTI
CPU: 7 UID: 0 PID: 117 Comm: kworker/7:1 Not tainted 6.13.0-rc3-00017-gf44d154d6
e3d #14
Hardware name: FreeBSD BHYVE/BHYVE, BIOS 14.0 10/17/2021
Workqueue: usb_hub_wq hub_event
RIP: 0010:usb_hub_adjust_deviceremovable+0x78/0x110
...
Call Trace:
<TASK>
? die_addr+0x31/0x80
? exc_general_protection+0x1b4/0x3c0
? asm_exc_general_protection+0x26/0x30
? usb_hub_adjust_deviceremovable+0x78/0x110
hub_probe+0x7c7/0xab0
usb_probe_interface+0x14b/0x350
really_probe+0xd0/0x2d0
? __pfx___device_attach_driver+0x10/0x10
__driver_probe_device+0x6e/0x110
driver_probe_device+0x1a/0x90
__device_attach_driver+0x7e/0xc0
bus_for_each_drv+0x7f/0xd0
__device_attach+0xaa/0x1a0
bus_probe_device+0x8b/0xa0
device_add+0x62e/0x810
usb_set_configuration+0x65d/0x990
usb_generic_driver_probe+0x4b/0x70
usb_probe_device+0x36/0xd0
really_probe+0xd0/0x2d0
? __pfx___device_attach_driver+0x10/0x10
__driver_probe_device+0x6e/0x110
driver_probe_device+0x1a/0x90
__device_attach_driver+0x7e/0xc0
bus_for_each_drv+0x7f/0xd0
__device_attach+0xaa/0x1a0
bus_probe_device+0x8b/0xa0
device_add+0x62e/0x810
? usb_detect_static_quirks+0x41/0xf0
usb_new_device+0x1c8/0x400
hub_event+0x1047/0x1870
process_one_work+0x13f/0x330
worker_thread+0x25a/0x370
? _raw_spin_unlock_irqrestore+0xd/0x20
? __pfx_worker_thread+0x10/0x10
kthread+0xdc/0x110
? __pfx_kthread+0x10/0x10
ret_from_fork+0x2f/0x50
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
Robert Morris
rtm@mit.edu
[-- Attachment #2: usbser1c.c --]
[-- Type: application/octet-stream, Size: 14444 bytes --]
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/socket.h>
#include <sys/select.h>
#include <sys/types.h>
#include <sys/time.h>
#include <netinet/in.h>
#include <signal.h>
#include <fcntl.h>
#include <string.h>
#include <sys/wait.h>
#include <sys/resource.h>
#include <assert.h>
struct op_common {
unsigned short version;
unsigned short code;
unsigned int status;
};
struct usbip_usb_device {
char path[256];
char busid[32];
uint32_t busnum;
uint32_t devnum;
uint32_t speed;
uint16_t idVendor;
uint16_t idProduct;
uint16_t bcdDevice;
uint8_t bDeviceClass;
uint8_t bDeviceSubClass;
uint8_t bDeviceProtocol;
uint8_t bConfigurationValue;
uint8_t bNumConfigurations;
uint8_t bNumInterfaces;
} __attribute__((packed));
struct usbip_header_basic {
unsigned int command;
unsigned int seqnum;
unsigned int devid;
unsigned int direction;
unsigned int ep;
};
struct usbip_header_cmd_submit {
unsigned int transfer_flags;
int transfer_buffer_length;
int start_frame;
int number_of_packets;
int interval;
unsigned char setup[8];
};
struct usbip_header_ret_submit {
int status;
int actual_length;
int start_frame;
int number_of_packets;
int error_count;
};
int
readable(int fd)
{
fd_set readfds;
FD_ZERO(&readfds);
FD_SET(fd, &readfds);
struct timeval tv;
tv.tv_sec = 4;
tv.tv_usec = 0;
int ss = select(fd + 1, &readfds, (fd_set*)0, (fd_set*)0, &tv);
return FD_ISSET(fd, &readfds);
}
int
readn(int fd, void *xbuf, int n)
{
char *buf = xbuf;
int got = 0;
while(got < n){
if(readable(fd) == 0){
printf("usbip0: timeout\n");
return -1;
}
int cc = read(fd, buf+got, n-got);
if(cc <= 0){
perror("usbip0: read");
return -1;
}
got += cc;
}
return got;
}
void
mkif(char **xp, int num, int alt, int eps, int cl, int subcl, int proto, int iff)
{
char *p = *xp;
// usb_interface_descriptor
*p++ = 9; // bLength
*p++ = 4; // bDescriptorType USB_DT_INTERFACE
*p++ = num; // bInterfaceNumber
*p++ = alt; // bAlternateSetting
*p++ = eps; // bNumEndpoints
*p++ = cl; // bInterfaceClass
*p++ = subcl; // bInterfaceSubClass
*p++ = proto; // bInterfaceProtocol
*p++ = iff; // iInterface
*xp = p;
}
void
mkad(char **xp, int type, int subtype)
{
char *p = *xp;
// Additional Descriptor
*p++ = 0; // bLength (filled in later)
*p++ = type; // bDescriptorType
*p++ = subtype; // bDescriptorSubtype
if(type == 36 && subtype == 1){
// AS_GENERAL
*p++ = 1; // bTerminalLink
*p++ = 1; // bDelay
*p++ = 1; // wFormatTag PCM
p++;
} else if(type == 36 && subtype == 2){
// FORMAT_TYPE
*p++ = 1; // bFormatType
*p++ = 2; // bNrChannels
*p++ = 3; // bSubframeSize
*p++ = 24; // bBitResolution
*p++ = 2; // bSamFreqType
*p++ = 2; // bSamFreqType
p += 5;
} else {
*p++ = 0; // bcdADC
*p++ = 1;
*(short*)p = 0x5f; // wTotalLength
p += 2;
*p++ = 2; // bInCollection
*p++ = 1; // baInterfaceNr(0)
*p++ = 2; // baInterfaceNr(1)
}
*(*xp) = p - (*xp); // bLength
*xp = p;
}
void
mkadx(char **xp, int type, int subtype, int len, int a[])
{
char *p = *xp;
// Additional Descriptor
*p++ = 0; // bLength (filled in later)
*p++ = type; // bDescriptorType
*p++ = subtype; // bDescriptorSubtype
for(int i = 0; i < len - 3; i++)
*p++ = a[i];
*(*xp) = p - (*xp); // bLength
*xp = p;
}
void
mkep(char **xp, int epa, int attr, int maxp)
{
char *p = *xp;
// usb_endpoint_descriptor
*p++ = 9;
*p++ = 5; // bDescriptorType USB_DT_ENDPOINT
*p++ = epa; // bEndpointAddress
*p++ = attr; // bmAttributes
*(short*)p = maxp; // wMaxPacketSize
p += 2;
*p++ = 7; // bInterval
p += 2; // ???
*xp = p;
}
int
main(int argc, char *argv[])
{
struct rlimit r;
r.rlim_cur = r.rlim_max = 0;
setrlimit(RLIMIT_CORE, &r);
int port = 3240;
int s, yes = 1;
struct sockaddr_in sin;
//system("killall usbip");
//sleep(1);
memset(&sin, 0, sizeof(sin));
sin.sin_family = AF_INET;
sin.sin_port = htons(port);
s = socket(AF_INET, SOCK_STREAM, 0);
if(s < 0){
perror("socket");
exit(1);
}
setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes));
if(bind(s, (struct sockaddr *) &sin, sizeof(sin)) < 0){
perror("fastfingerd: bind");
exit(1);
}
if(listen(s, 3000) < 0){
perror("fastfingerd: listen");
exit(1);
}
//system("usbip/src/usbip attach -r 127.0.0.1 -b 1-1 &");
system("usbip attach -r 127.0.0.1 -b 1-1 &");
sleep(2);
sync();
sleep(1);
int s1;
unsigned sinlen = sizeof(sin);
s1 = accept(s, (struct sockaddr *) &sin, &sinlen);
if(s1 < 0){
perror("accept");
exit(1);
}
close(s);
struct op_common op;
// OP_REQ_IMPORT
readn(s1, &op, sizeof(op));
//printf("version 0x%x code 0x%x status 0x%x\n",
// op.version, op.code, op.status);
char busid[32];
readn(s1, busid, sizeof(busid));
op.code = htons(0x03); // OP_REP_IMPORT
op.status = htonl(0); // ST_OK
write(s1, &op, sizeof(op));
struct usbip_usb_device uud;
memset(&uud, 0, sizeof(uud));
strcpy(uud.busid, busid);
//uud.speed = htonl(2); // USB_SPEED_FULL
uud.speed = htonl(3); // USB_SPEED_HIGH
//uud.speed = htonl(5); // USB_SPEED_SUPER
write(s1, &uud, sizeof(uud));
// now talking to the kernel
int cmdno = 0;
int lastTag = 0; // usb_stor_BulkTransport line 1255
unsigned char CDB[16]; // SCSI command
memset(CDB, 0, sizeof(CDB));
while(1){
struct usbip_header_basic ibh;
//sync(); // don't sync() -- deadlock.
if(readn(s1, &ibh, sizeof(ibh)) < 0)
break;
#if 1
printf("%d: command 0x%x seqnum %d devid 0x%x direction 0x%x ep 0x%x\n",
cmdno,
ntohl(ibh.command),
ntohl(ibh.seqnum),
ntohl(ibh.devid),
ntohl(ibh.direction),
ntohl(ibh.ep));
#endif
if(ntohl(ibh.command) == 1){
// USBIP_CMD_SUBMIT
struct usbip_header_cmd_submit cs;
memset(&cs, 0, sizeof(cs));
if(readn(s1, &cs, sizeof(cs)) < 0)
break;
#if 1
printf(" flags 0x%x buflen %d np %d, ",
ntohl(cs.transfer_flags),
ntohl(cs.transfer_buffer_length),
ntohl(cs.number_of_packets));
for(int i = 0; i < 8; i++)
printf("%02x ", cs.setup[i] & 0xff);
printf("\n");
#endif
int translen = ntohl(cs.transfer_buffer_length);
if(ibh.direction == 0){
char ibuf[4096];
assert(translen <= sizeof(ibuf));
if(readn(s1, ibuf, translen) < 0)
break;
if(translen >= 16 && memcmp(ibuf, "USBC", 4) == 0){
// struct bulk_cb_wrap
lastTag = ibuf[4];
printf(" USBC tag=%d dtl=%d fl=0x%02x lun=%d len=%d\n",
*(int*)(ibuf+4),
*(int*)(ibuf+8),
ibuf[12] & 0xff,
ibuf[13] & 0xff,
ibuf[14] & 0xff);
printf(" ");
for(int i = 0; i < 16; i++)
printf("%02x ", ibuf[15+i] & 0xff);
printf("\n");
memcpy(CDB, ibuf+15, sizeof(CDB));
} else if(translen > 0){
for(int i = 0; i < translen && i < 16; i++)
printf("%02x ", ibuf[i] & 0xff);
printf("\n");
}
}
struct usbip_header_basic obh;
memset(&obh, 0, sizeof(obh));
obh.command = htonl(3); // USBIP_RET_SUBMIT
obh.seqnum = ibh.seqnum;
obh.devid = ibh.devid;
obh.direction = htonl(!ntohl(ibh.direction));
obh.ep = ibh.ep;
write(s1, &obh, sizeof(obh));
char rsbuf[sizeof(cs)];
memset(rsbuf, 0, sizeof(rsbuf));
struct usbip_header_ret_submit *rs = (void*)rsbuf;
if(ibh.direction){
rs->actual_length = htonl(translen);
} else {
rs->actual_length = htonl(31);
}
write(s1, rs, sizeof(rsbuf));
if(ibh.direction){
char buf64[4096];
if(translen > sizeof(buf64)){
printf("huge translen\n");
break;
}
memset(buf64, 0, sizeof(buf64));
if(cs.setup[1] == 0x06){
// USB_REQ_GET_DESCRIPTOR
if(cs.setup[0] == 0x80 && cs.setup[3] == 1){
// USB_DT_DEVICE
// struct usb_device_descriptor
buf64[0] = 18; // bLength
buf64[1] = 1; // bDescriptorType = USB_DT_DEVICE
buf64[2] = 0x20; // bcdUSB
buf64[3] = 0x03; // bcdUSB
buf64[4] = 9; // 0xef; // bDeviceClass
buf64[5] = 2; // bDeviceSubClass
buf64[6] = 1; // bDeviceProtocol
buf64[7] = 8; // bMaxPacketSize0
*(short*)(buf64+8) = 0x0403; // idVendor FTDI
*(short*)(buf64+10) = 0x6010; // idProduct
//*(short*)(buf64+8) = 0x06cd; // idVendor Keyspan
//*(short*)(buf64+10) = 0x0121; // idProduct USA-19hs
buf64[12] = 0; // bcdDevice
buf64[13] = 1; // bcdDevice
buf64[14] = 0; // iManufacturer
buf64[15] = 0; // iProduct
buf64[16] = 0; // iSerial
buf64[17] = 1; // bNumConfigurations
} else if(cs.setup[0] == 0x80 && cs.setup[3] == 2){
// USB_DT_CONFIG
// struct usb_config_descriptor
char *p = buf64;
*p++ = 9; // bLength
*p++ = 2; // USB_DT_CONFIG
short *lenp = (short*) p;
*(short*)p = 9 + 4*9 + 15*10 + 2*7; // wTotalLength
p += 2;
*p++ = 2; // bNumInterfaces
*p++ = 1; // bConfigurationValue
*p++ = 0; // iConfiguration
*p++ = 0x80; // bmAttributes
*p++ = 1; // bMaxPower
// interface 0
mkif(&p, 0, 0, 0, 1, 1, 0, 0);
int ad1[] = { 1, 0, 0x5f, 0, 2, 1, 2 };
mkadx(&p, 36, 1, 10, ad1);
int ad2[] = { 1, 0x0101, 0, 2, 3, 0, 0, 0, 0 };
mkadx(&p, 36, 2, 12, ad2);
for(int i = 0; i < 7; i++)
mkad(&p, 0x24, i+1);
mkif(&p, 1, 0, 1, 3, 0, 0, 0);
mkad(&p, 0x21, 1);
mkep(&p, 0x87, 3, 0x10);
assert(p - buf64 <= sizeof(buf64));
*lenp = p - buf64;
} else if(cs.setup[0] == 0x80 && cs.setup[3] == 0x0f){
// USB_DT_BOS
// struct usb_bos_descriptor
char *p = buf64;
*p++ = 5; // bLength
*p++ = 15;
*(short*)p = 0x002a; // wTotalLength
p += 2;
*p++ = 3; // bNumDeviceCaps
// usb_ext_cap_descriptor
*p++ = 7; // bLength
*p++ = 16; // bDescriptorType
*p++ = 2; // bDevCapabilityType
*(int*)p = 0x0000f41e; // bmAttributes
p += 4;
// usb_ss_cap_descriptor
*p++ = 10; // bLength
*p++ = 16; // bDescriptorType
*p++ = 3; // bDevCapabilityType
*p++ = 0; // bmAttributes
*(short*)p = 0xe; // wSpeedsSupported
p += 2;
*p++ = 1; // bFunctionalitySupport
*p++ = 10; // bU1devExitLat
*(short*)p = 2047; // bU2DevExitLat
p += 2;
// usb_ssp_cap_descriptor
*p++ = 20; // bLength
*p++ = 16; // bDescriptorType
*p++ = 10; // bDevCapabilityType
*p++ = 0; // bReserved
*(int*)p = 0; // bmAttributes
p += 4;
*(short*)p = 1; // bFunctionalitySupport
p += 2;
p += 2; // wReserved
*(int*)p = 0x000a4030;
p += 4;
*(int*)p = 0x000a40b0;
p += 4;
} else if(cs.setup[0] == 0x80 && cs.setup[3] == 3){
// USB_DT_STRING
char *p = buf64;
*p++ = 6; // length
*p++ = 3; // descriptor type
*p++ = 'a';
*p++ = 'b';
*p++ = 'c';
*p++ = 'd';
} else if(cs.setup[0] == 0xa0){
// usb_hub_descriptor
buf64[0] = 12; // bDescLength
buf64[1] = 42; // bDescriptorType
buf64[2] = 1; // bNbrPorts
buf64[6] = 8; // bHubContrCurrent
}
} else if(cs.setup[1] == 0 && cs.setup[0] == 0x80){
// USB_REQ_GET_STATUS USB_RT_PORT
// usb_port_status
*(short*)(buf64+0) = 3; // wPortStatus
} else if(cs.setup[1] == 0 && cs.setup[0] == 0xa0){
// USB_REQ_GET_STATUS USB_RT_HUB
// usb_hub_status
} else if(cs.setup[1] == 0xfe){
if(ntohl(ibh.ep) == 0){
// US_BULK_GET_MAX_LUN
buf64[0] = 0; // maybe max unit #?
} else {
memcpy(buf64, "USBS", 4);
buf64[4] = lastTag; // make transport.c:1255 happy: bulk_cs_wrap.Tag
}
} else if(ntohl(cs.transfer_flags) == 0x40201 &&
cs.setup[0] == 0 && cs.setup[1] == 0){
// usb_stor_Bulk_transport reading SCSI cmd result
if(CDB[0] == 0x12){
// INQUIRY
buf64[4] = 36 - 5; // response_len
buf64[2] = 14; // scsi_level?
buf64[8] = 0xff; // flags?
buf64[16] = 0xff; // flags?
} else if(CDB[0] == 0x25){
// READ_CAPACITY
*(int*)(buf64+0) = htonl(1024); // lba? capacity?
*(int*)(buf64+4) = htonl(512); // sector size
}
}
write(s1, buf64, translen);
}
} else if(ntohl(ibh.command) == 2){
// USBIP_CMD_UNLINK
// struct usbip_header_cmd_unlink uh;
char buf[sizeof(struct usbip_header_cmd_submit)];
memset(buf, 0, sizeof(buf));
if(readn(s1, buf, sizeof(buf)) < 0)
break;
unsigned int uh = *(int*)buf;;
printf("unlink seq %d\n", ntohl(uh));
struct usbip_header_basic obh;
memset(&obh, 0, sizeof(obh));
obh.command = htonl(4); // USBIP_RET_UNLINK
obh.seqnum = ibh.seqnum;
obh.devid = ibh.devid;
obh.direction = htonl(!ntohl(ibh.direction));
obh.ep = ibh.ep;
write(s1, &obh, sizeof(obh));
char rsbuf[sizeof(struct usbip_header_cmd_submit)];
memset(rsbuf, 0, sizeof(rsbuf));
write(s1, rsbuf, sizeof(rsbuf));
}
if(cmdno >= 50)
break;
cmdno += 1;
}
usleep(500000);
close(s1);
}
next prev parent reply other threads:[~2025-01-22 11:37 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-20 17:27 USB hub code can dereference NULL hub and hub->ports rtm
2025-01-21 7:01 ` Greg KH
2025-01-22 11:37 ` rtm [this message]
2025-01-22 15:55 ` Alan Stern
2025-01-22 19:21 ` rtm
2025-01-22 19:26 ` [PATCH] USB: hub: Ignore non-compliant devices with too many configs or interfaces Alan Stern
2025-02-03 15:35 ` Alan Stern
2025-02-03 15:49 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=66581.1737545865@localhost \
--to=rtm@csail.mit.edu \
--cc=gregkh@linuxfoundation.org \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.