On Wed, 10 May 2017, Johan Hovold wrote:
> Add missing sanity check on the non-SuperSpeed hub-descriptor length in
> order to avoid parsing and leaking two bytes of uninitialised slab data
> through sysfs removable-attributes (or a compound-device debug
> statement).
>
> Note that we only make sure that the DeviceRemovable field is always
> present (and specifically ignore the unused PortPwrCtrlMask field) in
> order to continue support any hubs with non-compliant descriptors. As a
> further safeguard, the descriptor buffer is also cleared.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable <stable@xxxxxxxxxxxxxxx> # 2.6.12
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---
> drivers/usb/core/hub.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 3ff1e9f89f2d..f77a4ebde7d5 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -362,7 +362,8 @@ static void usb_set_lpm_parameters(struct usb_device *udev)
> }
>
> /* USB 2.0 spec Section 11.24.4.5 */
> -static int get_hub_descriptor(struct usb_device *hdev, void *data)
> +static int get_hub_descriptor(struct usb_device *hdev,
> + struct usb_hub_descriptor *desc)
> {
> int i, ret, size;
> unsigned dtype;
> @@ -378,12 +379,16 @@ static int get_hub_descriptor(struct usb_device *hdev, void *data)
> for (i = 0; i < 3; i++) {
> ret = usb_control_msg(hdev, usb_rcvctrlpipe(hdev, 0),
> USB_REQ_GET_DESCRIPTOR, USB_DIR_IN | USB_RT_HUB,
> - dtype << 8, 0, data, size,
> + dtype << 8, 0, desc, size,
> USB_CTRL_GET_TIMEOUT);
> if (hub_is_superspeed(hdev)) {
> if (ret == size)
> return ret;
> - } else if (ret >= (USB_DT_HUB_NONVAR_SIZE + 2)) {
> + } else if (ret >= USB_DT_HUB_NONVAR_SIZE + 2) {
> + /* Make sure we have the DeviceRemovable field. */
> + size = USB_DT_HUB_NONVAR_SIZE + desc->bNbrPorts / 8 + 1;
> + if (ret < size)
> + return -EMSGSIZE;
The logic could be simplified a little. Since we don't really care
about the return code when an error occurs, you could just do:
} else if (ret >= USB_DT_HUB_NONVAR_SIZE +
desc->bNbrPorts / 8 + 1) {
/* We have the entire DeviceRemovable field. */
return ret;
}
Alan Stern