Re: [PATCH v2 -next 04/18] fs: pass struct path to POSIX ACL helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] fs/ecryptfs: ACL corruption via unintended idmap translation in ecryptfs_set_acl()
--

commit 3198ac18e49aeee9567bdcfb9d89fa401024d0e1
Author: Cai Xinchen <caixinchen1@huawei.com>

fs: pass struct path to POSIX ACL helpers

This commit updates the POSIX ACL helpers (vfs_set_acl, vfs_get_acl, and
vfs_remove_acl) along with the syscall helpers (do_set_acl, do_get_acl) to
take a struct path instead of a struct mnt_idmap and struct dentry. The
intent is to simplify callers by deriving the idmap and dentry directly
from the path where needed.

> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 8b2542c761ca9..b08eac01d32fd 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
[ ... ]
> @@ -1102,11 +1103,10 @@ static int ecryptfs_set_acl(const struct mnt_idmap *idmap,
>  			    int type)
>  {
>  	int rc;
> -	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
> -	struct inode *lower_inode = d_inode(lower_dentry);
> +	struct path lower_path = ecryptfs_lower_path(dentry);
> +	struct inode *lower_inode = d_inode(lower_path.dentry);
>  
> -	rc = vfs_set_acl(&nop_mnt_idmap, lower_dentry,
> -			 posix_acl_xattr_name(type), acl);
> +	rc = vfs_set_acl(&lower_path, posix_acl_xattr_name(type), acl);

[Severity: High]
Does passing lower_path to vfs_set_acl() here cause an unintended idmap
translation to already-global UIDs?

When a user sets a POSIX ACL on an ecryptfs mount sitting on top of an
idmapped mount, ecryptfs_set_acl() receives a posix_acl containing true
global kuid_t UIDs. These UIDs have already been translated from VFS-level
IDs by the outer VFS layer.

Previously, passing nop_mnt_idmap bypassed idmap translation and passed these
global IDs directly to the lower filesystem. By passing lower_path, the lower
mount's idmap is exposed to vfs_set_acl().

Could this cause vfs_set_acl() to erroneously treat the already-global
kuid_t UIDs as mount-specific VFS-level IDs and invoke
vfs_set_acl_idmapped_mnt(), mapping them a second time and corrupting the
ACL ownership IDs written to the backing filesystem?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015125425.3246251-1-caixinchen1@huawei.com?part=4