Re: [PATCH v3] scsi: target: file: avoid recursive configfs open in fd_init_prot()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Reopening an existing PI sidecar file using `dentry_open()` bypasses `may_open()` and `inode_permission()` checks.
--

--- Patch [1]: [PATCH v3] scsi: target: file: avoid recursive configfs open in fd_init_prot() ---
commit 12b71238378c9751f15927b9f16e21baaa769408
Author: Runyu Xiao <[email protected]>

scsi: target: file: avoid recursive configfs open in fd_init_prot()

This patch fixes a recursive configfs open hazard by avoiding the reopening
of a user-controlled pathname. Instead, it derives the protection sidecar
file from the opened backing file itself and opens it directly.

Link: https://lore.kernel.org/r/[email protected]

> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
> index 62ced9f5102ff..f6c34e69dbfed 100644
> --- a/drivers/target/target_core_file.c
> +++ b/drivers/target/target_core_file.c
[ ... ]
> @@ -86,6 +90,131 @@ static struct se_device *fd_alloc_device(struct se_hba *hba, const char *name)
[ ... ]
> +static int fd_open_prot_file(struct file *file, int flags, umode_t mode,
> +			     struct file **filep)
> +{
[ ... ]
> +	if (d_is_negative(prot_path.dentry)) {
> +		prot_file = dentry_create(&prot_path, flags, mode, current_cred());
> +	} else {
> +		if (!d_is_reg(prot_path.dentry)) {
> +			ret = -EINVAL;
> +			goto out_dput;
> +		}
> +
> +		inode_unlock(d_inode(parent_path.dentry));
> +		mnt_drop_write_file(file);
> +		release_dentry_name_snapshot(&backing_name);
> +		kfree(prot_name);
> +
> +		prot_file = dentry_open(&prot_path, flags & ~O_CREAT,
> +					current_cred());

[Severity: High]
Does this dentry_open() call bypass standard permission checks?

When reopening an existing sidecar file, this goes straight to dentry_open()
without calling inode_permission() or may_open(). This appears to skip
access controls, including standard permissions, security hooks, and checks
for immutable or append-only file attributes.

If someone with configuration access sets up a backend and links an
immutable file (or one they lack write access to) at the sidecar path,
could they use this to overwrite the file since the permission gates
are bypassed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1