drivers/target/target_core_configfs.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
In flush_write_buffer, &p->frag_sem is acquired and then the loaded store
function is called, which, here, is target_core_item_dbroot_store().
This function called filp_open(), following which these functions were
called (in reverse order), according to the call trace:
down_read
__configfs_open_file
do_dentry_open
vfs_open
do_open
path_openat
do_filp_open
file_open_name
filp_open
target_core_item_dbroot_store
flush_write_buffer
configfs_write_iter
Hence ultimately, __configfs_open_file() was called, indirectly by
target_core_item_dbroot_store(), and it also attempted to acquire
&p->frag_sem, which was already held by the same thread, acquired earlier
in flush_write_buffer. This poses a possibility of recursive locking,
which triggers the lockdep warning.
Fix this by modifying target_core_item_dbroot_store() to use kern_path()
instead of filp_open() to avoid opening the file using filesystem-specific
function __configfs_open_file(), and further modifying it to make this
fix compatible.
Reported-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f6e8174215573a84b797
Tested-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>
---
drivers/target/target_core_configfs.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index b19acd662726..f29052e6a87d 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -108,8 +108,8 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
const char *page, size_t count)
{
ssize_t read_bytes;
- struct file *fp;
ssize_t r = -EINVAL;
+ struct path path = {};
mutex_lock(&target_devices_lock);
if (target_devices) {
@@ -131,17 +131,18 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
db_root_stage[read_bytes - 1] = '\0';
/* validate new db root before accepting it */
- fp = filp_open(db_root_stage, O_RDONLY, 0);
- if (IS_ERR(fp)) {
+ r = kern_path(db_root_stage, LOOKUP_FOLLOW, &path);
+ if (r) {
pr_err("db_root: cannot open: %s\n", db_root_stage);
goto unlock;
}
- if (!S_ISDIR(file_inode(fp)->i_mode)) {
- filp_close(fp, NULL);
+ if (!d_is_dir(path.dentry)) {
+ path_put(&path);
pr_err("db_root: not a directory: %s\n", db_root_stage);
+ r = -ENOTDIR;
goto unlock;
}
- filp_close(fp, NULL);
+ path_put(&path);
strscpy(db_root, db_root_stage);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
--
2.34.1On 1/8/26 12:15 PM, Prithvi Tambewagh wrote: > This poses a possibility of recursive locking, > which triggers the lockdep warning. Patches that fix a lockdep complaint should include the full lockdep complaint. Since the fixed lockdep complaint didn't trigger a deadlock it must be a false positive complaint, isn't it? Such complaints should be fixed but without additional information we can't tell what the best way is to fix the complaint. Thanks, Bart.
On Thu, Jan 15, 2026 at 08:57:28AM -0800, Bart Van Assche wrote:
> On 1/8/26 12:15 PM, Prithvi Tambewagh wrote:
> > This poses a possibility of recursive locking,
> > which triggers the lockdep warning.
>
> Patches that fix a lockdep complaint should include the full lockdep
> complaint.
>
> Since the fixed lockdep complaint didn't trigger a deadlock it must be
> a false positive complaint, isn't it? Such complaints should be fixed
> but without additional information we can't tell what the best way is to
> fix the complaint.
>
> Thanks,
>
> Bart.
Hello Bart,
Here is the full lockdep complaint, as per the syzkaller dashboard report
for the bug:
============================================
WARNING: possible recursive locking detected
syzkaller #0 Not tainted
--------------------------------------------
syz.0.17/5999 is trying to acquire lock:
ffff888140413f78 (&p->frag_sem){.+.+}-{4:4}, at: __configfs_open_file+0xe8/0x9c0 fs/configfs/file.c:304
but task is already holding lock:
ffff888140413f78 (&p->frag_sem){.+.+}-{4:4}, at: flush_write_buffer fs/configfs/file.c:205 [inline]
ffff888140413f78 (&p->frag_sem){.+.+}-{4:4}, at: configfs_write_iter+0x219/0x4e0 fs/configfs/file.c:229
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&p->frag_sem);
lock(&p->frag_sem);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by syz.0.17/5999:
#0: ffff888147ab0420 (sb_writers#12){.+.+}-{0:0}, at: ksys_write+0x12a/0x250 fs/read_write.c:738
#1: ffff888077d2d688 (&buffer->mutex){+.+.}-{4:4}, at: configfs_write_iter+0x75/0x4e0 fs/configfs/file.c:226
#2: ffff888140413f78 (&p->frag_sem){.+.+}-{4:4}, at: flush_write_buffer fs/configfs/file.c:205 [inline]
#2: ffff888140413f78 (&p->frag_sem){.+.+}-{4:4}, at: configfs_write_iter+0x219/0x4e0 fs/configfs/file.c:229
#3: ffffffff8f4097e8 (target_devices_lock){+.+.}-{4:4}, at: target_core_item_dbroot_store+0x21/0x350 drivers/target/target_core_configfs.c:114
stack backtrace:
CPU: 0 UID: 0 PID: 5999 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
print_deadlock_bug+0x1e9/0x240 kernel/locking/lockdep.c:3041
check_deadlock kernel/locking/lockdep.c:3093 [inline]
validate_chain kernel/locking/lockdep.c:3895 [inline]
__lock_acquire+0x1106/0x1c90 kernel/locking/lockdep.c:5237
lock_acquire kernel/locking/lockdep.c:5868 [inline]
lock_acquire+0x179/0x350 kernel/locking/lockdep.c:5825
down_read+0x9b/0x480 kernel/locking/rwsem.c:1537
__configfs_open_file+0xe8/0x9c0 fs/configfs/file.c:304
do_dentry_open+0x982/0x1530 fs/open.c:965
vfs_open+0x82/0x3f0 fs/open.c:1097
do_open fs/namei.c:3975 [inline]
path_openat+0x1de4/0x2cb0 fs/namei.c:4134
do_filp_open+0x20b/0x470 fs/namei.c:4161
file_open_name+0x2a3/0x450 fs/open.c:1381
filp_open+0x4b/0x80 fs/open.c:1401
target_core_item_dbroot_store+0x108/0x350 drivers/target/target_core_configfs.c:134
flush_write_buffer fs/configfs/file.c:207 [inline]
configfs_write_iter+0x306/0x4e0 fs/configfs/file.c:229
new_sync_write fs/read_write.c:593 [inline]
vfs_write+0x7d3/0x11d0 fs/read_write.c:686
ksys_write+0x12a/0x250 fs/read_write.c:738
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0xfa0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fbf49d8eec9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffd1d0ac1e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007fbf49fe5fa0 RCX: 00007fbf49d8eec9
RDX: 0000000000000fff RSI: 0000200000000000 RDI: 0000000000000003
RBP: 00007fbf49e11f91 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fbf49fe5fa0 R14: 00007fbf49fe5fa0 R15: 0000000000000003
</TASK>
db_root: not a directory: /sys/kernel/config/target/dbroot
Sorry, but I didn't get why this might be a false positive lockdep complaint,
can you please guide?
From what I understood, in this case, the same rw_semaphore &p->frag_sem is used,
identified by the same addresses (ffff888140413f78), for both the lock held
as well as one being tried to be acquired. This occurs since
flush_write_buffer() called in configfs_write_iter() first acquires the
&p->frag_sem lock and then next flush_write_buffer() calls
target_core_item_dbroot_store(), which attempts to open file with path stored
in db_root_stage using filp_open(). It ultimately calls __configfs_open_file()
which again tries to acquire exactly the same lock, &p->frag_sem.
A deadlock can arise, if a writer tries to acquire this lock after it was first
acquired by that thread in flush_write_buffer() and before acquiring it again in
__configfs_open_file(), trying to avoid writer starvation. After checking, I
found out that down_write() is called for frag_sem, the rw_semaphore in struct
configfs_fragment, at 3 places:
1. configfs_rmdir() - calls down_write_killable(&frag->frag_sem)
2. configfs_unregister_group() - calls down_write(&frag->frag_sem);
3. configfs_unregister_subsystem() - calls down_write(&frag->frag_sem);
I think any of these may result in a deadlock due to recursive locking, if
the lock is acquired by a writer after being acquired by a reader and then
again being tried to be acquired by a reader.
I attempt to solve this by replaing call to filp_open() in
target_core_item_dbroot_store() with kern_path(), which just checks if a file
path exists, as required in target_core_item_dbroot_store(), rather than
actually opening the file and using the same frag_sem lock, which removes
the possiblity of recursive deadlock on this path. What do you think of this
approach?
Best Regards,
Prithvi
On 1/19/26 10:50 AM, Prithvi wrote: > Possible unsafe locking scenario: > > CPU0 > ---- > lock(&p->frag_sem); > lock(&p->frag_sem); The least intrusive way to suppress this type of lockdep complaints is by using lockdep_register_key() and lockdep_unregister_key(). Thanks, Bart.
On Tue, Jan 20, 2026 at 05:48:16AM -0800, Bart Van Assche wrote: > On 1/19/26 10:50 AM, Prithvi wrote: > > Possible unsafe locking scenario: > > > > CPU0 > > ---- > > lock(&p->frag_sem); > > lock(&p->frag_sem); > The least intrusive way to suppress this type of lockdep complaints is > by using lockdep_register_key() and lockdep_unregister_key(). > > Thanks, > > Bart. Hello Bart, I tried using lockdep_register_key() and lockdep_unregister_key() for the frag_sem lock, however it stil gives the possible recursive locking warning. Here is the patch and the bug report from its test: https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#m3203ceddf3423b7116ba9225d182771608f93a6f Would using down_read_nested() and subclasses be a better option here? I also checked out some documentation regarding it and learnt that to use the _nested() form, the hierarchy among the locks should be mapped accurately; however, IIUC, there isn't any hierarchy between the locks in this case, is this right? Apologies if I am missing something obvious here, and thanks for your time and guidance. Best Regards, Prithvi
On Wed, Jan 21, 2026 at 11:21:46PM +0530, Prithvi wrote: > On Tue, Jan 20, 2026 at 05:48:16AM -0800, Bart Van Assche wrote: > > On 1/19/26 10:50 AM, Prithvi wrote: > > > Possible unsafe locking scenario: > > > > > > CPU0 > > > ---- > > > lock(&p->frag_sem); > > > lock(&p->frag_sem); > > The least intrusive way to suppress this type of lockdep complaints is > > by using lockdep_register_key() and lockdep_unregister_key(). > > > > Thanks, > > > > Bart. > > Hello Bart, > > I tried using lockdep_register_key() and lockdep_unregister_key() for the > frag_sem lock, however it stil gives the possible recursive locking > warning. Here is the patch and the bug report from its test: > > https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#m3203ceddf3423b7116ba9225d182771608f93a6f > > Would using down_read_nested() and subclasses be a better option here? > > I also checked out some documentation regarding it and learnt that to use > the _nested() form, the hierarchy among the locks should be mapped > accurately; however, IIUC, there isn't any hierarchy between the locks in > this case, is this right? > > Apologies if I am missing something obvious here, and thanks for your > time and guidance. > > Best Regards, > Prithvi Hello Andreas and Breno, This thread is regarding a patch for fixing possible deadlock in __configfs_open_file(); here is its dashboard link: https://syzkaller.appspot.com/bug?extid=f6e8174215573a84b797 Firstly, flush_write_buffer() is called, which acquires frag_sem lock, and then it calls the loaded store function, which, in this case, is target_core_item_dbroot_store(). target_core_item_dbroot_store() calls filp_open(), which ultimately calls configfs_write_iter() and in it, the thread tries to acquire frag_sem lock again, posing a possibility of recursive locking. In the initial patch, I tried to fix this by replacing the call to filp_open() in target_core_item_dbroot_store() with kern_path(), since it performs the task of checking if the file path exists, rather than opening the file using configfs_write_iter(). This also avoids acquiring frag_sem in nested manner and thus possibiliy of recursive locking is prevented. After checking I found 3 functions where down-write() is used, which, IIUC they might contribute to recursive locking: 1. configfs_rmdir() - calls down_write_killable(&frag->frag_sem) 2. configfs_unregister_group() - calls down_write(&frag->frag_sem); 3. configfs_unregister_subsystem() - calls down_write(&frag->frag_sem); Bart suggested that this can be a fals positive and can be solved using lockdep_register_key() and lockdep_unregister_key(). However, on trying this approach, the possibile recursive locking warning persisted, it can be found here: https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#m3203ceddf3423b7116ba9225d182771608f93a6f IIUC, we can then use down_read_nested() and lock subclasses here; but, according to documentation: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/locking/lockdep-design.rst#n230 I learnt that to use the _nested() form, the hierarchy among the locks should be mapped accurately; however, from the lockdep documentation, my understanding is that there isn't any hierarchy between the locks in this case. Your guidance on whether the kern_path based fix is the right direction here, or if there is a more appropriate way to handle this from a configfs or VFS point of view would be very valuable. Thank you for your time and guidance. Best Regards, Prithvi
On 1/21/26 9:51 AM, Prithvi wrote: > I tried using lockdep_register_key() and lockdep_unregister_key() for the > frag_sem lock, however it stil gives the possible recursive locking > warning. Here is the patch and the bug report from its test: > > https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#m3203ceddf3423b7116ba9225d182771608f93a6f > > Would using down_read_nested() and subclasses be a better option here? > > I also checked out some documentation regarding it and learnt that to use > the _nested() form, the hierarchy among the locks should be mapped > accurately; however, IIUC, there isn't any hierarchy between the locks in > this case, is this right? > > Apologies if I am missing something obvious here, and thanks for your > time and guidance. This is unexpected. Please ask help from someone who is familiar with VFS internals. I'm not familiar with these internals. Thanks, Bart.
On Wed, Jan 21, 2026 at 09:59:49AM -0800, Bart Van Assche wrote: > On 1/21/26 9:51 AM, Prithvi wrote: > > I tried using lockdep_register_key() and lockdep_unregister_key() for the > > frag_sem lock, however it stil gives the possible recursive locking > > warning. Here is the patch and the bug report from its test: > > > > https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#m3203ceddf3423b7116ba9225d182771608f93a6f > > > > Would using down_read_nested() and subclasses be a better option here? > > > > I also checked out some documentation regarding it and learnt that to use > > the _nested() form, the hierarchy among the locks should be mapped > > accurately; however, IIUC, there isn't any hierarchy between the locks in > > this case, is this right? > > > > Apologies if I am missing something obvious here, and thanks for your > > time and guidance. > > This is unexpected. Please ask help from someone who is familiar with VFS > internals. I'm not familiar with these internals. > > Thanks, > > Bart. Sure, no problem...thank you very much for your time and guidance :) Best regards, Prithvi
On Tue, Jan 20, 2026 at 05:48:16AM -0800, Bart Van Assche wrote: > On 1/19/26 10:50 AM, Prithvi wrote: > > Possible unsafe locking scenario: > > > > CPU0 > > ---- > > lock(&p->frag_sem); > > lock(&p->frag_sem); > The least intrusive way to suppress this type of lockdep complaints is > by using lockdep_register_key() and lockdep_unregister_key(). > > Thanks, > > Bart. Sure. I will make v2 patch for the same. Thanks, Prithvi
On Fri, Jan 09, 2026 at 12:45:23AM +0530, Prithvi Tambewagh wrote:
> In flush_write_buffer, &p->frag_sem is acquired and then the loaded store
> function is called, which, here, is target_core_item_dbroot_store().
> This function called filp_open(), following which these functions were
> called (in reverse order), according to the call trace:
>
> down_read
> __configfs_open_file
> do_dentry_open
> vfs_open
> do_open
> path_openat
> do_filp_open
> file_open_name
> filp_open
> target_core_item_dbroot_store
> flush_write_buffer
> configfs_write_iter
>
> Hence ultimately, __configfs_open_file() was called, indirectly by
> target_core_item_dbroot_store(), and it also attempted to acquire
> &p->frag_sem, which was already held by the same thread, acquired earlier
> in flush_write_buffer. This poses a possibility of recursive locking,
> which triggers the lockdep warning.
>
> Fix this by modifying target_core_item_dbroot_store() to use kern_path()
> instead of filp_open() to avoid opening the file using filesystem-specific
> function __configfs_open_file(), and further modifying it to make this
> fix compatible.
>
> Reported-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f6e8174215573a84b797
> Tested-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>
> ---
> drivers/target/target_core_configfs.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index b19acd662726..f29052e6a87d 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -108,8 +108,8 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> const char *page, size_t count)
> {
> ssize_t read_bytes;
> - struct file *fp;
> ssize_t r = -EINVAL;
> + struct path path = {};
>
> mutex_lock(&target_devices_lock);
> if (target_devices) {
> @@ -131,17 +131,18 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> db_root_stage[read_bytes - 1] = '\0';
>
> /* validate new db root before accepting it */
> - fp = filp_open(db_root_stage, O_RDONLY, 0);
> - if (IS_ERR(fp)) {
> + r = kern_path(db_root_stage, LOOKUP_FOLLOW, &path);
> + if (r) {
> pr_err("db_root: cannot open: %s\n", db_root_stage);
> goto unlock;
> }
> - if (!S_ISDIR(file_inode(fp)->i_mode)) {
> - filp_close(fp, NULL);
> + if (!d_is_dir(path.dentry)) {
> + path_put(&path);
> pr_err("db_root: not a directory: %s\n", db_root_stage);
> + r = -ENOTDIR;
> goto unlock;
> }
> - filp_close(fp, NULL);
> + path_put(&path);
>
> strscpy(db_root, db_root_stage);
> pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
>
> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> --
> 2.34.1
>
Hello all,
Just a gentle ping on this thread.
Thanks,
Prithvi
On Thu, Jan 15, 2026 at 08:50:12AM +0530, Prithvi wrote:
>
> On Fri, Jan 09, 2026 at 12:45:23AM +0530, Prithvi Tambewagh wrote:
> > In flush_write_buffer, &p->frag_sem is acquired and then the loaded store
> > function is called, which, here, is target_core_item_dbroot_store().
> > This function called filp_open(), following which these functions were
> > called (in reverse order), according to the call trace:
> >
> > down_read
> > __configfs_open_file
> > do_dentry_open
> > vfs_open
> > do_open
> > path_openat
> > do_filp_open
> > file_open_name
> > filp_open
> > target_core_item_dbroot_store
> > flush_write_buffer
> > configfs_write_iter
> >
> > Hence ultimately, __configfs_open_file() was called, indirectly by
> > target_core_item_dbroot_store(), and it also attempted to acquire
> > &p->frag_sem, which was already held by the same thread, acquired earlier
> > in flush_write_buffer. This poses a possibility of recursive locking,
> > which triggers the lockdep warning.
> >
> > Fix this by modifying target_core_item_dbroot_store() to use kern_path()
> > instead of filp_open() to avoid opening the file using filesystem-specific
> > function __configfs_open_file(), and further modifying it to make this
> > fix compatible.
> >
> > Reported-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=f6e8174215573a84b797
> > Tested-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>
> > ---
> > drivers/target/target_core_configfs.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> > index b19acd662726..f29052e6a87d 100644
> > --- a/drivers/target/target_core_configfs.c
> > +++ b/drivers/target/target_core_configfs.c
> > @@ -108,8 +108,8 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> > const char *page, size_t count)
> > {
> > ssize_t read_bytes;
> > - struct file *fp;
> > ssize_t r = -EINVAL;
> > + struct path path = {};
> >
> > mutex_lock(&target_devices_lock);
> > if (target_devices) {
> > @@ -131,17 +131,18 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> > db_root_stage[read_bytes - 1] = '\0';
> >
> > /* validate new db root before accepting it */
> > - fp = filp_open(db_root_stage, O_RDONLY, 0);
> > - if (IS_ERR(fp)) {
> > + r = kern_path(db_root_stage, LOOKUP_FOLLOW, &path);
> > + if (r) {
> > pr_err("db_root: cannot open: %s\n", db_root_stage);
> > goto unlock;
> > }
> > - if (!S_ISDIR(file_inode(fp)->i_mode)) {
> > - filp_close(fp, NULL);
> > + if (!d_is_dir(path.dentry)) {
> > + path_put(&path);
> > pr_err("db_root: not a directory: %s\n", db_root_stage);
> > + r = -ENOTDIR;
> > goto unlock;
> > }
> > - filp_close(fp, NULL);
> > + path_put(&path);
> >
> > strscpy(db_root, db_root_stage);
> > pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
> >
> > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> > --
> > 2.34.1
> >
You missed the very significant thing in the commit message - that this
lockdep warning is due to try to write its own filename to dbroot file:
db_root: not a directory: /sys/kernel/config/target/dbroot
That is why the semaphore is the same - it is of the same file.
Without that explanation nobody understands wheter it is a false positive or not.
The fix itself looks good.
Reviewed-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
On Thu, Jan 22, 2026 at 12:56:34PM +0300, Dmitry Bogdanov wrote:
> On Thu, Jan 15, 2026 at 08:50:12AM +0530, Prithvi wrote:
> >
> > On Fri, Jan 09, 2026 at 12:45:23AM +0530, Prithvi Tambewagh wrote:
> > > In flush_write_buffer, &p->frag_sem is acquired and then the loaded store
> > > function is called, which, here, is target_core_item_dbroot_store().
> > > This function called filp_open(), following which these functions were
> > > called (in reverse order), according to the call trace:
> > >
> > > down_read
> > > __configfs_open_file
> > > do_dentry_open
> > > vfs_open
> > > do_open
> > > path_openat
> > > do_filp_open
> > > file_open_name
> > > filp_open
> > > target_core_item_dbroot_store
> > > flush_write_buffer
> > > configfs_write_iter
> > >
> > > Hence ultimately, __configfs_open_file() was called, indirectly by
> > > target_core_item_dbroot_store(), and it also attempted to acquire
> > > &p->frag_sem, which was already held by the same thread, acquired earlier
> > > in flush_write_buffer. This poses a possibility of recursive locking,
> > > which triggers the lockdep warning.
> > >
> > > Fix this by modifying target_core_item_dbroot_store() to use kern_path()
> > > instead of filp_open() to avoid opening the file using filesystem-specific
> > > function __configfs_open_file(), and further modifying it to make this
> > > fix compatible.
> > >
> > > Reported-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=f6e8174215573a84b797
> > > Tested-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>
> > > ---
> > > drivers/target/target_core_configfs.c | 13 +++++++------
> > > 1 file changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> > > index b19acd662726..f29052e6a87d 100644
> > > --- a/drivers/target/target_core_configfs.c
> > > +++ b/drivers/target/target_core_configfs.c
> > > @@ -108,8 +108,8 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> > > const char *page, size_t count)
> > > {
> > > ssize_t read_bytes;
> > > - struct file *fp;
> > > ssize_t r = -EINVAL;
> > > + struct path path = {};
> > >
> > > mutex_lock(&target_devices_lock);
> > > if (target_devices) {
> > > @@ -131,17 +131,18 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> > > db_root_stage[read_bytes - 1] = '\0';
> > >
> > > /* validate new db root before accepting it */
> > > - fp = filp_open(db_root_stage, O_RDONLY, 0);
> > > - if (IS_ERR(fp)) {
> > > + r = kern_path(db_root_stage, LOOKUP_FOLLOW, &path);
> > > + if (r) {
> > > pr_err("db_root: cannot open: %s\n", db_root_stage);
> > > goto unlock;
> > > }
> > > - if (!S_ISDIR(file_inode(fp)->i_mode)) {
> > > - filp_close(fp, NULL);
> > > + if (!d_is_dir(path.dentry)) {
> > > + path_put(&path);
> > > pr_err("db_root: not a directory: %s\n", db_root_stage);
> > > + r = -ENOTDIR;
> > > goto unlock;
> > > }
> > > - filp_close(fp, NULL);
> > > + path_put(&path);
> > >
> > > strscpy(db_root, db_root_stage);
> > > pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
> > >
> > > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> > > --
> > > 2.34.1
> > >
>
> You missed the very significant thing in the commit message - that this
> lockdep warning is due to try to write its own filename to dbroot file:
>
> db_root: not a directory: /sys/kernel/config/target/dbroot
>
> That is why the semaphore is the same - it is of the same file.
>
> Without that explanation nobody understands wheter it is a false positive or not.
>
> The fix itself looks good.
>
> Reviewed-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
Hello Dmitry,
I have sent v2 patch with this change incorporated, however it doesn't
include your Reviewed-by tag. Since your review applies, and the changes
in v2 don't invalidate it, I wanted to confirm if its okay to carry
forward your Reviewed-by tag or if you would prefer to review it agian.
Apologies if this is an obvious point.
Best Regards,
Prithvi
On Thu, Jan 22, 2026 at 12:56:34PM +0300, Dmitry Bogdanov wrote:
> On Thu, Jan 15, 2026 at 08:50:12AM +0530, Prithvi wrote:
> >
> > On Fri, Jan 09, 2026 at 12:45:23AM +0530, Prithvi Tambewagh wrote:
> > > In flush_write_buffer, &p->frag_sem is acquired and then the loaded store
> > > function is called, which, here, is target_core_item_dbroot_store().
> > > This function called filp_open(), following which these functions were
> > > called (in reverse order), according to the call trace:
> > >
> > > down_read
> > > __configfs_open_file
> > > do_dentry_open
> > > vfs_open
> > > do_open
> > > path_openat
> > > do_filp_open
> > > file_open_name
> > > filp_open
> > > target_core_item_dbroot_store
> > > flush_write_buffer
> > > configfs_write_iter
> > >
> > > Hence ultimately, __configfs_open_file() was called, indirectly by
> > > target_core_item_dbroot_store(), and it also attempted to acquire
> > > &p->frag_sem, which was already held by the same thread, acquired earlier
> > > in flush_write_buffer. This poses a possibility of recursive locking,
> > > which triggers the lockdep warning.
> > >
> > > Fix this by modifying target_core_item_dbroot_store() to use kern_path()
> > > instead of filp_open() to avoid opening the file using filesystem-specific
> > > function __configfs_open_file(), and further modifying it to make this
> > > fix compatible.
> > >
> > > Reported-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=f6e8174215573a84b797
> > > Tested-by: syzbot+f6e8174215573a84b797@syzkaller.appspotmail.com
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>
> > > ---
> > > drivers/target/target_core_configfs.c | 13 +++++++------
> > > 1 file changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> > > index b19acd662726..f29052e6a87d 100644
> > > --- a/drivers/target/target_core_configfs.c
> > > +++ b/drivers/target/target_core_configfs.c
> > > @@ -108,8 +108,8 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> > > const char *page, size_t count)
> > > {
> > > ssize_t read_bytes;
> > > - struct file *fp;
> > > ssize_t r = -EINVAL;
> > > + struct path path = {};
> > >
> > > mutex_lock(&target_devices_lock);
> > > if (target_devices) {
> > > @@ -131,17 +131,18 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> > > db_root_stage[read_bytes - 1] = '\0';
> > >
> > > /* validate new db root before accepting it */
> > > - fp = filp_open(db_root_stage, O_RDONLY, 0);
> > > - if (IS_ERR(fp)) {
> > > + r = kern_path(db_root_stage, LOOKUP_FOLLOW, &path);
> > > + if (r) {
> > > pr_err("db_root: cannot open: %s\n", db_root_stage);
> > > goto unlock;
> > > }
> > > - if (!S_ISDIR(file_inode(fp)->i_mode)) {
> > > - filp_close(fp, NULL);
> > > + if (!d_is_dir(path.dentry)) {
> > > + path_put(&path);
> > > pr_err("db_root: not a directory: %s\n", db_root_stage);
> > > + r = -ENOTDIR;
> > > goto unlock;
> > > }
> > > - filp_close(fp, NULL);
> > > + path_put(&path);
> > >
> > > strscpy(db_root, db_root_stage);
> > > pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
> > >
> > > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> > > --
> > > 2.34.1
> > >
>
> You missed the very significant thing in the commit message - that this
> lockdep warning is due to try to write its own filename to dbroot file:
>
> db_root: not a directory: /sys/kernel/config/target/dbroot
>
> That is why the semaphore is the same - it is of the same file.
>
> Without that explanation nobody understands wheter it is a false positive or not.
>
> The fix itself looks good.
>
> Reviewed-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
Hello Dmitry,
Thanks a lot for the feedback! I missed out this curcial explanation in the
commit message. I will update the commit message and send a v2 patch for
the same.
Best Regards,
Prithvi
© 2016 - 2026 Red Hat, Inc.