mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] configfs: fix NULL dereference in configfs_depend_item_unlocked()
@ 2026-09-14  9:44 pavankumaryalagada
  2026-09-14 10:05 ` Breno Leitao
  0 siblings, 1 reply; 3+ messages in thread
From: pavankumaryalagada @ 2026-09-14  9:44 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andreas Hindborg, Nicholas Bellinger, Sebastian Andrzej Siewior,
	Andrzej Pietrasiewicz, linux-kernel, Shuah Khan,
	Yalagada Pavan Kumar, syzbot+a9efa71b884a23e74153, stable

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] configfs: fix NULL dereference in configfs_depend_item_unlocked()
  2026-09-14  9:44 [PATCH] configfs: fix NULL dereference in configfs_depend_item_unlocked() pavankumaryalagada
@ 2026-09-14 10:05 ` Breno Leitao
  2026-09-14 13:27   ` Yalagada Pavan Kumar
  0 siblings, 1 reply; 3+ messages in thread
From: Breno Leitao @ 2026-09-14 10:05 UTC (permalink / raw)
  To: pavankumaryalagada
  Cc: Andreas Hindborg, Nicholas Bellinger, Sebastian Andrzej Siewior,
	Andrzej Pietrasiewicz, linux-kernel, Shuah Khan,
	syzbot+a9efa71b884a23e74153, stable

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] configfs: fix NULL dereference in configfs_depend_item_unlocked()
  2026-09-14 10:05 ` Breno Leitao
@ 2026-09-14 13:27   ` Yalagada Pavan Kumar
  0 siblings, 0 replies; 3+ messages in thread
From: Yalagada Pavan Kumar @ 2026-09-14 13:27 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andreas Hindborg, Nicholas Bellinger, Sebastian Andrzej Siewior,
	Andrzej Pietrasiewicz, linux-kernel, Shuah Khan,
	syzbot+a9efa71b884a23e74153, stable

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-14 13:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-14  9:44 [PATCH] configfs: fix NULL dereference in configfs_depend_item_unlocked() pavankumaryalagada
2026-09-14 10:05 ` Breno Leitao
2026-09-14 13:27   ` Yalagada Pavan Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®