| Message ID | Pine.LNX.4.44L0.1910281052370.1485-100000@iolanthe.rowland.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 54f83b8c8ea9b22082a496deadf90447a326954e |
| Headers | show |
| Series | USB: gadget: Reject endpoints with 0 maxpacket value | expand |
On Mon, Oct 28, 2019 at 10:54:26AM -0400, Alan Stern wrote: > Endpoints with a maxpacket length of 0 are probably useless. They > can't transfer any data, and it's not at all unlikely that a UDC will > crash or hang when trying to handle a non-zero-length usb_request for > such an endpoint. Indeed, dummy-hcd gets a divide error when trying > to calculate the remainder of a transfer length by the maxpacket > value, as discovered by the syzbot fuzzer. > > Currently the gadget core does not check for endpoints having a > maxpacket value of 0. This patch adds a check to usb_ep_enable(), > preventing such endpoints from being used. > > As far as I know, none of the gadget drivers in the kernel tries to > create an endpoint with maxpacket = 0, but until now there has been > nothing to prevent userspace programs under gadgetfs or configfs from > doing it. > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > Reported-and-tested-by: syzbot+8ab8bf161038a8768553@syzkaller.appspotmail.com > CC: <stable@vger.kernel.org> > > --- > > > [as1925] > > > 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; > Felipe, I can take this now in my tree if I can get an ack. thanks, greg k-h
Hi, Greg KH <gregkh@linuxfoundation.org> writes: > On Mon, Oct 28, 2019 at 10:54:26AM -0400, Alan Stern wrote: >> Endpoints with a maxpacket length of 0 are probably useless. They >> can't transfer any data, and it's not at all unlikely that a UDC will >> crash or hang when trying to handle a non-zero-length usb_request for >> such an endpoint. Indeed, dummy-hcd gets a divide error when trying >> to calculate the remainder of a transfer length by the maxpacket >> value, as discovered by the syzbot fuzzer. >> >> Currently the gadget core does not check for endpoints having a >> maxpacket value of 0. This patch adds a check to usb_ep_enable(), >> preventing such endpoints from being used. >> >> As far as I know, none of the gadget drivers in the kernel tries to >> create an endpoint with maxpacket = 0, but until now there has been >> nothing to prevent userspace programs under gadgetfs or configfs from >> doing it. >> >> Signed-off-by: Alan Stern <stern@rowland.harvard.edu> >> Reported-and-tested-by: syzbot+8ab8bf161038a8768553@syzkaller.appspotmail.com >> CC: <stable@vger.kernel.org> >> >> --- >> >> >> [as1925] >> >> >> 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; >> > > Felipe, I can take this now in my tree if I can get an ack. Definitely: Acked-by: Felipe Balbi <balbi@kernel.org>
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;
Endpoints with a maxpacket length of 0 are probably useless. They can't transfer any data, and it's not at all unlikely that a UDC will crash or hang when trying to handle a non-zero-length usb_request for such an endpoint. Indeed, dummy-hcd gets a divide error when trying to calculate the remainder of a transfer length by the maxpacket value, as discovered by the syzbot fuzzer. Currently the gadget core does not check for endpoints having a maxpacket value of 0. This patch adds a check to usb_ep_enable(), preventing such endpoints from being used. As far as I know, none of the gadget drivers in the kernel tries to create an endpoint with maxpacket = 0, but until now there has been nothing to prevent userspace programs under gadgetfs or configfs from doing it. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Reported-and-tested-by: syzbot+8ab8bf161038a8768553@syzkaller.appspotmail.com CC: <stable@vger.kernel.org> --- [as1925] drivers/usb/gadget/udc/core.c | 11 +++++++++++ 1 file changed, 11 insertions(+)