2023-03-09 20:39:28

by Seth Forshee

[permalink] [raw]
Subject: [PATCH] filelocks: use mount idmapping for setlease permission check

A user should be allowed to take out a lease via an idmapped mount if
the fsuid matches the mapped uid of the inode. generic_setlease() is
checking the unmapped inode uid, causing these operations to be denied.

Fix this by comparing against the mapped inode uid instead of the
unmapped uid.

Fixes: 9caccd41541a ("fs: introduce MOUNT_ATTR_IDMAP")
Signed-off-by: Seth Forshee (DigitalOcean) <[email protected]>
---
fs/locks.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/locks.c b/fs/locks.c
index 66b4eef09db5..cf07f72eaf45 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1863,9 +1863,10 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp,
void **priv)
{
struct inode *inode = file_inode(filp);
+ vfsuid_t vfsuid = i_uid_into_vfsuid(file_mnt_idmap(filp), inode);
int error;

- if ((!uid_eq(current_fsuid(), inode->i_uid)) && !capable(CAP_LEASE))
+ if ((!vfsuid_eq_kuid(vfsuid, current_fsuid())) && !capable(CAP_LEASE))
return -EACCES;
if (!S_ISREG(inode->i_mode))
return -EINVAL;

---
base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
change-id: 20230309-generic_setlease-use-idmapping-34e099aa2206

Best regards,
--
Seth Forshee (DigitalOcean) <[email protected]>



2023-03-09 21:17:00

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH] filelocks: use mount idmapping for setlease permission check

On Thu, Mar 09, 2023 at 02:39:09PM -0600, Seth Forshee (DigitalOcean) wrote:
> A user should be allowed to take out a lease via an idmapped mount if
> the fsuid matches the mapped uid of the inode. generic_setlease() is
> checking the unmapped inode uid, causing these operations to be denied.
>
> Fix this by comparing against the mapped inode uid instead of the
> unmapped uid.
>
> Fixes: 9caccd41541a ("fs: introduce MOUNT_ATTR_IDMAP")
> Signed-off-by: Seth Forshee (DigitalOcean) <[email protected]>
> ---

Thanks for catching this! This is pretty straightforward so I'll pick
this up in a bit,
Reviewed-by: Christian Brauner <[email protected]>

2023-03-09 21:40:56

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH] filelocks: use mount idmapping for setlease permission check

From: Christian Brauner (Microsoft) <[email protected]>


On Thu, 09 Mar 2023 14:39:09 -0600, Seth Forshee (DigitalOcean) wrote:
> A user should be allowed to take out a lease via an idmapped mount if
> the fsuid matches the mapped uid of the inode. generic_setlease() is
> checking the unmapped inode uid, causing these operations to be denied.
>
> Fix this by comparing against the mapped inode uid instead of the
> unmapped uid.
>
> [...]

I've added a Cc: [email protected] so it's clear we should backport this.
But just to note this here right away, this will likely apply cleanly on 5.15
and 6.2 but fail to compile because our internal apis changed. So I'll have to
do a custom backport for 5.15 and 6.2 once we'll get the failure report thingy
from the stable folks. But applied now,

[1/1] filelocks: use mount idmapping for setlease permission check
commit: 42d0c4bdf753063b6eec55415003184d3ca24f6e