Re: [PATCH] configfs: fix NULL dereference in configfs_depend_item_unlocked()

[Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>]

On Mon, Sep 14, 2026 at 03:05:26AM -0700, Breno Leitao wrote:
> On Mon, Sep 14, 2026 at 03:14:26PM +0530, pavankumaryalagada@gmail.com wrote:
> > From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
> > 
> > configfs_depend_item_unlocked() can dereference a NULL parent when
> > the configfs item is still being created.
> > 
> > The item can be found before it is linked to its parent group,
> > so ci_group can still be NULL. Check ci_group before using it
> > and return -ENOENT if it is NULL.
> 
> The crash itself looks real, but this reads like a fix in the wrong
> place.  include/linux/configfs.h already documents the opposite
> contract for this API:

I agree, I checked the code path again. The race is in f_tcm. The function
instace is published before its configfs item is linked, so usbg_make_tpg()
can see it while ci_group is NULL and pass it to configfs_depend_item_unlocked().

For v2, I will fix the caller in f_tcm.c by checking ci_group before calling
configfs_depend_item_unlocked().

> 
>  /*
>   * These functions can sleep and can alloc with GFP_KERNEL
>   * NOTE: These should be called only underneath configfs callbacks.
>   * NOTE: First parameter is a caller's subsystem, not target's.
>   * WARNING: These cannot be called on newly created item
>   *        (in make_group()/make_item() callback)
>   */
>  int configfs_depend_item_unlocked(struct configfs_subsystem *caller_subsys,
>                                    struct config_item *target);
> 
> "cannot be called on newly created item" is exactly the case being
> patched here. 
> 
> If configfs is now going to accept a not-yet-linked item and return
> -ENOENT for it, shouldn't that comment be updated in the same patch?
> 
> And if the contract is meant to stand, shouldn't the caller stop handing
> over an unlinked item instead?

Yes, the existing configfs API contract should remain unchanged. I will not modify it.

Instead, in v2 i will add check before passing it to configfs_depend_item_unlocked().

> 
> > Fixes: 4bb8548df632 ("usb: gadget: f_tcm: add configfs support")
> 
> This tag and the diff do not agree.  4bb8548df632 only touches
> Documentation/ABI, drivers/usb/gadget/Kconfig and
> drivers/usb/gadget/function/f_tcm.c; it does not add or modify anything
> under fs/configfs.

You're right. My previous fix was in wrong place.

For v2, i will move the fix to drivers/usb/gadget/function/f_tcm. I will keep the
Fixes: 4bb8548df632 ("usb: gadget: f_tcm: add configfs support") tag because this
commit introduced the f_tcm configfs dependency handling involved in this bug.

Thanks,
Pavan

> 
> --breno