Re: divide error in dummy_timer

[Alan Stern]

On Thu, 24 Oct 2019, syzbot wrote:

> Hello,
> 
> syzbot has tested the proposed patch but the reproducer still triggered  
> crash:
> divide error in dummy_timer

> Tested on:
> 
> commit:         22be26f7 usb-fuzzer: main usb gadget fuzzer driver
> git tree:       https://github.com/google/kasan.git
> console output: https://syzkaller.appspot.com/x/log.txt?x=17b3e44ce00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=5fe29bc39eff9627
> dashboard link: https://syzkaller.appspot.com/bug?extid=8ab8bf161038a8768553
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> patch:          https://syzkaller.appspot.com/x/patch.diff?x=15ea0a97600000

Okay, this error has a couple of different aspects.  In particular, we
don't want endpoints to have maxpacket = 0 on either the host side or
the gadget side.  (Note that it _is_ possible for the two sides to
disagree about the maxpacket value, because the gadget driver can in
theory provide different endpoint descriptors to the UDC driver and to
the host.)

So yes, the core should check the value in the endpoint descriptor, but 
also we have to watch out for invalid values coming from userspace 
gadget drivers.  And not just in dummy-hcd; all UDCs are vulnerable.

So let's try the patch below to handle the gadget side of things.

Alan Stern

#syz test: https://github.com/google/kasan.git 22be26f7

 drivers/usb/gadget/udc/core.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: usb-devel/drivers/usb/gadget/udc/core.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/core.c
+++ usb-devel/drivers/usb/gadget/udc/core.c
@@ -98,6 +98,17 @@ int usb_ep_enable(struct usb_ep *ep)
 	if (ep->enabled)
 		goto out;
 
+	/* UDC drivers can't handle endpoints with maxpacket size 0 */
+	if (usb_endpoint_maxp(ep->desc) == 0) {
+		/*
+		 * We should log an error message here, but we can't call
+		 * dev_err() because there's no way to find the gadget
+		 * given only ep.
+		 */
+		ret = -EINVAL;
+		goto out;
+	}
+
 	ret = ep->ops->enable(ep, ep->desc);
 	if (ret)
 		goto out;