Mmh no I see, the min descriptor length check we should add is 3 bytes, and my
check should be moved below in the idesc branch. I'll re-fix that next week.
Le 06/07/2019 à 10:04, Maxime Villard a écrit :
Can you add printfs in these two functions to dump 'bLength'?
I've reverted the change for now. I found these bugs a week ago while
manually crafting incorrect USB packets in Qemu's USB driver. It caused
memory corruptions in the NetBSD guest, which were detected by KASAN.
Fixing the length checks eliminated the corruptions, while still
allowing correctly formed USB devices to attach.
Le 06/07/2019 à 09:54, Thomas Klausner a écrit :
It could be a coincidence, but with yesterday's kernel my
non-malicious USB keyboard (Cherry G230) worked and today it doesn't.
-uhidev0 at uhub5 port 1 configuration 1 interface 0
-uhidev0: vendor 046a (0x46a) product 0023 (0x23), rev 2.00/2.20, addr 1,
iclass 3/1
-ukbd0 at uhidev0: 8 Variable keys, 6 Array codes
-wskbd0 at ukbd0: console keyboard
-uhidev1 at uhub5 port 1 configuration 1 interface 1
-uhidev1: vendor 046a (0x46a) product 0023 (0x23), rev 2.00/2.20, addr 1,
iclass 3/0
-uhidev1: 2 report ids
-uhid0 at uhidev1 reportid 1: input=2, output=0, feature=0
-uhid1 at uhidev1 reportid 2: input=1, output=0, feature=0
+uhub5: port 1, set config at addr 1 failed
+uhub5: autoconfiguration error: device problem, disabling port 1
Thomas
On Sat, Jul 06, 2019 at 05:05:54AM +0000, Maxime Villard wrote:
Module Name: src
Committed By: maxv
Date: Sat Jul 6 05:05:53 UTC 2019
Modified Files:
src/sys/dev/usb: usb_subr.c
Log Message:
Fix two length checks, otherwise a malicious USB key plugged in the
system could trigger overflows, seen with KASAN.
To generate a diff of this commit:
cvs rdiff -u -r1.230 -r1.231 src/sys/dev/usb/usb_subr.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/usb_subr.c
diff -u src/sys/dev/usb/usb_subr.c:1.230 src/sys/dev/usb/usb_subr.c:1.231
--- src/sys/dev/usb/usb_subr.c:1.230 Tue Feb 12 14:17:44 2019
+++ src/sys/dev/usb/usb_subr.c Sat Jul 6 05:05:53 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: usb_subr.c,v 1.230 2019/02/12 14:17:44 rin Exp $ */
+/* $NetBSD: usb_subr.c,v 1.231 2019/07/06 05:05:53 maxv 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.230 2019/02/12 14:17:44 rin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.231 2019/07/06 05:05:53 maxv Exp
$");
#ifdef _KERNEL_OPT
#include "opt_compat_netbsd.h"
@@ -366,8 +366,8 @@ usbd_find_idesc(usb_config_descriptor_t
altidx, curaidx);
DPRINTFN(4, "len=%jd type=%jd", d->bLength, d->bDescriptorType,
0, 0);
- if (d->bLength == 0) /* bad descriptor */
- break;
+ if (d->bLength < USB_INTERFACE_DESCRIPTOR_SIZE)
+ break; /* bad descriptor */
p += d->bLength;
if (p <= end && d->bDescriptorType == UDESC_INTERFACE) {
if (d->bInterfaceNumber != lastidx) {
@@ -402,8 +402,8 @@ usbd_find_edesc(usb_config_descriptor_t
curidx = -1;
for (p = (char *)d + d->bLength; p < end; ) {
e = (usb_endpoint_descriptor_t *)p;
- if (e->bLength == 0) /* bad descriptor */
- break;
+ if (e->bLength < USB_ENDPOINT_DESCRIPTOR_SIZE)
+ break; /* bad descriptor */
p += e->bLength;
if (p <= end && e->bDescriptorType == UDESC_INTERFACE)
return NULL;
