On Fri, May 13, 2022 at 02:57:09PM +0800, Albert Wang wrote:
> After inspecting further, we do see the locking is implicit, with the
> main gotcha being the unlock/re-lock.
This sentance makes no sense at all.
Who is "we"? What is the gotcha? What is the subject of the sentance?
What is going on?
> That creates a window for a race to happen.
What is "that"?
> This change moves the NULL check to be adjacent to where
> to it's used and after the window is "closed".
What is "this"?
Please read Documentation/process/submitting-patches.rst for how to
properly write a changelog text so that others can understand what is
going on.
thanks,
greg k-h
>
> Fixes: 26288448120b ("usb: dwc3: gadget: Fix null pointer exception")
> Signed-off-by: Albert Wang <albertccwang@xxxxxxxxxx>
> ---
> v3: Add change log to be compliant with the canonical patch format
> v2: Remove redundant 'else' and add additional comments and more
> descriptive commit text
>
> drivers/usb/dwc3/gadget.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 19477f4bbf54..fda58951cf27 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3366,14 +3366,19 @@ static bool dwc3_gadget_endpoint_trbs_complete(struct dwc3_ep *dep,
> struct dwc3 *dwc = dep->dwc;
> bool no_started_trb = true;
>
> - if (!dep->endpoint.desc)
> - return no_started_trb;
> -
> + /*
> + * This function eventually leads to dwc3_giveback() which unlocks
> + * the dwc->lock and relocks afterwards. This actually creates a
> + * a window for a race to happen.
What race? Why mention it here? Why not fix it instead of documenting
it?
this comment does not make sense, sorry.
thanks,
greg k-h
