linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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);
}

  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 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).