On Thu, Dec 09, 2021 at 04:33:29PM +0100, Oliver Neukum wrote:
> Hi,
>
> I have checked for type, direction and number of endpoints.
> But I keep thinking that I have overlooked a way to make broken
> endpoint descriptors. Any suggestions?
>
> Regards
> Oliver
>
> >From 853e421630f82fb3b7005ad0b294c091a064ac39 Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oneukum@xxxxxxxx>
> Date: Thu, 18 Nov 2021 18:15:03 +0100
> Subject: [PATCH] usbnet: sanity check for endpoint types
>
> A malicious device can pretend to be a device with a known
> configuration of endpoints yet present endpoints of the wrong type
> or too few or none at all.
>
> Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
> ---
> drivers/net/usb/usbnet.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 9a6450f796dc..b1f93810a6f3 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -91,6 +91,31 @@ static const char * const usbnet_event_names[] = {
> [EVENT_NO_IP_ALIGN] = "EVENT_NO_IP_ALIGN",
> };
>
> +bool usbnet_validate_endpoints(struct usbnet *dev, struct usb_interface *intf, const struct driver_info *info)
> +{
> + struct usb_host_interface *alt = intf->cur_altsetting;
> + struct usb_host_endpoint *e;
> + int num_endpoints = alt->desc.bNumEndpoints;
> +
> + if (info->in > num_endpoints)
> + return false;
> + e = alt->endpoint + info->in;
> + if (!e)
> + return false;
> + if (!usb_endpoint_is_bulk_in(&e->desc))
> + return false;
> +
> + if (info->out > num_endpoints)
> + return false;
> + e = alt->endpoint + info->out;
> + if (!e)
> + return false;
> + if (!usb_endpoint_is_bulk_out(&e->desc))
> + return false;
> +
> + return true;
Why not use usb_find_common_endpoints() and/or the other helper
functions instead? that's what they were created for.
thanks,
greg k-h
