[PATCH] usbnet: sanity check for maxpacket

[PATCH] usbnet: sanity check for maxpacket

[Oliver Neukum]

maxpacket of 0 makes no sense and oopdses as we need to divide
by it. Give up.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Reported-by: syzbot+76bb1d34ffa0adc03baa@syzkaller.appspotmail.com
---
 drivers/net/usb/usbnet.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 840c1c2ab16a..396f5e677bf0 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1788,6 +1788,9 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
 	if (!dev->rx_urb_size)
 		dev->rx_urb_size = dev->hard_mtu;
 	dev->maxpacket = usb_maxpacket (dev->udev, dev->out, 1);
+	if (dev->maxpacket == 0)
+		/* that is a broken device */
+		goto out4;
 
 	/* let userspace know we have a random address */
 	if (ether_addr_equal(net->dev_addr, node_id))
-- 
2.26.2

Re: [PATCH] usbnet: sanity check for maxpacket

[Johan Hovold]

On Wed, Oct 20, 2021 at 11:17:33AM +0200, Oliver Neukum wrote:
> maxpacket of 0 makes no sense and oopdses as we need to divide

typo: oopses

> by it. Give up.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Reported-by: syzbot+76bb1d34ffa0adc03baa@syzkaller.appspotmail.com
> ---
>  drivers/net/usb/usbnet.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 840c1c2ab16a..396f5e677bf0 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -1788,6 +1788,9 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
>  	if (!dev->rx_urb_size)
>  		dev->rx_urb_size = dev->hard_mtu;
>  	dev->maxpacket = usb_maxpacket (dev->udev, dev->out, 1);
> +	if (dev->maxpacket == 0)
> +		/* that is a broken device */
> +		goto out4;

I'd prefer brackets here since it's a multi-line block. Or just drop the
comment.

>  
>  	/* let userspace know we have a random address */
>  	if (ether_addr_equal(net->dev_addr, node_id))

Other than that, looks good:

Reviewed-by: Johan Hovold <johan@kernel.org>

Johan

[PATCH] usbnet: sanity check for maxpacket

[Tadeusz Struk]

[-- Attachment #1.1: Type: text/plain, Size: 356 bytes --]

Hi,
Upstream commit id: 397430b50a363d8b7bdda00522123f82df6adc5e
should be applied because it fixes an Oops (divide by zero) that can be
triggered from user space. See:
https://syzkaller.appspot.com/bug?id=2ebf7e5eb303ac9a598e0dab2c0c8b03ead7abce

It should be applied to stable kernels: 5.14, 5.10, 5.4, 4.19, 4.14, 4.9, 4.4
-- 
Thanks,
Tadeusz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH] usbnet: sanity check for maxpacket

[Greg KH]

On Mon, Oct 25, 2021 at 07:20:31PM -0700, Tadeusz Struk wrote:
> Hi,
> Upstream commit id: 397430b50a363d8b7bdda00522123f82df6adc5e
> should be applied because it fixes an Oops (divide by zero) that can be
> triggered from user space. See:
> https://syzkaller.appspot.com/bug?id=2ebf7e5eb303ac9a598e0dab2c0c8b03ead7abce
> 
> It should be applied to stable kernels: 5.14, 5.10, 5.4, 4.19, 4.14, 4.9, 4.4

It's already in the latest stable -rc releases, do you not see it there?

thanks,

greg k-h

Re: [PATCH] usbnet: sanity check for maxpacket

[Tadeusz Struk]

On 10/25/21 22:43, Greg KH wrote:
>> It should be applied to stable kernels: 5.14, 5.10, 5.4, 4.19, 4.14, 4.9, 4.4
> It's already in the latest stable -rc releases, do you not see it there?

I haven't checked the rc releases, just the latest stable versions.
Now I also see Sasha Levin's submissions on the stable mailing list archive.
All good. Sorry for the noise.

-- 
Thanks,
Tadeusz

Re: [PATCH] usbnet: sanity check for maxpacket

[Greg KH]

On Tue, Oct 26, 2021 at 07:52:00AM -0700, Tadeusz Struk wrote:
> On 10/25/21 22:43, Greg KH wrote:
> > > It should be applied to stable kernels: 5.14, 5.10, 5.4, 4.19, 4.14, 4.9, 4.4
> > It's already in the latest stable -rc releases, do you not see it there?
> 
> I haven't checked the rc releases, just the latest stable versions.
> Now I also see Sasha Levin's submissions on the stable mailing list archive.
> All good. Sorry for the noise.

Also note that there is a bug in this commit, so it was dropped from the
stable queues until the fix for this hits Linus's tree.

thanks,

greg k-h

Re: [PATCH] usbnet: sanity check for maxpacket

[Tadeusz Struk]

[-- Attachment #1.1: Type: text/plain, Size: 744 bytes --]

On 10/26/21 08:03, Greg KH wrote:
> On Tue, Oct 26, 2021 at 07:52:00AM -0700, Tadeusz Struk wrote:
>> On 10/25/21 22:43, Greg KH wrote:
>>>> It should be applied to stable kernels: 5.14, 5.10, 5.4, 4.19, 4.14, 4.9, 4.4
>>> It's already in the latest stable -rc releases, do you not see it there?
>>
>> I haven't checked the rc releases, just the latest stable versions.
>> Now I also see Sasha Levin's submissions on the stable mailing list archive.
>> All good. Sorry for the noise.
> 
> Also note that there is a bug in this commit, so it was dropped from the
> stable queues until the fix for this hits Linus's tree.
> 

Good to know. Ideally we could also add some netif_warn() message for the user.

-- 
Thanks,
Tadeusz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH] usbnet: sanity check for maxpacket

[Greg KH]

On Tue, Oct 26, 2021 at 08:08:37AM -0700, Tadeusz Struk wrote:
> On 10/26/21 08:03, Greg KH wrote:
> > On Tue, Oct 26, 2021 at 07:52:00AM -0700, Tadeusz Struk wrote:
> > > On 10/25/21 22:43, Greg KH wrote:
> > > > > It should be applied to stable kernels: 5.14, 5.10, 5.4, 4.19, 4.14, 4.9, 4.4
> > > > It's already in the latest stable -rc releases, do you not see it there?
> > > 
> > > I haven't checked the rc releases, just the latest stable versions.
> > > Now I also see Sasha Levin's submissions on the stable mailing list archive.
> > > All good. Sorry for the noise.
> > 
> > Also note that there is a bug in this commit, so it was dropped from the
> > stable queues until the fix for this hits Linus's tree.
> > 
> 
> Good to know. Ideally we could also add some netif_warn() message for the user.

Why?  It's an "obviously broken" device that is created just to try to
harm the kernel.  No need to be noisy about it.

thanks,

greg k-h