[PATCH v3] scsi: target: fix recursive locking in __configfs_open_file()

[PATCH v3] scsi: target: fix recursive locking in __configfs_open_file()

[Prithvi Tambewagh]

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

target_core_item_dbroot_store() tries to validate the new file path by
trying to open the file path provided to it; however, in this case,
the bug report shows:

db_root: not a directory: /sys/kernel/config/target/dbroot

indicating that the same configfs file was tried to be opened, on which
it is currently working on. Thus, it is trying to acquire frag_sem
semaphore of the same file of which it already holds the semaphore obtained
in flush_write_buffer(), leading to acquiring the semaphore in a nested
manner and a possibility of recursive locking.

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>
Reviewed-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
---
Changes since v2:
 - Add Reviewed-by tag received from Dmitry Bogdanov, which was accidentally
   left to be added in v2 patch.

v2 link: https://lore.kernel.org/linux-scsi/20260122154051.64132-1-activprithvi@gmail.com/T/#u
Reference for Reviewed-by Tag: https://lore.kernel.org/all/20260108191523.303114-1-activprithvi@gmail.com/T/#mb22d0fc06e747e2b2df8320a15afd2a0670fd0e7


Changes since v1:
 - Update commit message to reflect the fact that same file, which code was 
   currently operating on, was tried to be opened again, leading to 
   acquiring the same semaphore in nested manner & possibility of recursive
   locking.

v1 link: https://lore.kernel.org/all/20260108191523.303114-1-activprithvi@gmail.com/T/

 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

Re: [PATCH v3] scsi: target: fix recursive locking in __configfs_open_file()

[Al Viro]

On Thu, Feb 05, 2026 at 09:56:24PM +0530, Prithvi Tambewagh wrote:

> +	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);

Just pass it LOOKUP_FOLLOW | LOOKUP_DIRECTORY and be done with the manual
"is it a directory" tests in any form...

Re: [PATCH v3] scsi: target: fix recursive locking in __configfs_open_file()

[Prithvi]

On Thu, Feb 05, 2026 at 07:26:44PM +0000, Al Viro wrote:
> On Thu, Feb 05, 2026 at 09:56:24PM +0530, Prithvi Tambewagh wrote:
> 
> > +	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);
> 
> Just pass it LOOKUP_FOLLOW | LOOKUP_DIRECTORY and be done with the manual
> "is it a directory" tests in any form...

Hello Al,

I sincerely apologise for the delayed response. I was testing the change you 
suggested, however, whenever I tried testing my patch against the latest 
commit where syzbot reported this bug (commit 3a8660878839faadb4f1a6dd72c3179c1df56787
of upstream repository) it gave me a build failure immediately without any
debug log, just the message:

syzbot tried to test the proposed patch but the build/boot failed:

failed to run ["make" "KERNELVERSION=syzkaller" "KERNELRELEASE=syzkaller" "LOCALVERSION=-syzkaller" "-j" "48" "ARCH=x86_64" "bzImage"]: exit status 2

The issue seems to occur multiple times when a patch is tested against the 
latest commit where syzbot reported the issue while it doesn't occur on that 
latest commit of the upstream repository.

However, testing the change on the latest commit of upstream reprository 
(commit 72c395024dac5e215136cbff793455f065603b06) gives a positive result 
that the reproducer doesn't trigger any issue.

Reference: https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#mbf32aeb54c4cae609d3b6176ad8dcd99bfc51ad2

IIUC, since the reported failure appears to be unrelated to the change and is working
successfully on latest commit of upstream, I wanted to confirm if v4 based on 
these findings is acceptable.

What do you think?

Thank you,
Prithvi 

Re: [PATCH v3] scsi: target: fix recursive locking in __configfs_open_file()

[Prithvi]

On Tue, Feb 10, 2026 at 08:30:14PM +0530, Prithvi wrote:
> On Thu, Feb 05, 2026 at 07:26:44PM +0000, Al Viro wrote:
> > On Thu, Feb 05, 2026 at 09:56:24PM +0530, Prithvi Tambewagh wrote:
> > 
> > > +	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);
> > 
> > Just pass it LOOKUP_FOLLOW | LOOKUP_DIRECTORY and be done with the manual
> > "is it a directory" tests in any form...
> 
> Hello Al,
> 
> I sincerely apologise for the delayed response. I was testing the change you 
> suggested, however, whenever I tried testing my patch against the latest 
> commit where syzbot reported this bug (commit 3a8660878839faadb4f1a6dd72c3179c1df56787
> of upstream repository) it gave me a build failure immediately without any
> debug log, just the message:
> 
> syzbot tried to test the proposed patch but the build/boot failed:
> 
> failed to run ["make" "KERNELVERSION=syzkaller" "KERNELRELEASE=syzkaller" "LOCALVERSION=-syzkaller" "-j" "48" "ARCH=x86_64" "bzImage"]: exit status 2
> 
> The issue seems to occur multiple times when a patch is tested against the 
> latest commit where syzbot reported the issue while it doesn't occur on that 
> latest commit of the upstream repository.
> 
> However, testing the change on the latest commit of upstream reprository 
> (commit 72c395024dac5e215136cbff793455f065603b06) gives a positive result 
> that the reproducer doesn't trigger any issue.
> 
> Reference: https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#mbf32aeb54c4cae609d3b6176ad8dcd99bfc51ad2
> 
> IIUC, since the reported failure appears to be unrelated to the change and is working
> successfully on latest commit of upstream, I wanted to confirm if v4 based on 
> these findings is acceptable.
> 
> What do you think?
> 
> Thank you,
> Prithvi 

Hello Al, 

I checked out and found that when I try to test on commit 3a8660878839faadb4f1a6dd72c3179c1df56787
syzbot uses, in its kernel config:

CONFIG_CC_VERSION_TEXT="gcc (Debian 12.2.0-14+deb12u1) 12.2.0"

Ref: https://syzkaller.appspot.com/x/.config?x=e854293d7f44b5a5

While when I do #syz test (i.e. on HEAD commit of upstream) it uses, in
its kernel config:

CONFIG_CC_VERSION_TEXT="gcc (Debian 14.2.0-19) 14.2.0"

Ref: https://syzkaller.appspot.com/x/.config?x=99ac58566e9eb044

However in both cases it uses:

gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44

Probably due to mismatch in compiler version which syzbot actually uses and 
whats present in kernel config, the build fails for the first case. However, 
the patch succeeds in fixing the bug in second case.

Hence, I think it would be appropriate to send v4 patch, incorporating your 
feedback for v3 patch.

Thanks,
Prithvi