[PATCH v2] usb:host: fix divide-by-zero in function fhci_queue_urb

[PATCH v2] usb:host: fix divide-by-zero in function fhci_queue_urb

[zhuyan (M)]

On Wed, 17 Apr 2019, Alan Stern wrote:

> On Wed, 17 Apr 2019, zhuyan (M) wrote:
> 
> > On Tue, 16 Apr 2019 11:07:56 -0400, Alan Stern wrote:
> > 
> > > On Tue, 16 Apr 2019, zhuyan (M) wrote:
> > > > On Tue, 16 Apr 2019 at 11:45:45 +0200, Greg KH wrote:
> > > > > On Tue, Apr 09, 2019 at 10:37:12PM +0800, zhuyan wrote:
> > > > > > In function fhci_queue_urb, the divisor of expression 
> > > > > > (urb->transfer_buffer_length % usb_maxpacket(urb->dev, 
> > > > > > urb->pipe,
> > > > > > usb_pipeout(urb->pipe))) may be zero.
> > > > > 
> > > > > How can you hit that?
> > > > > 
> > > > > > When it is zero, unexpected results may occur, so it is 
> > > > > > necessary to ensure that the divisor is not zero.
> > > > > > 
> > > > > > Signed-off-by: zhuyan <zhuyan34@huawei.com>
> > > > > 
> > > > > I need a "Full" name here, not just a single name.  Whatever you use to sign documents is good.
> > > > > 
> > > > > thanks,
> > > > > 
> > > > > greg k-h
> > > > 
> > > > In function usb_maxpacket, when ep is NULL, its return value is 0.  
> > > 
> > > fhci_queue_urb() shouldn't use urb->pipe to compute the maxpacket 
> > > size anyway.  It should use usb_endpoint_maxp(&urb->ep->desc).
> > 
> > Currently, fhci_queue_urb(), call usb_maxpacket() multiple times to 
> > calculate  the maxpacket size. The usb_maxpacket() will call 
> > usb_endpoint_maxp() to compute the maxpacket size.
> 
> I know that.  What fhci_queue_urb() is doing is wrong.  You should change it: 
> Make it call usb_endpoint_maxp directly instead of calling usb_maxpacket.
> 

From 1996456d0cc17b5ff7746a598ff355b25d13db3e Mon Sep 17 00:00:00 2001
From: zhuyan <zhuyan34@huawei.com>
Date: Thu, 18 Apr 2019 00:53:03 +0800
Subject: [PATCH] usb: host: fix divide-by-zero in function fhci_queue_urb

fhci_queue_urb() shouldn't use urb->pipe to compute the maxpacket
size anyway.It should use usb_endpoint_maxp(&urb->ep->desc).

In function fhci_queue_urb, the divisor of expression
(urb->transfer_buffer_length % usb_maxpacket(urb->dev, urb->pipe,
usb_pipeout(urb->pipe))) may be zero. When it is zero, unexpected results
may occur, so it is necessary to ensure that the divisor is not zero.

Signed-off-by: zhuyan <zhuyan34@huawei.com>
---
 drivers/usb/host/fhci-sched.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/fhci-sched.c b/drivers/usb/host/fhci-sched.c
index 3d12cdd..7dcfe22 100644
--- a/drivers/usb/host/fhci-sched.c
+++ b/drivers/usb/host/fhci-sched.c
@@ -704,6 +704,7 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
 	struct td *td;
 	u8 *data;
 	u16 cnt = 0;
+	u16 max_pkt_size = 0;
 
 	if (ed == NULL) {
 		ed = fhci_get_empty_ed(fhci);
@@ -727,8 +728,7 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
 		}
 		ed->speed = (urb->dev->speed == USB_SPEED_LOW) ?
 			FHCI_LOW_SPEED : FHCI_FULL_SPEED;
-		ed->max_pkt_size = usb_maxpacket(urb->dev,
-			urb->pipe, usb_pipeout(urb->pipe));
+		ed->max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
 		urb->ep->hcpriv = ed;
 		fhci_dbg(fhci, "new ep speed=%d max_pkt_size=%d\n",
 			 ed->speed, ed->max_pkt_size);
