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

pavankumaryalagada@gmail.com posted 1 patch 1 week, 4 days ago
fs/configfs/dir.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] configfs: fix NULL dereference in configfs_depend_item_unlocked()
Posted by pavankumaryalagada@gmail.com 1 week, 4 days ago
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.

Reported-by: syzbot+a9efa71b884a23e74153@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a9efa71b884a23e74153
Fixes: 4bb8548df632 ("usb: gadget: f_tcm: add configfs support")
Cc: stable@vger.kernel.org
Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
---
 fs/configfs/dir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index eda80c2a2d38..301a64f41887 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -1244,6 +1244,9 @@ int configfs_depend_item_unlocked(struct configfs_subsystem *caller_subsys,
 		return -EINVAL;
 
 	parent = target->ci_group;
+	if (!parent)
+		return -ENOENT;
+
 	/*
 	 * This may happen when someone is trying to depend root
 	 * directory of some subsystem
-- 
2.43.0
Re: [PATCH] configfs: fix NULL dereference in configfs_depend_item_unlocked()
Posted by Breno Leitao 1 week, 3 days ago
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:

 /*
  * 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?

> 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.

--breno
Re: [PATCH] configfs: fix NULL dereference in configfs_depend_item_unlocked()
Posted by Yalagada Pavan Kumar 1 week, 3 days ago
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