Hi,
While running usbhidctl on my USB mouse it occasionally fails as
follows:

        # usbhidctl -f /dev/wsmouse2 
        usbhidctl: USB_GET_REPORT (probably not supported by device): Bad 
address

The EFAULT happens during copyin(9) in sys_ioctl() while copying the
supplied usb_ctl_report structure which is declared as follows:

        struct usb_ctl_report {
                int     ucr_report;
                u_char  ucr_data[1024]; /* filled data size will vary */
        };

... and the corresponding ioctl command:

        #define USB_GET_REPORT  _IOWR('U', 23, struct usb_ctl_report)

usbhidctl tries to be memory efficient by only allocating a buffer big
enough to hold the requested report. Such report is often smaller than
1024 bytes.

However, the kernel will always copy `sizeof(struct usb_ctl_report)'
bytes from the address passed from user space. I would assume the EFAULT
happens when `addr + sizeof(struct usb_ctl_report)' crosses a page
boundary and the adjacent page is not mapped. Unconditionally allocating
the correct size fixes the problem.

Comments? OK?

Index: usbhid.c
===================================================================
RCS file: /cvs/src/usr.bin/usbhidctl/usbhid.c,v
retrieving revision 1.15
diff -u -p -r1.15 usbhid.c
--- usbhid.c    28 Jun 2019 13:35:05 -0000      1.15
+++ usbhid.c    28 Jan 2021 20:27:17 -0000
@@ -394,13 +394,7 @@ allocreport(struct Sreport *report, repo
        report->size = reptsize;
 
        if (report->size > 0) {
-               /*
-                * Allocate a buffer with enough space for the
-                * report in the variable-sized data field.
-                */
-               report->buffer = malloc(sizeof(*report->buffer) -
-                                       sizeof(report->buffer->ucr_data) +
-                                       report->size);
+               report->buffer = malloc(sizeof(*report->buffer));
                if (report->buffer == NULL)
                        err(1, NULL);
        } else

Reply via email to