Re: [PATCH] usb: gadget: f_tcm: fix remaining nexus NULL dereferences

[Guangshuo Li]

Hi Thinh,

Thanks for the review.

On Sat, 27 Jun 2026 at 08:32, Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
>
> On Thu, Jun 04, 2026, Guangshuo Li wrote:
> > The previous nexus NULL-dereference fix added checks to the normal
> > command submission paths, but two UASP paths still dereference
> > tpg->tpg_nexus without checking it first.
> >
> > A TASK MANAGEMENT request reaches usbg_submit_tmr(), which fetches
> > tvn_se_sess directly from tpg->tpg_nexus. The RC_OVERLAPPED_TAG path in
> > usbg_cmd_work() does the same before walking sess_cmd_map for the active
> > command with the same tag.
> >
> > If userspace drops the nexus after the command is queued, these paths can
> > observe a NULL tpg_nexus and crash before they can ignore the command like
> > the already-fixed command paths do.
> >
> > Check tpg_nexus in both remaining paths and use the checked local nexus
> > before dereferencing tvn_se_sess.
> >
> > Fixes: b9fde5073553 ("usb: gadget: f_tcm: Fix NULL pointer dereferences in nexus handling")
> > Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> > ---
> >  drivers/usb/gadget/function/f_tcm.c | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> > index 34d9f49e9987..1717fdd1c466 100644
> > --- a/drivers/usb/gadget/function/f_tcm.c
> > +++ b/drivers/usb/gadget/function/f_tcm.c
> > @@ -1188,12 +1188,21 @@ static void usbg_aborted_task(struct se_cmd *se_cmd);
> >
> >  static void usbg_submit_tmr(struct usbg_cmd *cmd)
> >  {
> > +     struct tcm_usbg_nexus *tv_nexus;
> >       struct se_session *se_sess;
> >       struct se_cmd *se_cmd;
> >       int flags = TARGET_SCF_ACK_KREF;
> >
> >       se_cmd = &cmd->se_cmd;
> > -     se_sess = cmd->fu->tpg->tpg_nexus->tvn_se_sess;
> > +     tv_nexus = cmd->fu->tpg->tpg_nexus;
> > +     if (!tv_nexus) {
> > +             struct usb_gadget *gadget = fuas_to_gadget(cmd->fu);
> > +
> > +             dev_err(&gadget->dev, "Missing nexus for TMR, ignoring command\n");
>
> This is called after usbg_get_cmd(). We need to cleanup the
> sbitmap_queue and the hash (sbitmap_queue_clear and hash_del) before
> returning.
>
> > +             return;
> > +     }
> > +
> > +     se_sess = tv_nexus->tvn_se_sess;
> >
> >       target_submit_tmr(se_cmd, se_sess,
> >                         cmd->response_iu.add_response_info,
> > @@ -1271,12 +1280,21 @@ static void usbg_cmd_work(struct work_struct *work)
> >  skip:
> >       if (cmd->tmr_rsp == RC_OVERLAPPED_TAG) {
> >               struct f_uas *fu = cmd->fu;
> > +             struct tcm_usbg_nexus *tv_nexus;
> >               struct se_session *se_sess;
> >               struct uas_stream *stream = NULL;
> >               struct hlist_node *tmp;
> >               struct usbg_cmd *active_cmd = NULL;
> >
> > -             se_sess = cmd->fu->tpg->tpg_nexus->tvn_se_sess;
> > +             tv_nexus = fu->tpg->tpg_nexus;
> > +             if (!tv_nexus) {
>
> Same here. Also check for other if (!tv_nexus) checks.
>
> Maybe we need a helper function to do this cleanup?
>
> Thanks,
> Thinh
>
> > +                     struct usb_gadget *gadget = fuas_to_gadget(fu);
> > +
> > +                     dev_err(&gadget->dev, "Missing nexus for overlapped tag, ignoring command\n");
> > +                     return;
> > +             }
> > +
> > +             se_sess = tv_nexus->tvn_se_sess;
> >
> >               hash_for_each_possible_safe(fu->stream_hash, stream, tmp, node, cmd->tag) {
> >                       int i = stream - &fu->stream[0];
> > --
> > 2.43.0
> >

I have sent v2 to clean up the session tag pool and stream hash before
returning from the nexus-missing paths. I also stored the session
pointer in struct usbg_cmd so the cleanup path does not need to
dereference tpg_nexus after it becomes NULL.

Thanks,
Guangshuo