public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] scsi: target: fix recursive locking in __configfs_open_file()
@ 2026-02-16  6:20 Prithvi Tambewagh
  2026-02-23  3:48 ` Prithvi
  2026-03-01  2:11 ` Martin K. Petersen
  0 siblings, 2 replies; 3+ messages in thread
From: Prithvi Tambewagh @ 2026-02-16  6:20 UTC (permalink / raw)
  To: martin.petersen, d.bogdanov, bvanassche, viro
  Cc: linux-scsi, target-devel, linux-kernel, linux-kernel-mentees,
	skhan, david.hunter.linux, khalid, Prithvi Tambewagh,
	syzbot+f6e8174215573a84b797, stable

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 v3:
 - Add LOOKUP_DIRECTORY flag in call to kern_path() so as to check presence 
   of directory checks more efficiently

v3 link: https://lore.kernel.org/all/20260205162624.117957-1-activprithvi@gmail.com/T/#m175d152067817dd6e9dc1821b6fbf626e47a4007


Note:
I checked out and found that when I try to test on commit 3a8660878839faadb4f1a6dd72c3179c1df56787
(latest commit on which bug dashboard reports the bug on, in upstream repository) 
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
Syzbot Reply: https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#m62bc76de5549460ae98e843bb120712548489794

While when #syz test (i.e. on HEAD commit of upstream) is used, 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
Syzbot reply: https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#me8b79610e4c18a8d8a7d8d6bc249d1c7cf2f8819

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.

Earlier for v1 patch (sine v2 patch involved minor change to commit message 
and v3 involved adding a missed out Reviewed-by tag) patch the kernel builds 
as well as testing succeeded since syzbot used this in its kernel config:

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

as well as used the compiler:

gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40 

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 | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index b19acd662726..f94c242eff97 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,14 @@ 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 | LOOKUP_DIRECTORY, &path);
+	if (r) {
 		pr_err("db_root: cannot open: %s\n", db_root_stage);
+		if (r == -ENOTDIR)
+			pr_err("db_root: not a directory: %s\n", db_root_stage);
 		goto unlock;
 	}
-	if (!S_ISDIR(file_inode(fp)->i_mode)) {
-		filp_close(fp, NULL);
-		pr_err("db_root: not a directory: %s\n", db_root_stage);
-		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


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

* Re: [PATCH v4] scsi: target: fix recursive locking in __configfs_open_file()
  2026-02-16  6:20 [PATCH v4] scsi: target: fix recursive locking in __configfs_open_file() Prithvi Tambewagh
@ 2026-02-23  3:48 ` Prithvi
  2026-03-01  2:11 ` Martin K. Petersen
  1 sibling, 0 replies; 3+ messages in thread
From: Prithvi @ 2026-02-23  3:48 UTC (permalink / raw)
  To: martin.petersen, d.bogdanov, bvanassche, viro
  Cc: linux-scsi, target-devel, linux-kernel, linux-kernel-mentees,
	skhan, david.hunter.linux, khalid, syzbot+f6e8174215573a84b797,
	stable

On Mon, Feb 16, 2026 at 11:50:02AM +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
> 
> 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 v3:
>  - Add LOOKUP_DIRECTORY flag in call to kern_path() so as to check presence 
>    of directory checks more efficiently
> 
> v3 link: https://lore.kernel.org/all/20260205162624.117957-1-activprithvi@gmail.com/T/#m175d152067817dd6e9dc1821b6fbf626e47a4007
> 
> 
> Note:
> I checked out and found that when I try to test on commit 3a8660878839faadb4f1a6dd72c3179c1df56787
> (latest commit on which bug dashboard reports the bug on, in upstream repository) 
> 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
> Syzbot Reply: https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#m62bc76de5549460ae98e843bb120712548489794
> 
> While when #syz test (i.e. on HEAD commit of upstream) is used, 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
> Syzbot reply: https://lore.kernel.org/all/6767d8ea.050a0220.226966.0021.GAE@google.com/T/#me8b79610e4c18a8d8a7d8d6bc249d1c7cf2f8819
> 
> 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.
> 
> Earlier for v1 patch (sine v2 patch involved minor change to commit message 
> and v3 involved adding a missed out Reviewed-by tag) patch the kernel builds 
> as well as testing succeeded since syzbot used this in its kernel config:
> 
> CONFIG_CC_VERSION_TEXT="gcc (Debian 12.2.0-14+deb12u1) 12.2.0"
> 
> as well as used the compiler:
> 
> gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40 
> 
> 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 | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index b19acd662726..f94c242eff97 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,14 @@ 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 | LOOKUP_DIRECTORY, &path);
> +	if (r) {
>  		pr_err("db_root: cannot open: %s\n", db_root_stage);
> +		if (r == -ENOTDIR)
> +			pr_err("db_root: not a directory: %s\n", db_root_stage);
>  		goto unlock;
>  	}
> -	if (!S_ISDIR(file_inode(fp)->i_mode)) {
> -		filp_close(fp, NULL);
> -		pr_err("db_root: not a directory: %s\n", db_root_stage);
> -		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 everyone,

Just a gentle ping on this v4 patch; it incorporates Al Viro's suggestion 
from v3 to use LOOKUP_DIRECTORY flag in kern_path(). Kindly let me know if 
anything else is needed from my side or any feedback is to be addressed.

Thanks,
Prithvi

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

* Re: [PATCH v4] scsi: target: fix recursive locking in __configfs_open_file()
  2026-02-16  6:20 [PATCH v4] scsi: target: fix recursive locking in __configfs_open_file() Prithvi Tambewagh
  2026-02-23  3:48 ` Prithvi
@ 2026-03-01  2:11 ` Martin K. Petersen
  1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2026-03-01  2:11 UTC (permalink / raw)
  To: d.bogdanov, bvanassche, viro, Prithvi Tambewagh
  Cc: Martin K . Petersen, linux-scsi, target-devel, linux-kernel,
	linux-kernel-mentees, skhan, david.hunter.linux, khalid,
	syzbot+f6e8174215573a84b797, stable

On Mon, 16 Feb 2026 11:50:02 +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
> 
> [...]

Applied to 7.0/scsi-fixes, thanks!

[1/1] scsi: target: fix recursive locking in __configfs_open_file()
      https://git.kernel.org/mkp/scsi/c/14d4ac19d189

-- 
Martin K. Petersen

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

end of thread, other threads:[~2026-03-01  2:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-16  6:20 [PATCH v4] scsi: target: fix recursive locking in __configfs_open_file() Prithvi Tambewagh
2026-02-23  3:48 ` Prithvi
2026-03-01  2:11 ` Martin K. Petersen

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