Module Name: src
Committed By: riastradh
Date: Sat Mar 19 20:44:07 UTC 2022
Modified Files:
src/sys/dev/usb: umidi.c
Log Message:
umidi(4): Parse descriptors a little more robustly.
Reported-by: syzbot+fd58d1d4dd12f8931...@syzkaller.appspotmail.com
To generate a diff of this commit:
cvs rdiff -u -r1.85 -r1.86 src/sys/dev/usb/umidi.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/usb/umidi.c
diff -u src/sys/dev/usb/umidi.c:1.85 src/sys/dev/usb/umidi.c:1.86
--- src/sys/dev/usb/umidi.c:1.85 Mon Mar 14 16:14:11 2022
+++ src/sys/dev/usb/umidi.c Sat Mar 19 20:44:07 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: umidi.c,v 1.85 2022/03/14 16:14:11 riastradh Exp $ */
+/* $NetBSD: umidi.c,v 1.86 2022/03/19 20:44:07 riastradh Exp $ */
/*
* Copyright (c) 2001, 2012, 2014 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: umidi.c,v 1.85 2022/03/14 16:14:11 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: umidi.c,v 1.86 2022/03/19 20:44:07 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -70,13 +70,6 @@ __KERNEL_RCSID(0, "$NetBSD: umidi.c,v 1.
#define UMIDI_EMBEDDED 0x01
#define UMIDI_EXTERNAL 0x02
-/* generic, for iteration */
-typedef struct {
- uByte bLength;
- uByte bDescriptorType;
- uByte bDescriptorSubtype;
-} UPACKED umidi_cs_descriptor_t;
-
typedef struct {
uByte bLength;
uByte bDescriptorType;
@@ -870,58 +863,75 @@ static usbd_status
alloc_all_endpoints_yamaha(struct umidi_softc *sc)
{
/* This driver currently supports max 1in/1out bulk endpoints */
+ char *end;
+ usb_config_descriptor_t *cdesc;
usb_descriptor_t *desc;
- umidi_cs_descriptor_t *udesc;
+ usb_interface_descriptor_t *idesc;
+ umidi_cs_interface_descriptor_t *udesc;
usb_endpoint_descriptor_t *epd;
int out_addr, in_addr, i;
int dir;
- size_t remain, descsize;
sc->sc_out_num_jacks = sc->sc_in_num_jacks = 0;
out_addr = in_addr = 0;
/* detect endpoints */
- desc = TO_D(usbd_get_interface_descriptor(sc->sc_iface));
- for (i=(int)TO_IFD(desc)->bNumEndpoints-1; i>=0; i--) {
+ cdesc = usbd_get_config_descriptor(sc->sc_udev);
+ end = (char *)cdesc + UGETW(cdesc->wTotalLength);
+ idesc = usbd_get_interface_descriptor(sc->sc_iface);
+ KASSERT((char *)cdesc <= (char *)idesc);
+ KASSERT((char *)idesc < end);
+ KASSERT(end - (char *)idesc >= sizeof(*idesc));
+ KASSERT(idesc->bLength >= sizeof(*idesc));
+ KASSERT(idesc->bLength <= end - (char *)idesc);
+ for (i = idesc->bNumEndpoints; i --> 0;) {
epd = usbd_interface2endpoint_descriptor(sc->sc_iface, i);
KASSERT(epd != NULL);
if (UE_GET_XFERTYPE(epd->bmAttributes) == UE_BULK) {
dir = UE_GET_DIR(epd->bEndpointAddress);
- if (dir==UE_DIR_OUT && !out_addr)
+ if (dir == UE_DIR_OUT && !out_addr)
out_addr = epd->bEndpointAddress;
- else if (dir==UE_DIR_IN && !in_addr)
+ else if (dir == UE_DIR_IN && !in_addr)
in_addr = epd->bEndpointAddress;
}
}
- udesc = (umidi_cs_descriptor_t *)NEXT_D(desc);
+ desc = NEXT_D(idesc);
+ if ((char *)desc > end || end - (char *)desc < sizeof(*desc) ||
+ desc->bLength < sizeof(*desc) ||
+ desc->bLength > end - (char *)desc)
+ return USBD_INVAL;
/* count jacks */
- if (!(udesc->bDescriptorType==UDESC_CS_INTERFACE &&
- udesc->bDescriptorSubtype==UMIDI_MS_HEADER))
+ if (!(desc->bDescriptorType == UDESC_CS_INTERFACE &&
+ desc->bDescriptorSubtype == UMIDI_MS_HEADER))
+ return USBD_INVAL;
+ if (desc->bLength < sizeof(*udesc))
+ return USBD_INVAL;
+ udesc = TO_CSIFD(desc);
+ if (UGETW(udesc->wTotalLength) > end - (char *)udesc)
return USBD_INVAL;
- remain = (size_t)UGETW(TO_CSIFD(udesc)->wTotalLength) -
- (size_t)udesc->bLength;
- udesc = (umidi_cs_descriptor_t *)NEXT_D(udesc);
-
- while (remain >= sizeof(usb_descriptor_t)) {
- descsize = udesc->bLength;
- if (descsize>remain || descsize==0)
+ if (UGETW(udesc->wTotalLength) < udesc->bLength)
+ return USBD_INVAL;
+ end = (char *)udesc + UGETW(udesc->wTotalLength);
+ desc = NEXT_D(udesc);
+
+ for (; end - (char *)desc >= sizeof(*desc); desc = NEXT_D(desc)) {
+ if (desc->bLength < sizeof(*desc) ||
+ desc->bLength > end - (char *)desc)
break;
- if (udesc->bDescriptorType == UDESC_CS_INTERFACE &&
- remain >= UMIDI_JACK_DESCRIPTOR_SIZE) {
- if (udesc->bDescriptorSubtype == UMIDI_OUT_JACK)
+ if (desc->bDescriptorType == UDESC_CS_INTERFACE &&
+ desc->bLength >= UMIDI_JACK_DESCRIPTOR_SIZE) {
+ if (desc->bDescriptorSubtype == UMIDI_OUT_JACK)
sc->sc_out_num_jacks++;
- else if (udesc->bDescriptorSubtype == UMIDI_IN_JACK)
+ else if (desc->bDescriptorSubtype == UMIDI_IN_JACK)
sc->sc_in_num_jacks++;
}
- udesc = (umidi_cs_descriptor_t *)NEXT_D(udesc);
- remain -= descsize;
}
/* validate some parameters */
- if (sc->sc_out_num_jacks>UMIDI_MAX_EPJACKS)
+ if (sc->sc_out_num_jacks > UMIDI_MAX_EPJACKS)
sc->sc_out_num_jacks = UMIDI_MAX_EPJACKS;
- if (sc->sc_in_num_jacks>UMIDI_MAX_EPJACKS)
+ if (sc->sc_in_num_jacks > UMIDI_MAX_EPJACKS)
sc->sc_in_num_jacks = UMIDI_MAX_EPJACKS;
if (sc->sc_out_num_jacks && out_addr) {
sc->sc_out_num_endpoints = 1;
@@ -949,7 +959,7 @@ alloc_all_endpoints_yamaha(struct umidi_
sc->sc_out_ep = NULL;
if (sc->sc_in_num_endpoints) {
- sc->sc_in_ep = sc->sc_endpoints+sc->sc_out_num_endpoints;
+ sc->sc_in_ep = sc->sc_endpoints + sc->sc_out_num_endpoints;
sc->sc_in_ep->sc = sc;
sc->sc_in_ep->addr = in_addr;
sc->sc_in_ep->num_jacks = sc->sc_in_num_jacks;
@@ -966,8 +976,8 @@ alloc_all_endpoints_genuine(struct umidi
usb_interface_descriptor_t *interface_desc;
usb_config_descriptor_t *config_desc;
usb_descriptor_t *desc;
+ char *end;
int num_ep;
- size_t remain, descsize;
struct umidi_endpoint *p, *q, *lowest, *endep, tmpep;
int epaddr;
@@ -983,25 +993,25 @@ alloc_all_endpoints_genuine(struct umidi
/* get the list of endpoints for midi stream */
config_desc = usbd_get_config_descriptor(sc->sc_udev);
- desc = (usb_descriptor_t *) config_desc;
- remain = (size_t)UGETW(config_desc->wTotalLength);
- while (remain>=sizeof(usb_descriptor_t)) {
- descsize = desc->bLength;
- if (descsize>remain || descsize==0)
+ end = (char *)config_desc + UGETW(config_desc->wTotalLength);
+ desc = TO_D(config_desc);
+ for (; end - (char *)desc >= sizeof(*desc); desc = NEXT_D(desc)) {
+ if (desc->bLength < sizeof(*desc) ||
+ desc->bLength > end - (char *)desc)
break;
- if (desc->bDescriptorType==UDESC_ENDPOINT &&
- remain>=USB_ENDPOINT_DESCRIPTOR_SIZE &&
+ if (desc->bDescriptorType == UDESC_ENDPOINT &&
+ desc->bLength >= sizeof(*TO_EPD(desc)) &&
UE_GET_XFERTYPE(TO_EPD(desc)->bmAttributes) == UE_BULK) {
epaddr = TO_EPD(desc)->bEndpointAddress;
- } else if (desc->bDescriptorType==UDESC_CS_ENDPOINT &&
- remain>=UMIDI_CS_ENDPOINT_DESCRIPTOR_SIZE &&
- epaddr!=-1) {
- if (num_ep>0) {
+ } else if (desc->bDescriptorType == UDESC_CS_ENDPOINT &&
+ desc->bLength >= sizeof(*TO_CSEPD(desc)) &&
+ epaddr != -1) {
+ if (num_ep > 0) {
num_ep--;
p->sc = sc;
p->addr = epaddr;
p->num_jacks = TO_CSEPD(desc)->bNumEmbMIDIJack;
- if (UE_GET_DIR(epaddr)==UE_DIR_OUT) {
+ if (UE_GET_DIR(epaddr) == UE_DIR_OUT) {
sc->sc_out_num_endpoints++;
sc->sc_out_num_jacks += p->num_jacks;
} else {
@@ -1012,8 +1022,6 @@ alloc_all_endpoints_genuine(struct umidi
}
} else
epaddr = -1;
- desc = NEXT_D(desc);
- remain-=descsize;
}
/* sort endpoints */
