Re: [RFC]How else could a malicious device sabotage endpoints for usbnet

[Greg KH <greg@kroah.com>]

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@suse.com>
> 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@suse.com>
> ---
>  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