Re: [PATCH] usb: cdns3: Do not access memory after free

[Siddharth Vadapalli <s-vadapalli@ti.com>]

On Fri, Aug 22, 2025 at 04:55:49PM +0200, Marek Vasut wrote:
> On 8/22/25 4:14 PM, Siddharth Vadapalli wrote:
> > On Thu, Aug 21, 2025 at 10:22:43PM +0200, Marek Vasut wrote:
> > > On 8/21/25 8:16 AM, Siddharth Vadapalli wrote:
> > > 
> > > Hi,
> > > 
> > > > > > > > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> > > > > > > > index a30c40ef80e..9eaf7e40ab6 100644
> > > > > > > > --- a/drivers/usb/cdns3/gadget.c
> > > > > > > > +++ b/drivers/usb/cdns3/gadget.c
> > > > > > > > @@ -557,10 +557,10 @@ static void
> > > > > > > > cdns3_wa2_remove_old_request(struct cdns3_endpoint *priv_ep)
> > > > > > > >             trace_cdns3_wa2(priv_ep, "removes eldest request");
> > > > > > > > +        list_del_init(&priv_req->list);
> > > > > > > >             kfree(priv_req->request.buf);
> > > > > > > >             cdns3_gadget_ep_free_request(&priv_ep->endpoint,
> > > > > > > >                              &priv_req->request);
> > > > > > > > -        list_del_init(&priv_req->list);
> > > > > > > 
> > > > > > > Shouldn't the kfree() be moved here instead ?
> > > > > > > cdns3_gadget_ep_free_request() also accesses priv_req->request .
> > > > > > 
> > > > > > No, I do not think so. The kfree frees priv_req->request.buf not
> > > > > > priv_req->request so must happen before the call to
> > > > > > cdns3_gadget_ep_free_request.
> > > > > Thank you for clarifying. Please add this into the commit message, ideally
> > > > > wait for TI to test this and provide RB, then send V2 so this can go in.
> > > > 
> > > > I was planning to test this patch but the change being made is only
> > > > applicable to Controller Versions:
> > > > 	#define DEV_VER_NXP_V1		0x00024502
> > > > 	#define DEV_VER_TI_V1		0x00024509
> > > > and not to:
> > > > 	#define DEV_VER_V2		0x0002450C
> > > > 	#define DEV_VER_V3		0x0002450d
> > > > 
> > > > Since I don't have an SoC and a Board with DEV_VER_TI_V1, I cannot test
> > > > it. However, the change looks correct to me.
> > > > 
> > > > Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> > > The change does indeed look correct.
> > > 
> > > Do you know who might still have that board and could test ? (and which
> > > board/soc is that) ?
> > 
> > None of the boards that I have worked with have a DEV_VER_TI_V1 version
> > of the controller. I also tried to use the Linux device-tree to check if
> > I could identify the SoC/board but I was unable to do so.
> Do you know which SoC is V2 and V3 ?

I spent more time on this and found out that J721E SR 1.0 has the
controller with DEV_VER_TI_V1 version but other revisions of J721E as
well as all of the following SoCs have DEV_VER_V3 version of the
controller:
AM64, AM68, AM69, J7200, J721S2, J722S, J742S2 and J784S4.

I will try to find an SR 1.0 J721E SoC and test the patch on it and
share the results here.

Regards,
Siddharth.