Re: [PATCH] usb: gadget: f_tcm: Fix NULL pointer dereferences in nexus handling

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Thu, Feb 19, 2026, Jiasheng Jiang wrote:
> On Thu, 19 Feb 2026 01:30:16 +0000, Thinh Nguyen wrote:
> > On Wed, Feb 18, 2026, Sebastian Andrzej Siewior wrote:
> >> On 2026-02-18 05:22:21 [+0000], Thinh Nguyen wrote:
> >> > > Fixes: 08a1cb0f65fd ("usb: gadget: tcm: factor out f_tcm")
> >> > > Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com>
> >> …
> >> > While the patch itself is fine, we should prevent this situation from
> >> > occurring in the first place. That is, we should enforce the config
> >> > dependency and prevent the users from removing the nexus if the gadget
> >> > driver is bound. Likewise, we should prevent the gadget driver from
> >> > binding if no nexus is established.
> >> 
> >> Is this an actual problem or just something that looks like it could
> >> happen? My memory is that the tcm holds a reference on it and the
> >> referenced commit just split/moved  the code. So if it is a problem then
> >> it should have been there longer than that.
> >> 
> > 
> > Looks like we only hold the reference to the session cmds, and we track
> > the tpg_port_count if there's an established link. But we don't guard or
> > deactivate the f_tcm usb function when we unregister the tcm and remove
> > the nexus. As far as the host can tell, the device is still connected
> > and the function is still active.
> > 
> > This exists since the beginning. The Fixes tag should have pointed to
> > c52661d60f63 ("usb-gadget: Initial merge of target module for UASP + BOT")
> > 
> > We can guard against unlinking the port while the gadget_driver is bound
> > or we can just deactivate the usb function when that happens. The latter
> > is probably a better option.
> > 
> > BR,
> > Thinh
> 
> Thanks for the clarification.
> 
> I understand your point about "The latter" option: we should deactivate
> the USB function when the nexus is dropped to stop new traffic from the
> host.
> 
> However, even if we implement the deactivation logic in
> tcm_usbg_drop_nexus, there could still be a race condition where pending
> work items (e.g., bot_cmd_work) are already scheduled but haven't executed
> yet.

Yes, we'd need to sync and cancel them properly.

> 
> Therefore, I believe the if (!tv_nexus) checks in this patch are still
> necessary as a safeguard to drain pending requests safely without
> crashing.
> 
> My Proposal:
> Can we proceed with this patch (adding the NULL checks and fixing the
> Fixes tag) to close the immediate panic? The architectural change to
> deactivate the function could then be handled in a separate follow-up
> patch, as it might require more extensive testing.
> 
> Does that sound reasonable to you?

Sounds reasonable to me. As I noted previously, this patch itself is
fine. However, please update your "Fixes" tag to point to the correct
commit. You should also add Cc stable.

Thanks,
Thinh