All of lore.kernel.org
 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 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.