All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fs/ceph/ioctl: add owner/capability checks for CEPH_IOC_SET_LAYOUT*
@ 2026-07-21  6:20 Max Kellermann
  2026-07-21  6:40 ` Xiubo Li
  0 siblings, 1 reply; 2+ messages in thread
From: Max Kellermann @ 2026-07-21  6:20 UTC (permalink / raw)
  To: idryomov, amarkuze, xiubo.li, ceph-devel, linux-kernel; +Cc: Max Kellermann

These permission checks were already missing in the initial
impementation of these ioctls.  This Ceph allows any user who owns a
file descriptor to manipulate the layout of any file, even if they
don't have write permissions.

It might be a good idea to guard other ioctls with permission checks
as well or even disallow regular users (even if they own the file) to
manipulate layout settings completely, as this may be abused to DoS
the Ceph servers, but right now, I find it most urgent to have setter
checks at all.

Fixes: 8f4e91dee2a2 ("ceph: ioctls")
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
---
Note: this is a resend.  I had already sent this to security@ceph.io
and security@kernel.org on 2024-11-25, but the Ceph maintainers
thought it was "not a big problem".  It was never merged.
---
 fs/ceph/ioctl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
index 15cde055f3da..de07f19b0caa 100644
--- a/fs/ceph/ioctl.c
+++ b/fs/ceph/ioctl.c
@@ -72,6 +72,9 @@ static long ceph_ioctl_set_layout(struct file *file, void __user *arg)
 	struct ceph_ioctl_layout nl;
 	int err;
 
+	if (!inode_owner_or_capable(&nop_mnt_idmap, inode))
+		return -EACCES;
+
 	if (copy_from_user(&l, arg, sizeof(l)))
 		return -EFAULT;
 
@@ -142,6 +145,9 @@ static long ceph_ioctl_set_layout_policy (struct file *file, void __user *arg)
 	int err;
 	struct ceph_mds_client *mdsc = ceph_sb_to_fs_client(inode->i_sb)->mdsc;
 
+	if (!inode_owner_or_capable(&nop_mnt_idmap, inode))
+		return -EACCES;
+
 	/* copy and validate */
 	if (copy_from_user(&l, arg, sizeof(l)))
 		return -EFAULT;
-- 
2.47.3


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

* Re: [PATCH] fs/ceph/ioctl: add owner/capability checks for CEPH_IOC_SET_LAYOUT*
  2026-07-21  6:20 [PATCH] fs/ceph/ioctl: add owner/capability checks for CEPH_IOC_SET_LAYOUT* Max Kellermann
@ 2026-07-21  6:40 ` Xiubo Li
  0 siblings, 0 replies; 2+ messages in thread
From: Xiubo Li @ 2026-07-21  6:40 UTC (permalink / raw)
  To: Max Kellermann; +Cc: idryomov, amarkuze, ceph-devel, linux-kernel

Hi Max,

On Tue, 21 Jul 2026 at 14:20, Max Kellermann <max.kellermann@ionos.com> wrote:
>
> These permission checks were already missing in the initial
> impementation of these ioctls.  This Ceph allows any user who owns a
> file descriptor to manipulate the layout of any file, even if they
> don't have write permissions.
>
> It might be a good idea to guard other ioctls with permission checks
> as well or even disallow regular users (even if they own the file) to
> manipulate layout settings completely, as this may be abused to DoS
> the Ceph servers, but right now, I find it most urgent to have setter
> checks at all.
>

Thanks for bringing this up again. LGTM :-)

Reviewed-by: Xiubo Li <xiubo.li@clyso.com>


> Fixes: 8f4e91dee2a2 ("ceph: ioctls")
> Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
> ---
> Note: this is a resend.  I had already sent this to security@ceph.io
> and security@kernel.org on 2024-11-25, but the Ceph maintainers
> thought it was "not a big problem".  It was never merged.

It should have just been missed.

The MDSAuthCaps::is_capable() in the MDS does validate the client's
cephx caps. However, with the default cephx cap configuration
("allow rw" / "allow rwp"), grant.match.uid defaults to MDS_AUTH_UID_ANY,
and is_capable() returns true immediately — all Unix user-level
permission checks (caller_uid vs inode_uid, mode bits) are skipped
entirely.  The MDS only enforces client-level caps; it delegates
user-level enforcement to the kernel client.

On a multi-user host with cephx 'rwp' credentials, any local user with
a file descriptor can manipulate another user's file layout, redirect data
to arbitrary RADOS pools, set directory quotas, or pin directories to
specific MDS ranks.  The kernel is the only place where per-user isolation
can happen for these operations — defense in depth cannot be outsourced
to the MDS here.

Thanks
Xiubo

> ---
>  fs/ceph/ioctl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
> index 15cde055f3da..de07f19b0caa 100644
> --- a/fs/ceph/ioctl.c
> +++ b/fs/ceph/ioctl.c
> @@ -72,6 +72,9 @@ static long ceph_ioctl_set_layout(struct file *file, void __user *arg)
>         struct ceph_ioctl_layout nl;
>         int err;
>
> +       if (!inode_owner_or_capable(&nop_mnt_idmap, inode))
> +               return -EACCES;
> +
>         if (copy_from_user(&l, arg, sizeof(l)))
>                 return -EFAULT;
>
> @@ -142,6 +145,9 @@ static long ceph_ioctl_set_layout_policy (struct file *file, void __user *arg)
>         int err;
>         struct ceph_mds_client *mdsc = ceph_sb_to_fs_client(inode->i_sb)->mdsc;
>
> +       if (!inode_owner_or_capable(&nop_mnt_idmap, inode))
> +               return -EACCES;
> +
>         /* copy and validate */
>         if (copy_from_user(&l, arg, sizeof(l)))
>                 return -EFAULT;
> --
> 2.47.3
>

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

end of thread, other threads:[~2026-07-21  6:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21  6:20 [PATCH] fs/ceph/ioctl: add owner/capability checks for CEPH_IOC_SET_LAYOUT* Max Kellermann
2026-07-21  6:40 ` Xiubo Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.