From patchwork Wed Oct 11 22:54:08 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yuxuan Luo X-Patchwork-Id: 1847054 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4S5SlM39Q7z1yqj for ; Thu, 12 Oct 2023 09:54:55 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1qqi6D-0001Zu-AQ; Wed, 11 Oct 2023 22:54:44 +0000 Received: from smtp-relay-internal-1.internal ([10.131.114.114] helo=smtp-relay-internal-1.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1qqi5y-0001U1-CW for kernel-team@lists.ubuntu.com; Wed, 11 Oct 2023 22:54:27 +0000 Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id CFF18404A1 for ; Wed, 11 Oct 2023 22:54:25 +0000 (UTC) Received: by mail-qt1-f200.google.com with SMTP id d75a77b69052e-4199725b054so3806841cf.3 for ; Wed, 11 Oct 2023 15:54:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697064862; x=1697669662; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=d7kMjtthEP+fw53WHW5ucxJ2FCak0fU0RZGctHS5Ieo=; b=szkUHUanYvS7Zzjud/fMflmfrg41YPuQMTwyJiyMIFC9bvVeAcH8kS7TwC3Chs6svo ALSULdPSkxpHSE1I/UzS8F3C14Qmq3VGrWaA9GxXv5oLctPJ+U25LpbTTgdPsv52UDnt JFXM0rooh2Zr3puRCe5Ac3ZD4Mxv8RhYhJY3VlVrulNyv4/eo478te6zetmoTkYJg2yX e8dVhM9E7bX5Wr5jCNH+URkfUaBEx+Xm+Rbw+jaVYRpP0cb2iz5VsyxTC4NpOfikIKtc jgT2wvRIVuLqCsQnqXivucEQm7Hxekr6z9S7nxVsmcKVCTY9/Sf640IvMfvxD9iOtPcn gcVQ== X-Gm-Message-State: AOJu0YwQMOZPC2owQ2pujCmWnVKv99W3XXe/tLXStdBcX31RgmdLuYoy 0OCcaxWNMOBgbicL0WI+9I4891CEKDvjG96/6k2HLnyhE+AEmZ2vGXo/XKxwPsJ3UX9L4eeok9W 6WM7asFEB54NOw/rnQuMCGJIzaNt5oC9H+wQHi32mH6TZkrzBWw== X-Received: by 2002:a05:622a:348:b0:418:1ea6:5530 with SMTP id r8-20020a05622a034800b004181ea65530mr28462825qtw.40.1697064861951; Wed, 11 Oct 2023 15:54:21 -0700 (PDT) X-Google-Smtp-Source: AGHT+IF1JF7fvTBzPjsXgeoZPWvcmQdgiKn249O/Oc0P2vxCBluLCn8m6BLShMlorYahPP5e+ii7Fg== X-Received: by 2002:a05:622a:348:b0:418:1ea6:5530 with SMTP id r8-20020a05622a034800b004181ea65530mr28462808qtw.40.1697064861530; Wed, 11 Oct 2023 15:54:21 -0700 (PDT) Received: from cache-ubuntu.hsd1.nj.comcast.net ([2601:86:200:98b0:5308:de8d:b54e:a80e]) by smtp.gmail.com with ESMTPSA id y23-20020ac87097000000b00417db2593bdsm5691098qto.72.2023.10.11.15.54.20 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Oct 2023 15:54:21 -0700 (PDT) From: Yuxuan Luo To: kernel-team@lists.ubuntu.com Subject: [SRU][J/L][PATCH 2/3] USB: core: Change usb_get_device_descriptor() API Date: Wed, 11 Oct 2023 18:54:08 -0400 Message-Id: <20231011225414.50811-4-yuxuan.luo@canonical.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231011225414.50811-1-yuxuan.luo@canonical.com> References: <20231011225414.50811-1-yuxuan.luo@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Alan Stern The usb_get_device_descriptor() routine reads the device descriptor from the udev device and stores it directly in udev->descriptor. This interface is error prone, because the USB subsystem expects in-memory copies of a device's descriptors to be immutable once the device has been initialized. The interface is changed so that the device descriptor is left in a kmalloc-ed buffer, not copied into the usb_device structure. A pointer to the buffer is returned to the caller, who is then responsible for kfree-ing it. The corresponding changes needed in the various callers are fairly small. Signed-off-by: Alan Stern Link: https://lore.kernel.org/r/d0111bb6-56c1-4f90-adf2-6cfe152f6561@rowland.harvard.edu Signed-off-by: Greg Kroah-Hartman (cherry picked from commit de28e469da75359a2bb8cd8778b78aa64b1be1f4) CVE-2023-37453 Signed-off-by: Yuxuan Luo --- drivers/usb/core/hcd.c | 10 ++++++--- drivers/usb/core/hub.c | 44 ++++++++++++++++++++------------------ drivers/usb/core/message.c | 29 +++++++++++-------------- drivers/usb/core/usb.h | 4 ++-- 4 files changed, 44 insertions(+), 43 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 8300baedafd20..6af0a31ff1475 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -983,6 +983,7 @@ static int register_root_hub(struct usb_hcd *hcd) { struct device *parent_dev = hcd->self.controller; struct usb_device *usb_dev = hcd->self.root_hub; + struct usb_device_descriptor *descr; const int devnum = 1; int retval; @@ -994,13 +995,16 @@ static int register_root_hub(struct usb_hcd *hcd) mutex_lock(&usb_bus_idr_lock); usb_dev->ep0.desc.wMaxPacketSize = cpu_to_le16(64); - retval = usb_get_device_descriptor(usb_dev, USB_DT_DEVICE_SIZE); - if (retval != sizeof usb_dev->descriptor) { + descr = usb_get_device_descriptor(usb_dev); + if (IS_ERR(descr)) { + retval = PTR_ERR(descr); mutex_unlock(&usb_bus_idr_lock); dev_dbg (parent_dev, "can't read %s device descriptor %d\n", dev_name(&usb_dev->dev), retval); - return (retval < 0) ? retval : -EMSGSIZE; + return retval; } + usb_dev->descriptor = *descr; + kfree(descr); if (le16_to_cpu(usb_dev->descriptor.bcdUSB) >= 0x0201) { retval = usb_get_bos_descriptor(usb_dev); diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index cef98758f597e..fd66187d944ee 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2656,12 +2656,17 @@ int usb_authorize_device(struct usb_device *usb_dev) } if (usb_dev->wusb) { - result = usb_get_device_descriptor(usb_dev, sizeof(usb_dev->descriptor)); - if (result < 0) { + struct usb_device_descriptor *descr; + + descr = usb_get_device_descriptor(usb_dev); + if (IS_ERR(descr)) { + result = PTR_ERR(descr); dev_err(&usb_dev->dev, "can't re-read device descriptor for " "authorization: %d\n", result); goto error_device_descriptor; } + usb_dev->descriptor = *descr; + kfree(descr); } usb_dev->authorized = 1; @@ -4789,7 +4794,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, const char *driver_name; bool do_new_scheme; int maxp0; - struct usb_device_descriptor *buf; + struct usb_device_descriptor *buf, *descr; buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO); if (!buf) @@ -5031,15 +5036,16 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, usb_ep0_reinit(udev); } - retval = usb_get_device_descriptor(udev, USB_DT_DEVICE_SIZE); - if (retval < (signed)sizeof(udev->descriptor)) { + descr = usb_get_device_descriptor(udev); + if (IS_ERR(descr)) { + retval = PTR_ERR(descr); if (retval != -ENODEV) dev_err(&udev->dev, "device descriptor read/all, error %d\n", retval); - if (retval >= 0) - retval = -ENOMSG; goto fail; } + udev->descriptor = *descr; + kfree(descr); /* * Some superspeed devices have finished the link training process @@ -5158,7 +5164,7 @@ hub_power_remaining(struct usb_hub *hub) static int descriptors_changed(struct usb_device *udev, - struct usb_device_descriptor *old_device_descriptor, + struct usb_device_descriptor *new_device_descriptor, struct usb_host_bos *old_bos) { int changed = 0; @@ -5169,8 +5175,8 @@ static int descriptors_changed(struct usb_device *udev, int length; char *buf; - if (memcmp(&udev->descriptor, old_device_descriptor, - sizeof(*old_device_descriptor)) != 0) + if (memcmp(&udev->descriptor, new_device_descriptor, + sizeof(*new_device_descriptor)) != 0) return 1; if ((old_bos && !udev->bos) || (!old_bos && udev->bos)) @@ -5495,9 +5501,8 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1, { struct usb_port *port_dev = hub->ports[port1 - 1]; struct usb_device *udev = port_dev->child; - struct usb_device_descriptor descriptor; + struct usb_device_descriptor *descr; int status = -ENODEV; - int retval; dev_dbg(&port_dev->dev, "status %04x, change %04x, %s\n", portstatus, portchange, portspeed(hub, portstatus)); @@ -5524,23 +5529,20 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1, * changed device descriptors before resuscitating the * device. */ - descriptor = udev->descriptor; - retval = usb_get_device_descriptor(udev, - sizeof(udev->descriptor)); - if (retval < 0) { + descr = usb_get_device_descriptor(udev); + if (IS_ERR(descr)) { dev_dbg(&udev->dev, - "can't read device descriptor %d\n", - retval); + "can't read device descriptor %ld\n", + PTR_ERR(descr)); } else { - if (descriptors_changed(udev, &descriptor, + if (descriptors_changed(udev, descr, udev->bos)) { dev_dbg(&udev->dev, "device descriptor has changed\n"); - /* for disconnect() calls */ - udev->descriptor = descriptor; } else { status = 0; /* Nothing to do */ } + kfree(descr); } #ifdef CONFIG_PM } else if (udev->state == USB_STATE_SUSPENDED && diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 127fac1af6767..ca5526b767be8 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1040,40 +1040,35 @@ char *usb_cache_string(struct usb_device *udev, int index) EXPORT_SYMBOL_GPL(usb_cache_string); /* - * usb_get_device_descriptor - (re)reads the device descriptor (usbcore) - * @dev: the device whose device descriptor is being updated - * @size: how much of the descriptor to read + * usb_get_device_descriptor - read the device descriptor + * @udev: the device whose device descriptor should be read * * Context: task context, might sleep. * - * Updates the copy of the device descriptor stored in the device structure, - * which dedicates space for this purpose. - * * Not exported, only for use by the core. If drivers really want to read * the device descriptor directly, they can call usb_get_descriptor() with * type = USB_DT_DEVICE and index = 0. * - * This call is synchronous, and may not be used in an interrupt context. - * - * Return: The number of bytes received on success, or else the status code - * returned by the underlying usb_control_msg() call. + * Returns: a pointer to a dynamically allocated usb_device_descriptor + * structure (which the caller must deallocate), or an ERR_PTR value. */ -int usb_get_device_descriptor(struct usb_device *dev, unsigned int size) +struct usb_device_descriptor *usb_get_device_descriptor(struct usb_device *udev) { struct usb_device_descriptor *desc; int ret; - if (size > sizeof(*desc)) - return -EINVAL; desc = kmalloc(sizeof(*desc), GFP_NOIO); if (!desc) - return -ENOMEM; + return ERR_PTR(-ENOMEM); + + ret = usb_get_descriptor(udev, USB_DT_DEVICE, 0, desc, sizeof(*desc)); + if (ret == sizeof(*desc)) + return desc; - ret = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, size); if (ret >= 0) - memcpy(&dev->descriptor, desc, size); + ret = -EMSGSIZE; kfree(desc); - return ret; + return ERR_PTR(ret); } /* diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index 0eac7d4285d14..907d13cb8733d 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -42,8 +42,8 @@ extern bool usb_endpoint_is_ignored(struct usb_device *udev, struct usb_endpoint_descriptor *epd); extern int usb_remove_device(struct usb_device *udev); -extern int usb_get_device_descriptor(struct usb_device *dev, - unsigned int size); +extern struct usb_device_descriptor *usb_get_device_descriptor( + struct usb_device *udev); extern int usb_set_isoch_delay(struct usb_device *dev); extern int usb_get_bos_descriptor(struct usb_device *dev); extern void usb_release_bos_descriptor(struct usb_device *dev);