USB: Fix "slab-out-of-bounds Write" bug in usb_hcd_poll_rh_status
authorAlan Stern <stern@rowland.harvard.edu>
Sat, 1 Jan 2022 02:07:12 +0000 (21:07 -0500)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 3 Jan 2022 13:38:34 +0000 (14:38 +0100)
When the USB core code for getting root-hub status reports was
originally written, it was assumed that the hub driver would be its
only caller.  But this isn't true now; user programs can use usbfs to
communicate with root hubs and get status reports.  When they do this,
they may use a transfer_buffer that is smaller than the data returned
by the HCD, which will lead to a buffer overflow error when
usb_hcd_poll_rh_status() tries to store the status data.  This was
discovered by syzbot:

BUG: KASAN: slab-out-of-bounds in memcpy include/linux/fortify-string.h:225 [inline]
BUG: KASAN: slab-out-of-bounds in usb_hcd_poll_rh_status+0x5f4/0x780 drivers/usb/core/hcd.c:776
Write of size 2 at addr ffff88801da403c0 by task syz-executor133/4062

This patch fixes the bug by reducing the amount of status data if it
won't fit in the transfer_buffer.  If some data gets discarded then
the URB's completion status is set to -EOVERFLOW rather than 0, to let
the user know what happened.

Reported-and-tested-by: syzbot+3ae6a2b06f131ab9849f@syzkaller.appspotmail.com
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/Yc+3UIQJ2STbxNua@rowland.harvard.edu
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/core/hcd.c

index 9ffc63a..3e01dd6 100644 (file)
@@ -753,6 +753,7 @@ void usb_hcd_poll_rh_status(struct usb_hcd *hcd)
 {
        struct urb      *urb;
        int             length;
+       int             status;
        unsigned long   flags;
        char            buffer[6];      /* Any root hubs with > 31 ports? */
 
@@ -770,11 +771,17 @@ void usb_hcd_poll_rh_status(struct usb_hcd *hcd)
                if (urb) {
                        clear_bit(HCD_FLAG_POLL_PENDING, &hcd->flags);
                        hcd->status_urb = NULL;
+                       if (urb->transfer_buffer_length >= length) {
+                               status = 0;
+                       } else {
+                               status = -EOVERFLOW;
+                               length = urb->transfer_buffer_length;
+                       }
                        urb->actual_length = length;
                        memcpy(urb->transfer_buffer, buffer, length);
 
                        usb_hcd_unlink_urb_from_ep(hcd, urb);
-                       usb_hcd_giveback_urb(hcd, urb, 0);
+                       usb_hcd_giveback_urb(hcd, urb, status);
                } else {
                        length = 0;
                        set_bit(HCD_FLAG_POLL_PENDING, &hcd->flags);