@@ -765,11 +765,12 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
 
 	switch (ed->mode) {
 	case FHCI_TF_BULK:
+		max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
 		if (urb->transfer_flags & URB_ZERO_PACKET &&
 				urb->transfer_buffer_length > 0 &&
+				(max_pkt_size != 0) &&
 				((urb->transfer_buffer_length %
-				usb_maxpacket(urb->dev, urb->pipe,
-				usb_pipeout(urb->pipe))) == 0))
+				max_pkt_size) == 0))
 			urb_state = US_BULK0;
 		while (data_len > 4096) {
 			td = fhci_td_fill(fhci, urb, urb_priv, ed, cnt,
@@ -807,8 +808,8 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
 		break;
 	case FHCI_TF_CTRL:
 		ed->dev_addr = usb_pipedevice(urb->pipe);
-		ed->max_pkt_size = usb_maxpacket(urb->dev, urb->pipe,
-			usb_pipeout(urb->pipe));
+		ed->max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
+
 		/* setup stage */
 		td = fhci_td_fill(fhci, urb, urb_priv, ed, cnt++, FHCI_TA_SETUP,
 			USB_TD_TOGGLE_DATA0, urb->setup_packet, 8, 0, 0, true);
-- 
1.8.5.6

Re: [PATCH v2] usb:host: fix divide-by-zero in function fhci_queue_urb

[Alan Stern]

On Wed, 17 Apr 2019, zhuyan (M) wrote:

> On Wed, 17 Apr 2019, Alan Stern wrote:
> 
> > On Wed, 17 Apr 2019, zhuyan (M) wrote:
> > 
> > > On Tue, 16 Apr 2019 11:07:56 -0400, Alan Stern wrote:
> > > 
> > > > On Tue, 16 Apr 2019, zhuyan (M) wrote:
> > > > > On Tue, 16 Apr 2019 at 11:45:45 +0200, Greg KH wrote:
> > > > > > On Tue, Apr 09, 2019 at 10:37:12PM +0800, zhuyan wrote:
> > > > > > > In function fhci_queue_urb, the divisor of expression 
> > > > > > > (urb->transfer_buffer_length % usb_maxpacket(urb->dev, 
> > > > > > > urb->pipe,
> > > > > > > usb_pipeout(urb->pipe))) may be zero.
> > > > > > 
> > > > > > How can you hit that?
> > > > > > 
> > > > > > > When it is zero, unexpected results may occur, so it is 
> > > > > > > necessary to ensure that the divisor is not zero.
> > > > > > > 
> > > > > > > Signed-off-by: zhuyan <zhuyan34@huawei.com>
> > > > > > 
> > > > > > I need a "Full" name here, not just a single name.  Whatever you use to sign documents is good.
> > > > > > 
> > > > > > thanks,
> > > > > > 
> > > > > > greg k-h
> > > > > 
> > > > > In function usb_maxpacket, when ep is NULL, its return value is 0.  
> > > > 
> > > > fhci_queue_urb() shouldn't use urb->pipe to compute the maxpacket 
> > > > size anyway.  It should use usb_endpoint_maxp(&urb->ep->desc).
> > > 
> > > Currently, fhci_queue_urb(), call usb_maxpacket() multiple times to 
> > > calculate  the maxpacket size. The usb_maxpacket() will call 
> > > usb_endpoint_maxp() to compute the maxpacket size.
> > 
> > I know that.  What fhci_queue_urb() is doing is wrong.  You should change it: 
> > Make it call usb_endpoint_maxp directly instead of calling usb_maxpacket.
> > 
> 
> From 1996456d0cc17b5ff7746a598ff355b25d13db3e Mon Sep 17 00:00:00 2001
> From: zhuyan <zhuyan34@huawei.com>
> Date: Thu, 18 Apr 2019 00:53:03 +0800
> Subject: [PATCH] usb: host: fix divide-by-zero in function fhci_queue_urb
> 
> fhci_queue_urb() shouldn't use urb->pipe to compute the maxpacket
> size anyway.It should use usb_endpoint_maxp(&urb->ep->desc).
> 
> In function fhci_queue_urb, the divisor of expression
> (urb->transfer_buffer_length % usb_maxpacket(urb->dev, urb->pipe,
> usb_pipeout(urb->pipe))) may be zero. When it is zero, unexpected results
> may occur, so it is necessary to ensure that the divisor is not zero.
> 
> Signed-off-by: zhuyan <zhuyan34@huawei.com>
> ---
>  drivers/usb/host/fhci-sched.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/host/fhci-sched.c b/drivers/usb/host/fhci-sched.c
> index 3d12cdd..7dcfe22 100644
> --- a/drivers/usb/host/fhci-sched.c
> +++ b/drivers/usb/host/fhci-sched.c
> @@ -704,6 +704,7 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
>  	struct td *td;
>  	u8 *data;
>  	u16 cnt = 0;
> +	u16 max_pkt_size = 0;
>  
>  	if (ed == NULL) {
>  		ed = fhci_get_empty_ed(fhci);
> @@ -727,8 +728,7 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
>  		}
>  		ed->speed = (urb->dev->speed == USB_SPEED_LOW) ?
>  			FHCI_LOW_SPEED : FHCI_FULL_SPEED;
> -		ed->max_pkt_size = usb_maxpacket(urb->dev,
> -			urb->pipe, usb_pipeout(urb->pipe));
> +		ed->max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
>  		urb->ep->hcpriv = ed;
>  		fhci_dbg(fhci, "new ep speed=%d max_pkt_size=%d\n",
>  			 ed->speed, ed->max_pkt_size);
> @@ -765,11 +765,12 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
>  
>  	switch (ed->mode) {
>  	case FHCI_TF_BULK:
> +		max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
>  		if (urb->transfer_flags & URB_ZERO_PACKET &&
>  				urb->transfer_buffer_length > 0 &&
> +				(max_pkt_size != 0) &&

Now you shouldn't need to add this extra test.

Alan Stern

>  				((urb->transfer_buffer_length %
> -				usb_maxpacket(urb->dev, urb->pipe,
> -				usb_pipeout(urb->pipe))) == 0))
> +				max_pkt_size) == 0))
>  			urb_state = US_BULK0;
>  		while (data_len > 4096) {
>  			td = fhci_td_fill(fhci, urb, urb_priv, ed, cnt,
> @@ -807,8 +808,8 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
>  		break;
>  	case FHCI_TF_CTRL:
>  		ed->dev_addr = usb_pipedevice(urb->pipe);
> -		ed->max_pkt_size = usb_maxpacket(urb->dev, urb->pipe,
> -			usb_pipeout(urb->pipe));
> +		ed->max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
> +
>  		/* setup stage */
>  		td = fhci_td_fill(fhci, urb, urb_priv, ed, cnt++, FHCI_TA_SETUP,
>  			USB_TD_TOGGLE_DATA0, urb->setup_packet, 8, 0, 0, true);

Re: [PATCH v2] usb:host: fix divide-by-zero in function fhci_queue_urb

[Greg KH]

On Wed, Apr 17, 2019 at 05:05:33PM +0000, zhuyan (M) wrote:
> On Wed, 17 Apr 2019, Alan Stern wrote:
> 
> > On Wed, 17 Apr 2019, zhuyan (M) wrote:
> > 
> > > On Tue, 16 Apr 2019 11:07:56 -0400, Alan Stern wrote:
> > > 
> > > > On Tue, 16 Apr 2019, zhuyan (M) wrote:
> > > > > On Tue, 16 Apr 2019 at 11:45:45 +0200, Greg KH wrote:
> > > > > > On Tue, Apr 09, 2019 at 10:37:12PM +0800, zhuyan wrote:
> > > > > > > In function fhci_queue_urb, the divisor of expression 
> > > > > > > (urb->transfer_buffer_length % usb_maxpacket(urb->dev, 
> > > > > > > urb->pipe,
> > > > > > > usb_pipeout(urb->pipe))) may be zero.
> > > > > > 
> > > > > > How can you hit that?
> > > > > > 
> > > > > > > When it is zero, unexpected results may occur, so it is 
> > > > > > > necessary to ensure that the divisor is not zero.
> > > > > > > 
> > > > > > > Signed-off-by: zhuyan <zhuyan34@huawei.com>
> > > > > > 
> > > > > > I need a "Full" name here, not just a single name.  Whatever you use to sign documents is good.
> > > > > > 
> > > > > > thanks,
> > > > > > 
> > > > > > greg k-h
> > > > > 
> > > > > In function usb_maxpacket, when ep is NULL, its return value is 0.  
> > > > 
> > > > fhci_queue_urb() shouldn't use urb->pipe to compute the maxpacket 
> > > > size anyway.  It should use usb_endpoint_maxp(&urb->ep->desc).
> > > 
> > > Currently, fhci_queue_urb(), call usb_maxpacket() multiple times to 
> > > calculate  the maxpacket size. The usb_maxpacket() will call 
> > > usb_endpoint_maxp() to compute the maxpacket size.
> > 
> > I know that.  What fhci_queue_urb() is doing is wrong.  You should change it: 
> > Make it call usb_endpoint_maxp directly instead of calling usb_maxpacket.
> > 
> 
> >From 1996456d0cc17b5ff7746a598ff355b25d13db3e Mon Sep 17 00:00:00 2001
> From: zhuyan <zhuyan34@huawei.com>
> Date: Thu, 18 Apr 2019 00:53:03 +0800
> Subject: [PATCH] usb: host: fix divide-by-zero in function fhci_queue_urb
> 
> fhci_queue_urb() shouldn't use urb->pipe to compute the maxpacket
> size anyway.It should use usb_endpoint_maxp(&urb->ep->desc).
> 
> In function fhci_queue_urb, the divisor of expression
> (urb->transfer_buffer_length % usb_maxpacket(urb->dev, urb->pipe,
> usb_pipeout(urb->pipe))) may be zero. When it is zero, unexpected results
> may occur, so it is necessary to ensure that the divisor is not zero.
> 
> Signed-off-by: zhuyan <zhuyan34@huawei.com>

I still need a full name here and on the From: line :(

thanks,

greg k-h

Re: [PATCH v2] usb:host: fix divide-by-zero in function fhci_queue_urb

[zhuyan (M)]

On Wed, 17 Apr 2019 14:59:27 -0400, Alan Stern wrote:
> 
> On Wed, 17 Apr 2019, zhuyan (M) wrote:
> 
> > On Wed, 17 Apr 2019, Alan Stern wrote:
> > 
> > > On Wed, 17 Apr 2019, zhuyan (M) wrote:
> > > 
> > > > On Tue, 16 Apr 2019 11:07:56 -0400, Alan Stern wrote:
> > > > 
> > > > > On Tue, 16 Apr 2019, zhuyan (M) wrote:
> > > > > > On Tue, 16 Apr 2019 at 11:45:45 +0200, Greg KH wrote:
> > > > > > > On Tue, Apr 09, 2019 at 10:37:12PM +0800, zhuyan wrote:
> > > > > > > > In function fhci_queue_urb, the divisor of expression 
> > > > > > > > (urb->transfer_buffer_length % usb_maxpacket(urb->dev,
> > > > > > > > urb->pipe,
> > > > > > > > usb_pipeout(urb->pipe))) may be zero.
> > > > > > > 
> > > > > > > How can you hit that?
> > > > > > > 
> > > > > > > > When it is zero, unexpected results may occur, so it is 
> > > > > > > > necessary to ensure that the divisor is not zero.
> > > > > > > > 
> > > > > > > > Signed-off-by: zhuyan <zhuyan34@huawei.com>
> > > > > > > 
> > > > > > > I need a "Full" name here, not just a single name.  Whatever you use to sign documents is good.
> > > > > > > 
> > > > > > > thanks,
> > > > > > > 
> > > > > > > greg k-h
> > > > > > 
> > > > > > In function usb_maxpacket, when ep is NULL, its return value is 0.  
> > > > > 
> > > > > fhci_queue_urb() shouldn't use urb->pipe to compute the 
> > > > > maxpacket size anyway.  It should use usb_endpoint_maxp(&urb->ep->desc).
> > > > 
> > > > Currently, fhci_queue_urb(), call usb_maxpacket() multiple times 
> > > > to calculate  the maxpacket size. The usb_maxpacket() will call
> > > > usb_endpoint_maxp() to compute the maxpacket size.
> > > 
> > > I know that.  What fhci_queue_urb() is doing is wrong.  You should change it: 
> > > Make it call usb_endpoint_maxp directly instead of calling usb_maxpacket.
> > > 
> > 
> > From 1996456d0cc17b5ff7746a598ff355b25d13db3e Mon Sep 17 00:00:00 2001
> > From: zhuyan <zhuyan34@huawei.com>
> > Date: Thu, 18 Apr 2019 00:53:03 +0800
> > Subject: [PATCH] usb: host: fix divide-by-zero in function 
> > fhci_queue_urb
> > 
> > fhci_queue_urb() shouldn't use urb->pipe to compute the maxpacket size 
> > anyway.It should use usb_endpoint_maxp(&urb->ep->desc).
> > 
> > In function fhci_queue_urb, the divisor of expression 
> > (urb->transfer_buffer_length % usb_maxpacket(urb->dev, urb->pipe,
> > usb_pipeout(urb->pipe))) may be zero. When it is zero, unexpected 
> > results may occur, so it is necessary to ensure that the divisor is not zero.
> > 
> > Signed-off-by: zhuyan <zhuyan34@huawei.com>
> > ---
> >  drivers/usb/host/fhci-sched.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/usb/host/fhci-sched.c 
> > b/drivers/usb/host/fhci-sched.c index 3d12cdd..7dcfe22 100644
> > --- a/drivers/usb/host/fhci-sched.c
> > +++ b/drivers/usb/host/fhci-sched.c
> > @@ -704,6 +704,7 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
> >  	struct td *td;
> >  	u8 *data;
> >  	u16 cnt = 0;
> > +	u16 max_pkt_size = 0;
> >  
> >  	if (ed == NULL) {
> >  		ed = fhci_get_empty_ed(fhci);
> > @@ -727,8 +728,7 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
> >  		}
> >  		ed->speed = (urb->dev->speed == USB_SPEED_LOW) ?
> >  			FHCI_LOW_SPEED : FHCI_FULL_SPEED;
> > -		ed->max_pkt_size = usb_maxpacket(urb->dev,
> > -			urb->pipe, usb_pipeout(urb->pipe));
> > +		ed->max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
> >  		urb->ep->hcpriv = ed;
> >  		fhci_dbg(fhci, "new ep speed=%d max_pkt_size=%d\n",
> >  			 ed->speed, ed->max_pkt_size);
> > @@ -765,11 +765,12 @@ void fhci_queue_urb(struct fhci_hcd *fhci, 
> > struct urb *urb)
> >  
> >  	switch (ed->mode) {
> >  	case FHCI_TF_BULK:
> > +		max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
> >  		if (urb->transfer_flags & URB_ZERO_PACKET &&
> >  				urb->transfer_buffer_length > 0 &&
> > +				(max_pkt_size != 0) &&
> 
> Now you shouldn't need to add this extra test.
> 
> Alan Stern
> 
> >  				((urb->transfer_buffer_length %
> > -				usb_maxpacket(urb->dev, urb->pipe,
> > -				usb_pipeout(urb->pipe))) == 0))
> > +				max_pkt_size) == 0))
> >  			urb_state = US_BULK0;
> >  		while (data_len > 4096) {
> >  			td = fhci_td_fill(fhci, urb, urb_priv, ed, cnt, @@ -807,8 +808,8 
> > @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
> >  		break;
> >  	case FHCI_TF_CTRL:
> >  		ed->dev_addr = usb_pipedevice(urb->pipe);
> > -		ed->max_pkt_size = usb_maxpacket(urb->dev, urb->pipe,
> > -			usb_pipeout(urb->pipe));
> > +		ed->max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
> > +
> >  		/* setup stage */
> >  		td = fhci_td_fill(fhci, urb, urb_priv, ed, cnt++, FHCI_TA_SETUP,
> >  			USB_TD_TOGGLE_DATA0, urb->setup_packet, 8, 0, 0, true);

But the return value of usb_endpoint_maxp() may be 0. 
Do we need to take protective measures?

Yan Zhu

Re: [PATCH v2] usb:host: fix divide-by-zero in function fhci_queue_urb

[Alan Stern]

On Thu, 18 Apr 2019, zhuyan (M) wrote:

> > > @@ -765,11 +765,12 @@ void fhci_queue_urb(struct fhci_hcd *fhci, 
> > > struct urb *urb)
> > >  
> > >  	switch (ed->mode) {
> > >  	case FHCI_TF_BULK:
> > > +		max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
> > >  		if (urb->transfer_flags & URB_ZERO_PACKET &&
> > >  				urb->transfer_buffer_length > 0 &&
> > > +				(max_pkt_size != 0) &&
> > 
> > Now you shouldn't need to add this extra test.
> > 
> > Alan Stern

> But the return value of usb_endpoint_maxp() may be 0. 
> Do we need to take protective measures?

Yes, we do.  But the protective measures don't belong in the fhci 
driver.

They should go in core/config.c:usb_parse_endpoint().  That routine 
already has code to check for maxpacket sizes that are too large; you 
can add code that checks for maxpacket sizes that are too small.

In theory an interrupt or isochronous endpoint might have maxpacket set
to 0 -- we could warn about these and accept them -- but this is not
allowed for bulk endpoints (see section 5.8.3 of the USB 2.0
specification).  In fact, bulk endpoints are not allowed to have
maxpacket size smaller than 8.

Maybe if you make this change then fhci-hcd won't need any
modifications at all.

Alan Stern

Re: [PATCH v2] usb:host: fix divide-by-zero in function fhci_queue_urb

[zhuyan (M)]

On Wed, 17 Apr 2019 21:49:03 +0200, Greg KH wrote:
> On Wed, Apr 17, 2019 at 05:05:33PM +0000, zhuyan (M) wrote:
> > On Wed, 17 Apr 2019, Alan Stern wrote:
> > 
> > > On Wed, 17 Apr 2019, zhuyan (M) wrote:
> > > 
> > > > On Tue, 16 Apr 2019 11:07:56 -0400, Alan Stern wrote:
> > > > 
> > > > > On Tue, 16 Apr 2019, zhuyan (M) wrote:
> > > > > > On Tue, 16 Apr 2019 at 11:45:45 +0200, Greg KH wrote:
> > > > > > > On Tue, Apr 09, 2019 at 10:37:12PM +0800, zhuyan wrote:
> > > > > > > > In function fhci_queue_urb, the divisor of expression 
> > > > > > > > (urb->transfer_buffer_length % usb_maxpacket(urb->dev,
> > > > > > > > urb->pipe,
> > > > > > > > usb_pipeout(urb->pipe))) may be zero.
> > > > > > > 
> > > > > > > How can you hit that?
> > > > > > > 
> > > > > > > > When it is zero, unexpected results may occur, so it is 
> > > > > > > > necessary to ensure that the divisor is not zero.
> > > > > > > > 
> > > > > > > > Signed-off-by: zhuyan <zhuyan34@huawei.com>
> > > > > > > 
> > > > > > > I need a "Full" name here, not just a single name.  Whatever you use to sign documents is good.
> > > > > > > 
> > > > > > > thanks,
> > > > > > > 
> > > > > > > greg k-h
> > > > > > 
> > > > > > In function usb_maxpacket, when ep is NULL, its return value is 0.  
> > > > > 
> > > > > fhci_queue_urb() shouldn't use urb->pipe to compute the 
> > > > > maxpacket size anyway.  It should use usb_endpoint_maxp(&urb->ep->desc).
> > > > 
> > > > Currently, fhci_queue_urb(), call usb_maxpacket() multiple times 
> > > > to calculate  the maxpacket size. The usb_maxpacket() will call
> > > > usb_endpoint_maxp() to compute the maxpacket size.
> > > 
> > > I know that.  What fhci_queue_urb() is doing is wrong.  You should change it: 
> > > Make it call usb_endpoint_maxp directly instead of calling usb_maxpacket.
> > > 
> > 
> > >From 1996456d0cc17b5ff7746a598ff355b25d13db3e Mon Sep 17 00:00:00 
> > >2001
> > From: zhuyan <zhuyan34@huawei.com>
> > Date: Thu, 18 Apr 2019 00:53:03 +0800
> > Subject: [PATCH] usb: host: fix divide-by-zero in function 
> > fhci_queue_urb
> > 
> > fhci_queue_urb() shouldn't use urb->pipe to compute the maxpacket size 
> > anyway.It should use usb_endpoint_maxp(&urb->ep->desc).
> > 
> > In function fhci_queue_urb, the divisor of expression 
> > (urb->transfer_buffer_length % usb_maxpacket(urb->dev, urb->pipe,
> > usb_pipeout(urb->pipe))) may be zero. When it is zero, unexpected 
> > results may occur, so it is necessary to ensure that the divisor is not zero.
> > 
> > Signed-off-by: zhuyan <zhuyan34@huawei.com>
> 
> I still need a full name here and on the From: line :(

I am so sorry. I will change it.

From 1996456d0cc17b5ff7746a598ff355b25d13db3e Mon Sep 17 00:00:00 2001
From: Yan Zhu <zhuyan34@huawei.com>
Date: Thu, 18 Apr 2019 00:53:03 +0800
Subject: [PATCH] usb: host: fix divide-by-zero in function fhci_queue_urb

fhci_queue_urb() shouldn't use urb->pipe to compute the maxpacket
size anyway.It should use usb_endpoint_maxp(&urb->ep->desc).

In function fhci_queue_urb, the divisor of expression
(urb->transfer_buffer_length % usb_maxpacket(urb->dev, urb->pipe,
usb_pipeout(urb->pipe))) may be zero. When it is zero, unexpected results
may occur, so it is necessary to ensure that the divisor is not zero.

Signed-off-by: Yan Zhu <zhuyan34@huawei.com>
---
 drivers/usb/host/fhci-sched.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/fhci-sched.c b/drivers/usb/host/fhci-sched.c
index 3d12cdd..7dcfe22 100644
--- a/drivers/usb/host/fhci-sched.c
+++ b/drivers/usb/host/fhci-sched.c
@@ -704,6 +704,7 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
 	struct td *td;
 	u8 *data;
 	u16 cnt = 0;
+	u16 max_pkt_size = 0;
 
 	if (ed == NULL) {
 		ed = fhci_get_empty_ed(fhci);
@@ -727,8 +728,7 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
 		}
 		ed->speed = (urb->dev->speed == USB_SPEED_LOW) ?
 			FHCI_LOW_SPEED : FHCI_FULL_SPEED;
-		ed->max_pkt_size = usb_maxpacket(urb->dev,
-			urb->pipe, usb_pipeout(urb->pipe));
+		ed->max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
 		urb->ep->hcpriv = ed;
 		fhci_dbg(fhci, "new ep speed=%d max_pkt_size=%d\n",
 			 ed->speed, ed->max_pkt_size);
@@ -765,11 +765,12 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
 
 	switch (ed->mode) {
 	case FHCI_TF_BULK:
+		max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
 		if (urb->transfer_flags & URB_ZERO_PACKET &&
 				urb->transfer_buffer_length > 0 &&
+				(max_pkt_size != 0) &&
 				((urb->transfer_buffer_length %
-				usb_maxpacket(urb->dev, urb->pipe,
-				usb_pipeout(urb->pipe))) == 0))
+				max_pkt_size) == 0))
 			urb_state = US_BULK0;
 		while (data_len > 4096) {
 			td = fhci_td_fill(fhci, urb, urb_priv, ed, cnt,
@@ -807,8 +808,8 @@ void fhci_queue_urb(struct fhci_hcd *fhci, struct urb *urb)
 		break;
 	case FHCI_TF_CTRL:
 		ed->dev_addr = usb_pipedevice(urb->pipe);
-		ed->max_pkt_size = usb_maxpacket(urb->dev, urb->pipe,
-			usb_pipeout(urb->pipe));
+		ed->max_pkt_size = usb_endpoint_maxp(&urb->ep->desc);
+
 		/* setup stage */
 		td = fhci_td_fill(fhci, urb, urb_priv, ed, cnt++, FHCI_TA_SETUP,
 			USB_TD_TOGGLE_DATA0, urb->setup_packet, 8, 0, 0, true);
-- 
1.8.5.6