USB hub code can dereference NULL hub and hub->ports

USB hub code can dereference NULL hub and hub->ports

[rtm]

[-- Attachment #1: Type: text/plain, Size: 4176 bytes --]

The attached program, which acts via usbip as a USB device or hub,
causes my linux machines to dereference some NULL pointers in
drivers/usb/core/hub.c. These are places where udev->maxchild > 0, but
either usb_hub_to_struct_hub(udev) returns NULL, or the returned hub
has hub->ports == NULL.

This is one such place:

static void recursively_mark_NOTATTACHED(struct usb_device *udev)
{
        struct usb_hub *hub = usb_hub_to_struct_hub(udev);
        int i;

        for (i = 0; i < udev->maxchild; ++i) {
                if (hub->ports[i]->child)

And this:

static void hub_disconnect_children(struct usb_device *udev)
{
        struct usb_hub *hub = usb_hub_to_struct_hub(udev);
        int i;

        /* Free up all the children before we remove this device */
        for (i = 0; i < udev->maxchild; i++) {
                if (hub->ports[i]->child)

This can see NULL hub->ports:

void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
                struct usb_hub_descriptor *desc)
{
        struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
        enum usb_port_connect_type connect_type;
        int i;

        if (!hub)
                return;

        if (!hub_is_superspeed(hdev)) {
                for (i = 1; i <= hdev->maxchild; i++) {
                        struct usb_port *port_dev = hub->ports[i - 1];

This can see a NULL hub:

static int hub_set_address(struct usb_device *udev, int devnum)
{
        int retval;
        unsigned int timeout_ms = USB_CTRL_SET_TIMEOUT;
        struct usb_hcd *hcd = bus_to_hcd(udev->bus);
        struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);

        if (hub->hdev->quirks & USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT)

I've attached a demo that runs into some of these NULL dereferences.
It depends on being able to run usbip (and modeprobe vhci-hcd).

# uname -a
Linux xxx 6.13.0-rc3-00017-gf44d154d6e3d #14 SMP Mon Jan 20 04:52:59 EST 2025 x86_64 x86_64 x86_64 GNU/Linux
# cc usbhub11b.c
# ./a.out
...
hub 1-1:1.16: bad descriptor, ignoring hub
hub 1-1:1.16: probe with driver hub failed with error -5
BUG: kernel NULL pointer dereference, address: 0000000000000250
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: Oops: 0000 [#1] SMP DEBUG_PAGEALLOC PTI
CPU: 8 UID: 0 PID: 302 Comm: kworker/8: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:recursively_mark_NOTATTACHED+0x37/0x90
Code: 48 85 ff 74 71 4c 8b a7 18 04 00 00 4d 85 e4 74 13 85 c0 74 3a 49 8b 94 24
 98 00 00 00 4c 8b a2 c8 00 00 00 85 c0 7e 27 31 db <49> 8b 84 24 50 02 00 00 48
 8b 04 d8 48 8b 38 48 85 ff 74 05 e8 b0
RSP: 0018:ffffc9000096bd28 EFLAGS: 00010046
RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffff888106224c00
RDX: ffff8881037bc000 RSI: 0000000000000007 RDI: ffff88810a502000
RBP: ffff88810a502000 R08: 000000000000005a R09: 0000000000000000
R10: ffff88810a502000 R11: ffff88810a502000 R12: 0000000000000000
R13: ffff88810a502000 R14: ffff8881062241f0 R15: 0000000000000001
FS:  0000000000000000(0000) GS:ffff88842dc00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000250 CR3: 0000000003636001 CR4: 00000000003706f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 ? __die+0x1e/0x60
 ? page_fault_oops+0x157/0x450
 ? set_track_prepare+0x3b/0x60
 ? usb_control_msg+0xfd/0x150
 ? check_bytes_and_report.isra.0+0x48/0x120
 ? exc_page_fault+0x66/0x140
 ? asm_exc_page_fault+0x26/0x30
 ? recursively_mark_NOTATTACHED+0x37/0x90
 ? usb_control_msg+0xfd/0x150
 usb_disconnect+0x37/0x2c0
 hub_event+0xc8f/0x1870
 usb_disconnect+0x37/0x2c0
 hub_event+0xc8f/0x1870
 ? trace_event_raw_event_sched_switch+0x51/0x150
 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
 </TASK>

Robert Morris
rtm@mit.edu



[-- Attachment #2: usbhub11b.c --]
[-- Type: application/octet-stream, Size: 18361 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>

int symcmd = 5;
int symcmd2 = -1;
int sym_all_scsi = 0;
int sym_scsi_cmd = -1;

unsigned long aa[] = {
0x1000000000ull,
0x10000000ull,
0x25000000ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
0x0ull,
};
int aai;

static inline unsigned long real_symx() {
  return aa[aai++];
}

static int started = 0;
static unsigned long symx() {
#define NSYM 64
  static unsigned long sa[NSYM];
  static int sai = 0;
  if(started == 0){
    started = 1;
    usleep(200000);
    for(int i = 0; i < NSYM; i++){
      sa[i] = real_symx();
    }
  }
  if(sai >= NSYM){
    printf("!!! ran out of SYM\n");
    return 0;
  }
  return sa[sai++];
}

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;

  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("modprobe vhci-hcd");

  // 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] = 0x10; // bcdUSB
            buf64[3] = 0x02; // bcdUSB
            buf64[4] = 9; // bDeviceClass
            buf64[5] = 0; // bDeviceSubClass
            buf64[6] = 2; // bDeviceProtocol
            buf64[7] = 64; // bMaxPacketSize0
            *(short*)(buf64+8) = 0x2109; // idVendor VIA Labs
            *(short*)(buf64+10) = 0x2822; // idProduct Hub
            //*(short*)(buf64+8) = 0x1a40; // idVendor Terminus Technology
            //*(short*)(buf64+10) = 0x0101; // idProduct Hub
            buf64[12] = 0xa3; // bcdDevice
            buf64[13] = 1; // bcdDevice
            buf64[14] = 1; // iManufacturer
            buf64[15] = 2; // 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++ = 1; // bNumInterfaces
            *p++ = 1; // bConfigurationValue
            *p++ = 0; // iConfiguration
            *p++ = 0xe0; // bmAttributes
            *p++ = 1; // bMaxPower

            // interface 0
            mkif(&p, 0, 0, 1, 9, 0, 1, 0);
            mkep(&p, 0x81, 3, 1);

            // interface 0 alt 1
            mkif(&p, 0, 1, 1, 9, 0, 2, 0);
            mkep(&p, 0x81, 3, 1);

            // Binary Object Store Descriptor
            *p++ = 5; // bLength
            *p++ = 15; // bDescriptorType
            *(short*)p = 0x49; // wTotalLength
            p += 2;
            *p++ = 3; // bNumDeviceCaps

            // USB 2.0 Extension Device Capability
            *p++ = 7;
            *p++ = 16;
            *p++ = 2; // bDevCapabilityType
            *(int*)p = 6; // bmAttributes
            p += 4;

            // SuperSpeed USB Device Capability
            *p++ = 10;
            *p++ = 16;
            *p++ = 3; // bDevCapabilityType
            *p++ = 0; // bmAttributes
            *(short*)p = 0x000e; // wSpeedsSupported
            p += 2;
            *p++ = 1; // bFunctionalitySupport
            *p++ = 4;
            *p++ = 231;
            *p++ = 0; // ???
            
            // Container ID Device Capability
            *p++ = 20;
            *p++ = 16;
            *p++ = 4;
            *p++ = 0;
            p += 16;

            // SuperSpeedPlus USB Device Capability
            *p++ = 28;
            *p++ = 16;
            *p++ = 10;
            *(int*)p = 0x23;
            p += 4;
            *(short*)p = 0x1100;
            p += 2;
            *(int*)p = 0x50030;
            p += 4;
            *(int*)p = 0x500b0;
            p += 4;
            *(int*)p = 0xa4031;
            p += 4;
            *(int*)p = 0xa40b1;
            p += 4;
            p += 3; // ???

            // Hub Descriptor
            *p++ = 9; // bLength
            *p++ = 41; // bDescriptorType
            *p++ = 4; // nNbrPorts
            *(short*)p = 0xe9; // wHubCharacteristic
            p += 2;
            *p++ = 50; // bPwrOn2PwrGood
            *p++ = 1; // bHubContrCurrent
            *p++ = 0; // DeviceRemovable
            *p++ = 0xff; // PortPwrCtrlMask

            // Device Qualifier
            *p++ = 10; // bLength
            *p++ = 6; // bDescriptorType
            *p++ = 0; // bcdUSB 
            *p++ = 2;
            *p++ = 9; // bDeviceClass
            *p++ = 0; // bDeviceSubClass
            *p++ = 0; // bDeviceProtocol
            *p++ = 64; // bMaxPacketSize0
            *p++ = 1; // bNumConfigurations
            *p++ = 1; // ???

            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
            memset(buf64, 0xff, 32);
            buf64[0] = 15; // 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
          memset(buf64, 0xff, 4);
        } else if(cs.setup[1] == 0 && cs.setup[0] == 0xa3){
          // USB_REQ_GET_STATUS USB_RT_PORT ???
          // usb_port_status ???
          memset(buf64, 0xff, 8);
        } else if(cs.setup[1] == 0 && cs.setup[0] == 0){
          // ???
          memset(buf64, 0xff, 8);
        } 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
          }
          if(sym_all_scsi > 0 || sym_scsi_cmd == CDB[0]){
            for(int i = 0; i < 64 && i < translen; i += 8){
              *(long*)(buf64 + i) ^= symx();
            }
            if(CDB[0] == 0x12){
              // INQUIRY
              buf64[4] = 36 - 5; // response_len
            }
          }
        }
        if(cmdno == symcmd || cmdno == symcmd2){
          for(int i = 0; i < 64 && i+8 <= sizeof(buf64); i += 8){
            *(long*)(buf64 + i) ^= symx();
          }
        }
        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.seqnum = uh;
      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 >= 25)
      break;
      
    cmdno += 1;
  }
    
  usleep(500000);
  close(s1);
  usleep(500000);
}

Re: USB hub code can dereference NULL hub and hub->ports

[Greg KH]

On Mon, Jan 20, 2025 at 12:27:19PM -0500, rtm@csail.mit.edu wrote:
> The attached program, which acts via usbip as a USB device or hub,
> causes my linux machines to dereference some NULL pointers in
> drivers/usb/core/hub.c. These are places where udev->maxchild > 0, but
> either usb_hub_to_struct_hub(udev) returns NULL, or the returned hub
> has hub->ports == NULL.
> 
> This is one such place:
> 
> static void recursively_mark_NOTATTACHED(struct usb_device *udev)
> {
>         struct usb_hub *hub = usb_hub_to_struct_hub(udev);
>         int i;
> 
>         for (i = 0; i < udev->maxchild; ++i) {
>                 if (hub->ports[i]->child)
> 
> And this:
> 
> static void hub_disconnect_children(struct usb_device *udev)
> {
>         struct usb_hub *hub = usb_hub_to_struct_hub(udev);
>         int i;
> 
>         /* Free up all the children before we remove this device */
>         for (i = 0; i < udev->maxchild; i++) {
>                 if (hub->ports[i]->child)
> 
> This can see NULL hub->ports:
> 
> void usb_hub_adjust_deviceremovable(struct usb_device *hdev,
>                 struct usb_hub_descriptor *desc)
> {
>         struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
>         enum usb_port_connect_type connect_type;
>         int i;
> 
>         if (!hub)
>                 return;
> 
>         if (!hub_is_superspeed(hdev)) {
>                 for (i = 1; i <= hdev->maxchild; i++) {
>                         struct usb_port *port_dev = hub->ports[i - 1];
> 
> This can see a NULL hub:
> 
> static int hub_set_address(struct usb_device *udev, int devnum)
> {
>         int retval;
>         unsigned int timeout_ms = USB_CTRL_SET_TIMEOUT;
>         struct usb_hcd *hcd = bus_to_hcd(udev->bus);
>         struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
> 
>         if (hub->hdev->quirks & USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT)
> 
> I've attached a demo that runs into some of these NULL dereferences.
> It depends on being able to run usbip (and modeprobe vhci-hcd).

Great, can you submit patches to fix these issues now that you have a
reliable test program to verify the problem?

thanks,

greg k-h

Re: USB hub code can dereference NULL hub and hub->ports

[rtm]

[-- 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);
}

Re: USB hub code can dereference NULL hub and hub->ports

[Alan Stern]

On Wed, Jan 22, 2025 at 06:37:45AM -0500, rtm@csail.mit.edu wrote:
> > 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.

Okay, that explains the problem.  usb_hub_to_struct_hub() assumes that a 
hub device will never have more than one interface, because that 
requirement is in the USB spec.  But of course, a bogus or malicious 
device can violate the requirement.

I think the best way to deal with this issue is to prevent the hub 
driver from binding to non-compliant devices.  Does the patch below fix 
the problem for you?

Alan Stern



Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -1848,6 +1848,17 @@ static int hub_probe(struct usb_interfac
 	hdev = interface_to_usbdev(intf);
 
 	/*
+	 * The USB 2.0 spec prohibits hubs from having more than one
+	 * configuration or interface, and we rely on this prohibition.
+	 * Refuse to accept a device that violates it.
+	 */
+	if (hdev->descriptor.bNumConfigurations > 1 ||
+			hdev->actconfig->desc.bNumInterfaces > 1) {
+		dev_err(&intf->dev, "Invalid hub with more than one config or interface\n");
+		return -EINVAL;
+	}
+
+	/*
 	 * Set default autosuspend delay as 0 to speedup bus suspend,
 	 * based on the below considerations:
 	 *

Re: USB hub code can dereference NULL hub and hub->ports

[rtm]

Alan,

Yes, your patch makes the NULL hub and hub->ports crashes
I've seen go away!

Robert

> Date: Wed, 22 Jan 2025 10:55:24 -0500
> From: Alan Stern <stern@rowland.harvard.edu>
> To: rtm@csail.mit.edu
> Cc: Greg KH <gregkh@linuxfoundation.org>, linux-usb@vger.kernel.org
> Subject: Re: USB hub code can dereference NULL hub and hub->ports
> 
> On Wed, Jan 22, 2025 at 06:37:45AM -0500, rtm@csail.mit.edu wrote:
> > > 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.
> 
> Okay, that explains the problem.  usb_hub_to_struct_hub() assumes that a 
> hub device will never have more than one interface, because that 
> requirement is in the USB spec.  But of course, a bogus or malicious 
> device can violate the requirement.
> 
> I think the best way to deal with this issue is to prevent the hub 
> driver from binding to non-compliant devices.  Does the patch below fix 
> the problem for you?
> 
> Alan Stern
> 
> 
> 
> Index: usb-devel/drivers/usb/core/hub.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/hub.c
> +++ usb-devel/drivers/usb/core/hub.c
> @@ -1848,6 +1848,17 @@ static int hub_probe(struct usb_interfac
>  	hdev = interface_to_usbdev(intf);
>  
>  	/*
> +	 * The USB 2.0 spec prohibits hubs from having more than one
> +	 * configuration or interface, and we rely on this prohibition.
> +	 * Refuse to accept a device that violates it.
> +	 */
> +	if (hdev->descriptor.bNumConfigurations > 1 ||
> +			hdev->actconfig->desc.bNumInterfaces > 1) {
> +		dev_err(&intf->dev, "Invalid hub with more than one config or interface\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
>  	 * Set default autosuspend delay as 0 to speedup bus suspend,
>  	 * based on the below considerations:
>  	 *

[PATCH] USB: hub: Ignore non-compliant devices with too many configs or interfaces

[Alan Stern]

Robert Morris created a test program which can cause
usb_hub_to_struct_hub() to dereference a NULL or inappropriate
pointer:

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-gf44d154d6e3d #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

The cause of this error is that the device has two interfaces, and the
hub driver binds to interface 1 instead of interface 0, which is where
usb_hub_to_struct_hub() looks.

We can prevent the problem from occurring by refusing to accept hub
devices that violate the USB spec by having more than one
configuration or interface.

Reported-and-tested-by: Robert Morris <rtm@csail.mit.edu>
Closes: https://lore.kernel.org/linux-usb/95564.1737394039@localhost/
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

---

 drivers/usb/core/hub.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -1848,6 +1848,17 @@ static int hub_probe(struct usb_interfac
 	hdev = interface_to_usbdev(intf);
 
 	/*
+	 * The USB 2.0 spec prohibits hubs from having more than one
+	 * configuration or interface, and we rely on this prohibition.
+	 * Refuse to accept a device that violates it.
+	 */
+	if (hdev->descriptor.bNumConfigurations > 1 ||
+			hdev->actconfig->desc.bNumInterfaces > 1) {
+		dev_err(&intf->dev, "Invalid hub with more than one config or interface\n");
+		return -EINVAL;
+	}
+
+	/*
 	 * Set default autosuspend delay as 0 to speedup bus suspend,
 	 * based on the below considerations:
 	 *

Re: [PATCH] USB: hub: Ignore non-compliant devices with too many configs or interfaces

[Alan Stern]

On Wed, Jan 22, 2025 at 02:26:17PM -0500, Alan Stern wrote:
> Robert Morris created a test program which can cause
> usb_hub_to_struct_hub() to dereference a NULL or inappropriate
> pointer:
> 
> 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-gf44d154d6e3d #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
> 
> The cause of this error is that the device has two interfaces, and the
> hub driver binds to interface 1 instead of interface 0, which is where
> usb_hub_to_struct_hub() looks.
> 
> We can prevent the problem from occurring by refusing to accept hub
> devices that violate the USB spec by having more than one
> configuration or interface.
> 
> Reported-and-tested-by: Robert Morris <rtm@csail.mit.edu>
> Closes: https://lore.kernel.org/linux-usb/95564.1737394039@localhost/
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> 
> ---

Greg:

I originally didn't mark this patch for the -stable kernels because it 
doesn't really fix a bug.  But on the other hand, it does protect 
against a possible DOS attack from a malicious device, so perhaps it 
does belong in the -stable kernels.

I'll leave the decision up to you.

Alan Stern

> 
>  drivers/usb/core/hub.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Index: usb-devel/drivers/usb/core/hub.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/hub.c
> +++ usb-devel/drivers/usb/core/hub.c
> @@ -1848,6 +1848,17 @@ static int hub_probe(struct usb_interfac
>  	hdev = interface_to_usbdev(intf);
>  
>  	/*
> +	 * The USB 2.0 spec prohibits hubs from having more than one
> +	 * configuration or interface, and we rely on this prohibition.
> +	 * Refuse to accept a device that violates it.
> +	 */
> +	if (hdev->descriptor.bNumConfigurations > 1 ||
> +			hdev->actconfig->desc.bNumInterfaces > 1) {
> +		dev_err(&intf->dev, "Invalid hub with more than one config or interface\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
>  	 * Set default autosuspend delay as 0 to speedup bus suspend,
>  	 * based on the below considerations:
>  	 *

Re: [PATCH] USB: hub: Ignore non-compliant devices with too many configs or interfaces

[Greg KH]

On Mon, Feb 03, 2025 at 10:35:42AM -0500, Alan Stern wrote:
> On Wed, Jan 22, 2025 at 02:26:17PM -0500, Alan Stern wrote:
> > Robert Morris created a test program which can cause
> > usb_hub_to_struct_hub() to dereference a NULL or inappropriate
> > pointer:
> > 
> > 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-gf44d154d6e3d #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
> > 
> > The cause of this error is that the device has two interfaces, and the
> > hub driver binds to interface 1 instead of interface 0, which is where
> > usb_hub_to_struct_hub() looks.
> > 
> > We can prevent the problem from occurring by refusing to accept hub
> > devices that violate the USB spec by having more than one
> > configuration or interface.
> > 
> > Reported-and-tested-by: Robert Morris <rtm@csail.mit.edu>
> > Closes: https://lore.kernel.org/linux-usb/95564.1737394039@localhost/
> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > 
> > ---
> 
> Greg:
> 
> I originally didn't mark this patch for the -stable kernels because it 
> doesn't really fix a bug.  But on the other hand, it does protect 
> against a possible DOS attack from a malicious device, so perhaps it 
> does belong in the -stable kernels.
> 
> I'll leave the decision up to you.

We need to protect from malicious USB devices as we have a whole history
of that being an exploit vector for systems, with the most recent ones
happening just a few months ago, allowing anyone full access to a locked
Android device by just plugging in a device with some specially crafted
USB configuration headers.

So let's try our best here where we can.  Ideally the Android exploit
shouldn't have even attempted to read the bad headers at the driver
level, but this fix is at the hub level where I think all userspace
"don't bind a driver unless we know it is ok" seem to ignore, so it's a
good fix.

thanks,

greg k-h