| Message ID | 20210307090030.22369-1-baijiaju1990@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb: renesas_usbhs: fix error return code of usbhsf_pkt_handler() | expand |
Hi Jia-Ju, Thank you for the patch! > From: Jia-Ju Bai, Sent: Sunday, March 7, 2021 6:01 PM > > When __usbhsf_pkt_get() returns NULL to pkt, no error return code of > usbhsf_pkt_handler() is assigned. Yes. Also I realized that no error return code of usbhsf_pkt_handler() was assigned if the type value was unexpected value. So, I'm thinking initial value of ret should be -EINVAL instead of 0. --- int ret = 0; // should be -EINVAL int is_done = 0; /******************** spin lock ********************/ usbhs_lock(priv, flags); pkt = __usbhsf_pkt_get(pipe); if (!pkt) goto __usbhs_pkt_handler_end; switch (type) { case USBHSF_PKT_PREPARE: func = pkt->handler->prepare; break; case USBHSF_PKT_TRY_RUN: func = pkt->handler->try_run; break; case USBHSF_PKT_DMA_DONE: func = pkt->handler->dma_done; break; default: dev_err(dev, "unknown pkt handler\n"); goto __usbhs_pkt_handler_end; /// here } if (likely(func)) /// [1] ret = func(pkt, &is_done); [1] This is always true here, so ret is always assigned by the func(). --- > To fix this bug, ret is assigned with -EINVAL in this case. Just a record: After fixed this, actual behavior is almost the same except printing error message. Best regards, Yoshihiro Shimoda
Thanks for the reply! On 2021/3/9 19:59, Yoshihiro Shimoda wrote: > Hi Jia-Ju, > > Thank you for the patch! > >> From: Jia-Ju Bai, Sent: Sunday, March 7, 2021 6:01 PM >> >> When __usbhsf_pkt_get() returns NULL to pkt, no error return code of >> usbhsf_pkt_handler() is assigned. > Yes. Also I realized that no error return code of usbhsf_pkt_handler() > was assigned if the type value was unexpected value. So, I'm thinking > initial value of ret should be -EINVAL instead of 0. This is okay to me. Need I write a new patch for this? Best wishes, Jia-Ju Bai
Hi Jia-Ju, > From: Jia-Ju Bai, Sent: Tuesday, March 9, 2021 10:39 PM > On 2021/3/9 19:59, Yoshihiro Shimoda wrote: > > Hi Jia-Ju, > > > > Thank you for the patch! > > > >> From: Jia-Ju Bai, Sent: Sunday, March 7, 2021 6:01 PM > >> > >> When __usbhsf_pkt_get() returns NULL to pkt, no error return code of > >> usbhsf_pkt_handler() is assigned. > > Yes. Also I realized that no error return code of usbhsf_pkt_handler() > > was assigned if the type value was unexpected value. So, I'm thinking > > initial value of ret should be -EINVAL instead of 0. > > This is okay to me. > Need I write a new patch for this? Thank you for your reply. I can write such a new patch with your Reported-by for this as minor refactoring of the usbhsf_pkt_handler(). May I write such a patch? Best regards, Yoshihiro Shimoda
On 2021/3/10 10:54, Yoshihiro Shimoda wrote: > Hi Jia-Ju, > >> From: Jia-Ju Bai, Sent: Tuesday, March 9, 2021 10:39 PM >> On 2021/3/9 19:59, Yoshihiro Shimoda wrote: >>> Hi Jia-Ju, >>> >>> Thank you for the patch! >>> >>>> From: Jia-Ju Bai, Sent: Sunday, March 7, 2021 6:01 PM >>>> >>>> When __usbhsf_pkt_get() returns NULL to pkt, no error return code of >>>> usbhsf_pkt_handler() is assigned. >>> Yes. Also I realized that no error return code of usbhsf_pkt_handler() >>> was assigned if the type value was unexpected value. So, I'm thinking >>> initial value of ret should be -EINVAL instead of 0. >> This is okay to me. >> Need I write a new patch for this? > Thank you for your reply. I can write such a new patch with your > Reported-by for this as minor refactoring of the usbhsf_pkt_handler(). > May I write such a patch? Okay, sure :) Best wishes, Jia-Ju Bai
> From: Jia-Ju Bai, On 2021/3/10 10:54, Yoshihiro Shimoda wrote: > >> From: Jia-Ju Bai, Sent: Tuesday, March 9, 2021 10:39 PM > >> On 2021/3/9 19:59, Yoshihiro Shimoda wrote: > >>>> From: Jia-Ju Bai, Sent: Sunday, March 7, 2021 6:01 PM > >>>> > >>>> When __usbhsf_pkt_get() returns NULL to pkt, no error return code of > >>>> usbhsf_pkt_handler() is assigned. > >>> Yes. Also I realized that no error return code of usbhsf_pkt_handler() > >>> was assigned if the type value was unexpected value. So, I'm thinking > >>> initial value of ret should be -EINVAL instead of 0. > >> This is okay to me. > >> Need I write a new patch for this? > > Thank you for your reply. I can write such a new patch with your > > Reported-by for this as minor refactoring of the usbhsf_pkt_handler(). > > May I write such a patch? > > Okay, sure :) I got it :) Best regards, Yoshihiro Shimoda
diff --git a/drivers/usb/renesas_usbhs/fifo.c b/drivers/usb/renesas_usbhs/fifo.c index e6fa13701808..b5e7991dc7d9 100644 --- a/drivers/usb/renesas_usbhs/fifo.c +++ b/drivers/usb/renesas_usbhs/fifo.c @@ -160,8 +160,10 @@ static int usbhsf_pkt_handler(struct usbhs_pipe *pipe, int type) usbhs_lock(priv, flags); pkt = __usbhsf_pkt_get(pipe); - if (!pkt) + if (!pkt) { + ret = -EINVAL; goto __usbhs_pkt_handler_end; + } switch (type) { case USBHSF_PKT_PREPARE:
When __usbhsf_pkt_get() returns NULL to pkt, no error return code of usbhsf_pkt_handler() is assigned. To fix this bug, ret is assigned with -EINVAL in this case. Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> --- drivers/usb/renesas_usbhs/fifo.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)