Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/netbsd-9]: src/sys/dev/usb Pull up following revision(s) (requested by m...



details:   https://anonhg.NetBSD.org/src/rev/797578a7900c
branches:  netbsd-9
changeset: 843731:797578a7900c
user:      martin <martin%NetBSD.org@localhost>
date:      Thu Jan 02 09:42:06 2020 +0000

description:
Pull up following revision(s) (requested by maxv in ticket #595):

        sys/dev/usb/uthum.c: revision 1.18
        sys/dev/usb/ucycom.c: revision 1.49
        sys/dev/usb/uhid.c: revision 1.111

Fix buffer overflows. sc_{o,f}len are controlled by the USB device. By
crafting the former the device can leak stack data. By crafting the latter
the device can overwrite the stack. The combination of the two means the
device can ROP the kernel and obtain code execution (demonstrated with an
actual exploit over vHCI).

Truncate the lengths to the size of the buffers, and also drop sc_ilen
since it is unused. Patch tested with vHCI+kASan.

 -

Fix buffer overflows. Also add missing mutex_exit.

 -

Fix buffer overflows: validate the lengths at attach time, given that they
are apparently not supposed to be variable. Drop sc_ilen since it is
unused.

diffstat:

 sys/dev/usb/ucycom.c |  23 +++++++++++++++++------
 sys/dev/usb/uhid.c   |  16 +++++++++++++---
 sys/dev/usb/uthum.c  |  20 +++++++++++---------
 3 files changed, 41 insertions(+), 18 deletions(-)

diffs (229 lines):

diff -r f9881cd4cd36 -r 797578a7900c sys/dev/usb/ucycom.c
--- a/sys/dev/usb/ucycom.c      Thu Jan 02 09:23:47 2020 +0000
+++ b/sys/dev/usb/ucycom.c      Thu Jan 02 09:42:06 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ucycom.c,v 1.47 2019/05/05 03:17:54 mrg Exp $  */
+/*     $NetBSD: ucycom.c,v 1.47.2.1 2020/01/02 09:42:06 martin Exp $   */
 
 /*
  * Copyright (c) 2005 The NetBSD Foundation, Inc.
@@ -38,7 +38,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ucycom.c,v 1.47 2019/05/05 03:17:54 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ucycom.c,v 1.47.2.1 2020/01/02 09:42:06 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -121,11 +121,15 @@
 
        struct tty              *sc_tty;
 
+       enum {
+               UCYCOM_INIT_NONE,
+               UCYCOM_INIT_INITED
+       } sc_init_state;
+
        kmutex_t sc_lock;       /* protects refcnt, others */
 
        /* uhidev parameters */
        size_t                  sc_flen; /* feature report length */
-       size_t                  sc_ilen; /* input report length */
        size_t                  sc_olen; /* output report length */
 
        uint8_t                 *sc_obuf;
@@ -219,13 +223,18 @@
        sc->sc_hdev.sc_intr = ucycom_intr;
        sc->sc_hdev.sc_parent = uha->parent;
        sc->sc_hdev.sc_report_id = uha->reportid;
+       sc->sc_init_state = UCYCOM_INIT_NONE;
 
        uhidev_get_report_desc(uha->parent, &desc, &size);
        repid = uha->reportid;
-       sc->sc_ilen = hid_report_size(desc, size, hid_input, repid);
        sc->sc_olen = hid_report_size(desc, size, hid_output, repid);
        sc->sc_flen = hid_report_size(desc, size, hid_feature, repid);
 
+       if (sc->sc_olen != 8 && sc->sc_olen != 32)
+               return;
+       if (sc->sc_flen != 5)
+               return;
+
        sc->sc_msr = sc->sc_mcr = 0;
 
        /* set up tty */
@@ -238,6 +247,8 @@
 
        /* Nothing interesting to report */
        aprint_normal("\n");
+
+       sc->sc_init_state = UCYCOM_INIT_INITED;
 }
 
 
@@ -334,10 +345,10 @@
 
        if (sc == NULL)
                return ENXIO;
-
        if (sc->sc_dying)
                return EIO;
-
+       if (sc->sc_init_state != UCYCOM_INIT_INITED)
+               return ENXIO;
        if (!device_is_active(sc->sc_hdev.sc_dev))
                return ENXIO;
 
diff -r f9881cd4cd36 -r 797578a7900c sys/dev/usb/uhid.c
--- a/sys/dev/usb/uhid.c        Thu Jan 02 09:23:47 2020 +0000
+++ b/sys/dev/usb/uhid.c        Thu Jan 02 09:42:06 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uhid.c,v 1.108 2019/05/05 03:17:54 mrg Exp $   */
+/*     $NetBSD: uhid.c,v 1.108.2.1 2020/01/02 09:42:06 martin Exp $    */
 
 /*
  * Copyright (c) 1998, 2004, 2008, 2012 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uhid.c,v 1.108 2019/05/05 03:17:54 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uhid.c,v 1.108.2.1 2020/01/02 09:42:06 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -398,6 +398,8 @@
        if (sc->sc_state & UHID_IMMED) {
                DPRINTFN(1, ("uhidread immed\n"));
                extra = sc->sc_hdev.sc_report_id != 0;
+               if (sc->sc_isize + extra > sizeof(buffer))
+                       return ENOBUFS;
                err = uhidev_get_report(&sc->sc_hdev, UHID_INPUT_REPORT,
                                        buffer, sc->sc_isize + extra);
                if (err)
@@ -541,8 +543,10 @@
        case FIOASYNC:
                mutex_enter(proc_lock);
                if (*(int *)addr) {
-                       if (sc->sc_async != NULL)
+                       if (sc->sc_async != NULL) {
+                               mutex_exit(proc_lock);
                                return EBUSY;
+                       }
                        sc->sc_async = l->l_proc;
                        DPRINTF(("uhid_do_ioctl: FIOASYNC %p\n", l->l_proc));
                } else
@@ -589,6 +593,8 @@
        case USB_SET_IMMED:
                if (*(int *)addr) {
                        extra = sc->sc_hdev.sc_report_id != 0;
+                       if (sc->sc_isize + extra > sizeof(buffer))
+                               return ENOBUFS;
                        err = uhidev_get_report(&sc->sc_hdev, UHID_INPUT_REPORT,
                                                buffer, sc->sc_isize + extra);
                        if (err)
@@ -615,6 +621,8 @@
                        return EINVAL;
                }
                extra = sc->sc_hdev.sc_report_id != 0;
+               if (size + extra > sizeof(re->ucr_data))
+                       return ENOBUFS;
                err = uhidev_get_report(&sc->sc_hdev, re->ucr_report,
                    re->ucr_data, size + extra);
                if (extra)
@@ -638,6 +646,8 @@
                default:
                        return EINVAL;
                }
+               if (size > sizeof(re->ucr_data))
+                       return ENOBUFS;
                err = uhidev_set_report(&sc->sc_hdev, re->ucr_report,
                    re->ucr_data, size);
                if (err)
diff -r f9881cd4cd36 -r 797578a7900c sys/dev/usb/uthum.c
--- a/sys/dev/usb/uthum.c       Thu Jan 02 09:23:47 2020 +0000
+++ b/sys/dev/usb/uthum.c       Thu Jan 02 09:42:06 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uthum.c,v 1.16 2019/05/05 03:17:54 mrg Exp $   */
+/*     $NetBSD: uthum.c,v 1.16.2.1 2020/01/02 09:42:06 martin Exp $   */
 /*     $OpenBSD: uthum.c,v 1.6 2010/01/03 18:43:02 deraadt Exp $   */
 
 /*
@@ -22,7 +22,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uthum.c,v 1.16 2019/05/05 03:17:54 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uthum.c,v 1.16.2.1 2020/01/02 09:42:06 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -82,7 +82,6 @@
 
        /* uhidev parameters */
        size_t                   sc_flen;       /* feature report length */
-       size_t                   sc_ilen;       /* input report length */
        size_t                   sc_olen;       /* output report length */
 
        /* sensor framework */
@@ -148,7 +147,6 @@
 
        uhidev_get_report_desc(uha->parent, &desc, &size);
        repid = uha->reportid;
-       sc->sc_ilen = hid_report_size(desc, size, hid_input, repid);
        sc->sc_olen = hid_report_size(desc, size, hid_output, repid);
        sc->sc_flen = hid_report_size(desc, size, hid_feature, repid);
 
@@ -263,33 +261,36 @@
 {
        int i;
        uint8_t cmdbuf[32], report[256];
+       size_t olen, flen;
 
        /* if return buffer is null, do nothing */
        if ((buf == NULL) || len == 0)
                return 0;
 
+       olen = uimin(sc->sc_olen, sizeof(cmdbuf));
+
        /* issue query */
        memset(cmdbuf, 0, sizeof(cmdbuf));
        memcpy(cmdbuf, cmd_start, sizeof(cmd_start));
        if (uhidev_set_report(&sc->sc_hdev, UHID_OUTPUT_REPORT,
-           cmdbuf, sc->sc_olen))
+           cmdbuf, olen))
                return EIO;
 
        memset(cmdbuf, 0, sizeof(cmdbuf));
        cmdbuf[0] = target_cmd;
        if (uhidev_set_report(&sc->sc_hdev, UHID_OUTPUT_REPORT,
-           cmdbuf, sc->sc_olen))
+           cmdbuf, olen))
                return EIO;
 
        memset(cmdbuf, 0, sizeof(cmdbuf));
        for (i = 0; i < 7; i++) {
                if (uhidev_set_report(&sc->sc_hdev, UHID_OUTPUT_REPORT,
-                   cmdbuf, sc->sc_olen))
+                   cmdbuf, olen))
                        return EIO;
        }
        memcpy(cmdbuf, cmd_end, sizeof(cmd_end));
        if (uhidev_set_report(&sc->sc_hdev, UHID_OUTPUT_REPORT,
-           cmdbuf, sc->sc_olen))
+           cmdbuf, olen))
                return EIO;
 
        /* wait if required */
@@ -297,8 +298,9 @@
                kpause("uthum", false, (need_delay*hz+999)/1000 + 1, NULL);
 
        /* get answer */
+       flen = uimin(sc->sc_flen, sizeof(report));
        if (uhidev_get_report(&sc->sc_hdev, UHID_FEATURE_REPORT,
-           report, sc->sc_flen))
+           report, flen))
                return EIO;
        memcpy(buf, report, len);
        return 0;



Home | Main Index | Thread Index | Old Index