* [RFC]How else could a malicious device sabotage endpoints for usbnet
@ 2021-12-09 15:33 Oliver Neukum
2021-12-09 15:47 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Oliver Neukum @ 2021-12-09 15:33 UTC (permalink / raw)
To: linux-usb
[-- Attachment #1: Type: text/plain, Size: 206 bytes --]
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
[-- Attachment #2: 0001-usbnet-sanity-check-for-endpoint-types.patch --]
[-- Type: text/x-patch, Size: 1967 bytes --]
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;
+}
+
/* handles CDC Ethernet and many other network "bulk data" interfaces */
int usbnet_get_endpoints(struct usbnet *dev, struct usb_interface *intf)
{
@@ -1772,6 +1797,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
} else if (!info->in || !info->out)
status = usbnet_get_endpoints (dev, udev);
else {
+ if (!usbnet_validate_endpoints(dev, udev, info))
+ goto out3;
dev->in = usb_rcvbulkpipe (xdev, info->in);
dev->out = usb_sndbulkpipe (xdev, info->out);
if (!(info->flags & FLAG_NO_SETINT))
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC]How else could a malicious device sabotage endpoints for usbnet
2021-12-09 15:33 [RFC]How else could a malicious device sabotage endpoints for usbnet Oliver Neukum
@ 2021-12-09 15:47 ` Greg KH
2021-12-15 14:47 ` Oliver Neukum
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2021-12-09 15:47 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-usb
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC]How else could a malicious device sabotage endpoints for usbnet
2021-12-09 15:47 ` Greg KH
@ 2021-12-15 14:47 ` Oliver Neukum
2021-12-15 14:57 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Oliver Neukum @ 2021-12-15 14:47 UTC (permalink / raw)
To: Greg KH, Oliver Neukum; +Cc: linux-usb
On 09.12.21 16:47, Greg KH wrote:
>
> Why not use usb_find_common_endpoints() and/or the other helper
> functions instead? that's what they were created for.
Hi,
which one would I use? In this case I already know the endpoints
to be verified.
Regards
Oliver
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC]How else could a malicious device sabotage endpoints for usbnet
2021-12-15 14:47 ` Oliver Neukum
@ 2021-12-15 14:57 ` Greg KH
2021-12-16 10:16 ` Oliver Neukum
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2021-12-15 14:57 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-usb
On Wed, Dec 15, 2021 at 03:47:55PM +0100, Oliver Neukum wrote:
>
> On 09.12.21 16:47, Greg KH wrote:
> >
> > Why not use usb_find_common_endpoints() and/or the other helper
> > functions instead? that's what they were created for.
>
> Hi,
>
> which one would I use? In this case I already know the endpoints
> to be verified.
I have no context here so I have no idea, sorry.
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC]How else could a malicious device sabotage endpoints for usbnet
2021-12-15 14:57 ` Greg KH
@ 2021-12-16 10:16 ` Oliver Neukum
2021-12-21 7:54 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Oliver Neukum @ 2021-12-16 10:16 UTC (permalink / raw)
To: Greg KH, Oliver Neukum; +Cc: linux-usb
On 15.12.21 15:57, Greg KH wrote:
> On Wed, Dec 15, 2021 at 03:47:55PM +0100, Oliver Neukum wrote:
>> On 09.12.21 16:47, Greg KH wrote:
>>> Why not use usb_find_common_endpoints() and/or the other helper
>>> functions instead? that's what they were created for.
>> Hi,
>>
>> which one would I use? In this case I already know the endpoints
>> to be verified.
> I have no context here so I have no idea, sorry.
usbnet has three ways to match the endpoints
1) the subdriver provides a method
2) a heuristic to find the endpoints is used (that should be converted
to the new API)
3) they are given nummerically by the subdriver
It turns out that #3 needs to verify the endpoints against malicious
devices.
So the following questions
a) should that verification go into usbcore
b) what possible ways for a malicious device to spoof us can you come
up with
Regards
Oliver
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC]How else could a malicious device sabotage endpoints for usbnet
2021-12-16 10:16 ` Oliver Neukum
@ 2021-12-21 7:54 ` Greg KH
2021-12-21 8:36 ` Oliver Neukum
0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2021-12-21 7:54 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-usb
On Thu, Dec 16, 2021 at 11:16:26AM +0100, Oliver Neukum wrote:
>
> On 15.12.21 15:57, Greg KH wrote:
> > On Wed, Dec 15, 2021 at 03:47:55PM +0100, Oliver Neukum wrote:
> >> On 09.12.21 16:47, Greg KH wrote:
> >>> Why not use usb_find_common_endpoints() and/or the other helper
> >>> functions instead? that's what they were created for.
> >> Hi,
> >>
> >> which one would I use? In this case I already know the endpoints
> >> to be verified.
> > I have no context here so I have no idea, sorry.
>
> usbnet has three ways to match the endpoints
>
> 1) the subdriver provides a method
>
> 2) a heuristic to find the endpoints is used (that should be converted
> to the new API)
>
> 3) they are given nummerically by the subdriver
>
> It turns out that #3 needs to verify the endpoints against malicious
> devices.
> So the following questions
>
> a) should that verification go into usbcore
the usb_find_common_endpoints() functions are in the usbcore for drivers
to use for this type of problem.
> b) what possible ways for a malicious device to spoof us can you come
> up with
Start with:
- invalid endpoint sizes and types
- invalid data being sent on valid endpoint types
and you will catch almost all possible errors.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC]How else could a malicious device sabotage endpoints for usbnet
2021-12-21 7:54 ` Greg KH
@ 2021-12-21 8:36 ` Oliver Neukum
0 siblings, 0 replies; 7+ messages in thread
From: Oliver Neukum @ 2021-12-21 8:36 UTC (permalink / raw)
To: Greg KH, Oliver Neukum; +Cc: linux-usb
On 21.12.21 08:54, Greg KH wrote:
> On Thu, Dec 16, 2021 at 11:16:26AM +0100, Oliver Neukum wrote:
>>
>> 2) a heuristic to find the endpoints is used (that should be converted
>> to the new API)
>>
>> 3) they are given nummerically by the subdriver
>>
>> It turns out that #3 needs to verify the endpoints against malicious
>> devices.
>> So the following questions
>>
>> a) should that verification go into usbcore
> the usb_find_common_endpoints() functions are in the usbcore for drivers
> to use for this type of problem.
That API insist on finding the endpoints. It is a heuristic, so we need
to have a fallback in case it fails.
>> b) what possible ways for a malicious device to spoof us can you come
>> up with
> Start with:
> - invalid endpoint sizes and types
> - invalid data being sent on valid endpoint types
> and you will catch almost all possible errors.
>
OK. But I still need a way to do verification _only_.
Regards
Oliver
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-21 8:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09 15:33 [RFC]How else could a malicious device sabotage endpoints for usbnet Oliver Neukum
2021-12-09 15:47 ` Greg KH
2021-12-15 14:47 ` Oliver Neukum
2021-12-15 14:57 ` Greg KH
2021-12-16 10:16 ` Oliver Neukum
2021-12-21 7:54 ` Greg KH
2021-12-21 8:36 ` Oliver Neukum
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.