fscrypt: use the mount idmap for the owner check in fscrypt_ioctl_set_policy()

From: Zhan Xusheng <[email protected]>

fscrypt_ioctl_set_policy() calls inode_owner_or_capable() with &nop_mnt_idmap before allowing an encryption policy to be set, instead of the idmap of the mount the ioctl was issued on.

fscrypt is used by filesystems that support idmapped mounts (e.g. ext4, f2fs), so on such a mount this compares the caller's fsuid against the unmapped on-disk owner rather than the mapped owner: the actual owner can be wrongly denied with -EACCES and an unrelated caller wrongly allowed. Use file_mnt_idmap(filp) instead.

Fixes 01beba7957a2 ("fs: port inode_owner_or_capable() to mnt_idmap")

Cc [email protected]

Signed-off-by Zhan Xusheng <[email protected]>

---
 fs/crypto/policy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/crypto/policy.c +1 -1

index 9915e39362db..c80b24a941ad 100644
@@ -534,7 +534,7 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
 		return -EFAULT;
 	policy.version = version;
 
-	if (!inode_owner_or_capable(&nop_mnt_idmap, inode))
+	if (!inode_owner_or_capable(file_mnt_idmap(filp), inode))
 		return -EACCES;
 
 	ret = mnt_want_write_file(filp);

Signature · 1 line

2.43.0

[+Cc [email protected]]

Quoted 34 lines · Zhan Xusheng

From: Zhan Xusheng <[email protected]>

fscrypt_ioctl_set_policy() calls inode_owner_or_capable() with &nop_mnt_idmap before allowing an encryption policy to be set, instead of the idmap of the mount the ioctl was issued on.

fscrypt is used by filesystems that support idmapped mounts (e.g. ext4, f2fs), so on such a mount this compares the caller's fsuid against the unmapped on-disk owner rather than the mapped owner: the actual owner can be wrongly denied with -EACCES and an unrelated caller wrongly allowed. Use file_mnt_idmap(filp) instead.

Fixes 01beba7957a2 ("fs: port inode_owner_or_capable() to mnt_idmap")

Cc [email protected]

Signed-off-by Zhan Xusheng <[email protected]>

---
 fs/crypto/policy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/crypto/policy.c +1 -1

index 9915e39362db..c80b24a941ad 100644
@@ -534,7 +534,7 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
 		return -EFAULT;
 	policy.version = version;
 
-	if (!inode_owner_or_capable(&nop_mnt_idmap, inode))
+	if (!inode_owner_or_capable(file_mnt_idmap(filp), inode))
 		return -EACCES;
 
 	ret = mnt_want_write_file(filp);

Signature · 1 line

2.43.0

Thanks. I don't think the Fixes commit is quite right: 01beba7957a2 seems to have been a behavior-preserving commit. It even says that it was "eliminating the possibility of any bugs" :-)

Really it seems a patch to this function was just missing entirely, but I would probably use 14f3db5542e6 ("ext4: support idmapped mounts"). That's when the other ext4 ioctls got converted to use file_mnt_user_ns() and the idmap support was enabled on ext4, but this was missed. (And ext4 was the first fscrypt-capable filesystem to get idmap support.) As usual, this also needs 'Cc stable'.

I guess I'll plan to take this through the fscrypt tree. I'll update the tags when committing if there's no other feedback.