Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/usb usb: Parse descriptors a little more robustly.
details: https://anonhg.NetBSD.org/src/rev/b01ed1f7fb28
branches: trunk
changeset: 363472:b01ed1f7fb28
user: riastradh <riastradh%NetBSD.org@localhost>
date: Sun Mar 13 11:30:12 2022 +0000
description:
usb: Parse descriptors a little more robustly.
- Avoid reading past the end in the event of bogus bLength.
- Avoid arithmetic overflow by rearranging inequalities.
Reported-by: syzbot+511227c050a2f164e34c%syzkaller.appspotmail.com@localhost
diffstat:
sys/dev/usb/usb_subr.c | 43 ++++++++++++++++++++++++-------------------
sys/dev/usb/usbdi.c | 11 ++++++-----
sys/dev/usb/usbdi_util.c | 16 +++++++++-------
3 files changed, 39 insertions(+), 31 deletions(-)
diffs (211 lines):
diff -r 15338634c20c -r b01ed1f7fb28 sys/dev/usb/usb_subr.c
--- a/sys/dev/usb/usb_subr.c Sun Mar 13 11:30:04 2022 +0000
+++ b/sys/dev/usb/usb_subr.c Sun Mar 13 11:30:12 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: usb_subr.c,v 1.271 2022/03/13 11:28:42 riastradh Exp $ */
+/* $NetBSD: usb_subr.c,v 1.272 2022/03/13 11:30:12 riastradh Exp $ */
/* $FreeBSD: src/sys/dev/usb/usb_subr.c,v 1.18 1999/11/17 22:33:47 n_hibma Exp $ */
/*
@@ -32,7 +32,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.271 2022/03/13 11:28:42 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.272 2022/03/13 11:30:12 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_compat_netbsd.h"
@@ -326,7 +326,7 @@
usb_interface_descriptor_t *idesc;
int curidx, lastidx, curaidx = 0;
- for (curidx = lastidx = -1; p < end; ) {
+ for (curidx = lastidx = -1; end - p >= sizeof(*desc);) {
desc = (usb_descriptor_t *)p;
DPRINTFN(4, "idx=%jd(%jd) altidx=%jd(%jd)", ifaceidx, curidx,
@@ -336,15 +336,15 @@
if (desc->bLength < USB_DESCRIPTOR_SIZE)
break;
+ if (desc->bLength > end - p)
+ break;
p += desc->bLength;
- if (p > end)
- break;
if (desc->bDescriptorType != UDESC_INTERFACE)
continue;
+ if (desc->bLength < USB_INTERFACE_DESCRIPTOR_SIZE)
+ break;
idesc = (usb_interface_descriptor_t *)desc;
- if (idesc->bLength < USB_INTERFACE_DESCRIPTOR_SIZE)
- break;
if (idesc->bInterfaceNumber != lastidx) {
lastidx = idesc->bInterfaceNumber;
@@ -378,23 +378,23 @@
return NULL;
curidx = -1;
- for (p = (char *)idesc + idesc->bLength; p < end; ) {
+ for (p = (char *)idesc + idesc->bLength; end - p >= sizeof(*edesc);) {
desc = (usb_descriptor_t *)p;
if (desc->bLength < USB_DESCRIPTOR_SIZE)
break;
+ if (desc->bLength > end - p)
+ break;
p += desc->bLength;
- if (p > end)
- break;
if (desc->bDescriptorType == UDESC_INTERFACE)
break;
if (desc->bDescriptorType != UDESC_ENDPOINT)
continue;
+ if (desc->bLength < USB_ENDPOINT_DESCRIPTOR_SIZE)
+ break;
edesc = (usb_endpoint_descriptor_t *)desc;
- if (edesc->bLength < USB_ENDPOINT_DESCRIPTOR_SIZE)
- break;
curidx++;
if (curidx == endptidx)
@@ -553,23 +553,26 @@
p = (char *)idesc + idesc->bLength;
end = (char *)dev->ud_cdesc + UGETW(dev->ud_cdesc->wTotalLength);
+ KASSERTMSG((char *)dev->ud_cdesc <= (char *)idesc, "cdesc=%p idesc=%p",
+ dev->ud_cdesc, idesc);
+ KASSERTMSG((char *)idesc < end, "idesc=%p end=%p", idesc, end);
#define ed ((usb_endpoint_descriptor_t *)p)
for (endpt = 0; endpt < nendpt; endpt++) {
DPRINTFN(10, "endpt=%jd", endpt, 0, 0, 0);
- for (; p < end; p += ed->bLength) {
+ for (; end - p >= sizeof(*ed); p += ed->bLength) {
DPRINTFN(10, "p=%#jx end=%#jx len=%jd type=%jd",
(uintptr_t)p, (uintptr_t)end, ed->bLength,
ed->bDescriptorType);
- if (p + ed->bLength <= end &&
- ed->bLength >= USB_ENDPOINT_DESCRIPTOR_SIZE &&
+ if (ed->bLength < sizeof(*ed) ||
+ ed->bLength > end - p ||
+ ed->bDescriptorType == UDESC_INTERFACE)
+ break;
+ if (ed->bLength >= USB_ENDPOINT_DESCRIPTOR_SIZE &&
ed->bDescriptorType == UDESC_ENDPOINT)
goto found;
- if (ed->bLength == 0 ||
- ed->bDescriptorType == UDESC_INTERFACE)
- break;
}
/* passed end, or bad desc */
- if (p < end) {
+ if (end - p >= sizeof(*ed)) {
if (ed->bLength == 0) {
printf("%s: bad descriptor: 0 length\n",
__func__);
@@ -607,6 +610,8 @@
}
endpoints[endpt].ue_refcnt = 0;
endpoints[endpt].ue_toggle = 0;
+ KASSERTMSG(end - p >= ed->bLength, "p=%p end=%p length=%u",
+ p, end, ed->bLength);
p += ed->bLength;
}
#undef ed
diff -r 15338634c20c -r b01ed1f7fb28 sys/dev/usb/usbdi.c
--- a/sys/dev/usb/usbdi.c Sun Mar 13 11:30:04 2022 +0000
+++ b/sys/dev/usb/usbdi.c Sun Mar 13 11:30:12 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: usbdi.c,v 1.236 2022/03/13 11:29:01 riastradh Exp $ */
+/* $NetBSD: usbdi.c,v 1.237 2022/03/13 11:30:13 riastradh Exp $ */
/*
* Copyright (c) 1998, 2012, 2015 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.236 2022/03/13 11:29:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.237 2022/03/13 11:30:13 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -976,10 +976,11 @@
usb_interface_descriptor_t *d;
int n;
- for (n = 0; p < end; p += d->bLength) {
+ for (n = 0; end - p >= sizeof(*d); p += d->bLength) {
d = (usb_interface_descriptor_t *)p;
- if (p + d->bLength <= end &&
- d->bDescriptorType == UDESC_INTERFACE &&
+ if (d->bLength < sizeof(*d) || d->bLength > end - p)
+ break;
+ if (d->bDescriptorType == UDESC_INTERFACE &&
d->bInterfaceNumber == ifaceno)
n++;
}
diff -r 15338634c20c -r b01ed1f7fb28 sys/dev/usb/usbdi_util.c
--- a/sys/dev/usb/usbdi_util.c Sun Mar 13 11:30:04 2022 +0000
+++ b/sys/dev/usb/usbdi_util.c Sun Mar 13 11:30:12 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: usbdi_util.c,v 1.84 2020/06/16 17:25:56 maxv Exp $ */
+/* $NetBSD: usbdi_util.c,v 1.85 2022/03/13 11:30:13 riastradh Exp $ */
/*
* Copyright (c) 1998, 2012 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbdi_util.c,v 1.84 2020/06/16 17:25:56 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi_util.c,v 1.85 2022/03/13 11:30:13 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -599,10 +599,11 @@
p = (char *)idesc + idesc->bLength;
end = (char *)cdesc + UGETW(cdesc->wTotalLength);
- for (; p < end; p += hd->bLength) {
+ for (; end - p >= sizeof(*hd); p += hd->bLength) {
hd = (usb_hid_descriptor_t *)p;
- if (p + hd->bLength <= end &&
- hd->bLength >= USB_HID_DESCRIPTOR_SIZE(0) &&
+ if (hd->bLength < sizeof(*hd) || hd->bLength > end - p)
+ break;
+ if (hd->bLength >= USB_HID_DESCRIPTOR_SIZE(0) &&
hd->bDescriptorType == UDESC_HID)
return hd;
if (hd->bDescriptorType == UDESC_INTERFACE)
@@ -725,7 +726,7 @@
{
const usb_descriptor_t *desc;
- if (iter->cur + sizeof(usb_descriptor_t) > iter->end) {
+ if (iter->end - iter->cur < sizeof(usb_descriptor_t)) {
if (iter->cur != iter->end)
printf("%s: bad descriptor\n", __func__);
return NULL;
@@ -735,7 +736,7 @@
printf("%s: descriptor length too small\n", __func__);
return NULL;
}
- if (iter->cur + desc->bLength > iter->end) {
+ if (desc->bLength > iter->end - iter->cur) {
printf("%s: descriptor length too large\n", __func__);
return NULL;
}
@@ -748,6 +749,7 @@
const usb_descriptor_t *desc = usb_desc_iter_peek(iter);
if (desc == NULL)
return NULL;
+ KASSERT(desc->bLength <= iter->end - iter->cur);
iter->cur += desc->bLength;
return desc;
}
Home |
Main Index |
Thread Index |
Old Index