Hi Yu,
On Sun, 9 Jul 2023 17:48:15 -0700
Yu Hao <yhao016@xxxxxxx> wrote:
> Hi Hervé,
>
> Thanks for the comments. How about this patch?
> ---
> drivers/usb/mtu3/mtu3_gadget_ep0.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/mtu3/mtu3_gadget_ep0.c
> b/drivers/usb/mtu3/mtu3_gadget_ep0.c
> index e4fd1bb14a55..af2884943c2a 100644
> --- a/drivers/usb/mtu3/mtu3_gadget_ep0.c
> +++ b/drivers/usb/mtu3/mtu3_gadget_ep0.c
> @@ -600,7 +600,7 @@ static void ep0_tx_state(struct mtu3 *mtu)
> mtu3_readl(mtu->mac_base, U3D_EP0CSR));
> }
>
> -static void ep0_read_setup(struct mtu3 *mtu, struct usb_ctrlrequest *setup)
> +static int ep0_read_setup(struct mtu3 *mtu, struct usb_ctrlrequest *setup)
> {
> struct mtu3_request *mreq;
> u32 count;
> @@ -608,6 +608,8 @@ static void ep0_read_setup(struct mtu3 *mtu,
> struct usb_ctrlrequest *setup)
>
> csr = mtu3_readl(mtu->mac_base, U3D_EP0CSR) & EP0_W1C_BITS;
> count = mtu3_readl(mtu->mac_base, U3D_RXCOUNT0);
> + if (count == 0)
> + return -EINVAL;
'count' should be tested against sizeof(*setup). Indeed, we need to have a
setup data packet in the fifo.
What do you think about:
if (count < sizef(*setup))
return -EINVAL;
>
> ep0_read_fifo(mtu->ep0, (u8 *)setup, count);
>
> @@ -642,7 +644,8 @@ __acquires(mtu->lock)
> struct mtu3_request *mreq;
> int handled = 0;
>
> - ep0_read_setup(mtu, &setup);
> + if (ep0_read_setup(mtu, &setup))
> + return -EINVAL;
Forward the error code to the caller ?
ret = ep0_read_setup(mtu, &setup)
if (ret < 0)
return ret;
> trace_mtu3_handle_setup(&setup);
>
> if ((setup.bRequestType & USB_TYPE_MASK) == USB_TYPE_STANDARD)
> @@ -764,7 +767,9 @@ irqreturn_t mtu3_ep0_isr(struct mtu3 *mtu)
> break;
> }
>
> - ep0_handle_setup(mtu);
> + if (ep0_handle_setup(mtu))
> + break;
> +
Ok
> ret = IRQ_HANDLED;
> break;
> default:
Be careful, your patch is wrongly indented.
tabs replaced by 4 spaces. You need to keep tabs.
Regards,
Hervé Codina
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
