| Message ID | 20230607180958.645115-12-aleksandr.mikhalitsyn@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: support idmapped mounts | expand |
On 6/8/23 02:09, Alexander Mikhalitsyn wrote: > From: Christian Brauner <christian.brauner@ubuntu.com> > > Enable __ceph_setattr() to handle idmapped mounts. This is just a matter > of passing down the mount's idmapping. > > Cc: Xiubo Li <xiubli@redhat.com> > Cc: Jeff Layton <jlayton@kernel.org> > Cc: Ilya Dryomov <idryomov@gmail.com> > Cc: ceph-devel@vger.kernel.org > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > [ adapted to b27c82e12965 ("attr: port attribute changes to new types") ] > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com> > --- > v4: > - introduced fsuid/fsgid local variables > v3: > - reworked as Christian suggested here: > https://lore.kernel.org/lkml/20230602-vorzeichen-praktikum-f17931692301@brauner/ > --- > fs/ceph/inode.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > index bcd9b506ec3b..ca438d1353b2 100644 > --- a/fs/ceph/inode.c > +++ b/fs/ceph/inode.c > @@ -2052,31 +2052,35 @@ int __ceph_setattr(struct mnt_idmap *idmap, struct inode *inode, > dout("setattr %p issued %s\n", inode, ceph_cap_string(issued)); > > if (ia_valid & ATTR_UID) { > + kuid_t fsuid = from_vfsuid(idmap, i_user_ns(inode), attr->ia_vfsuid); > + > dout("setattr %p uid %d -> %d\n", inode, > from_kuid(&init_user_ns, inode->i_uid), > from_kuid(&init_user_ns, attr->ia_uid)); > if (issued & CEPH_CAP_AUTH_EXCL) { > - inode->i_uid = attr->ia_uid; > + inode->i_uid = fsuid; > dirtied |= CEPH_CAP_AUTH_EXCL; > } else if ((issued & CEPH_CAP_AUTH_SHARED) == 0 || > - !uid_eq(attr->ia_uid, inode->i_uid)) { > + !uid_eq(fsuid, inode->i_uid)) { > req->r_args.setattr.uid = cpu_to_le32( > - from_kuid(&init_user_ns, attr->ia_uid)); > + from_kuid(&init_user_ns, fsuid)); > mask |= CEPH_SETATTR_UID; > release |= CEPH_CAP_AUTH_SHARED; > } > } > if (ia_valid & ATTR_GID) { > + kgid_t fsgid = from_vfsgid(idmap, i_user_ns(inode), attr->ia_vfsgid); > + > dout("setattr %p gid %d -> %d\n", inode, > from_kgid(&init_user_ns, inode->i_gid), > from_kgid(&init_user_ns, attr->ia_gid)); > if (issued & CEPH_CAP_AUTH_EXCL) { > - inode->i_gid = attr->ia_gid; > + inode->i_gid = fsgid; > dirtied |= CEPH_CAP_AUTH_EXCL; > } else if ((issued & CEPH_CAP_AUTH_SHARED) == 0 || > - !gid_eq(attr->ia_gid, inode->i_gid)) { > + !gid_eq(fsgid, inode->i_gid)) { > req->r_args.setattr.gid = cpu_to_le32( > - from_kgid(&init_user_ns, attr->ia_gid)); > + from_kgid(&init_user_ns, fsgid)); > mask |= CEPH_SETATTR_GID; > release |= CEPH_CAP_AUTH_SHARED; > } > @@ -2241,7 +2245,7 @@ int ceph_setattr(struct mnt_idmap *idmap, struct dentry *dentry, > if (ceph_inode_is_shutdown(inode)) > return -ESTALE; > > - err = setattr_prepare(&nop_mnt_idmap, dentry, attr); > + err = setattr_prepare(idmap, dentry, attr); > if (err != 0) > return err; > > @@ -2256,7 +2260,7 @@ int ceph_setattr(struct mnt_idmap *idmap, struct dentry *dentry, > err = __ceph_setattr(idmap, inode, attr); > > if (err >= 0 && (attr->ia_valid & ATTR_MODE)) > - err = posix_acl_chmod(&nop_mnt_idmap, dentry, attr->ia_mode); > + err = posix_acl_chmod(idmap, dentry, attr->ia_mode); > > return err; > } You should also do 'req->r_mnt_idmap = idmap;' for sync setattr request. the setattr will just cache the change locally in client side if the 'x' caps are issued and returns directly or will set a sync setattr reqeust. Thanks - Xiubo
On Thu, Jun 8, 2023 at 4:50 AM Xiubo Li <xiubli@redhat.com> wrote: > > > On 6/8/23 02:09, Alexander Mikhalitsyn wrote: > > From: Christian Brauner <christian.brauner@ubuntu.com> > > > > Enable __ceph_setattr() to handle idmapped mounts. This is just a matter > > of passing down the mount's idmapping. > > > > Cc: Xiubo Li <xiubli@redhat.com> > > Cc: Jeff Layton <jlayton@kernel.org> > > Cc: Ilya Dryomov <idryomov@gmail.com> > > Cc: ceph-devel@vger.kernel.org > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > > [ adapted to b27c82e12965 ("attr: port attribute changes to new types") ] > > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com> > > --- > > v4: > > - introduced fsuid/fsgid local variables > > v3: > > - reworked as Christian suggested here: > > https://lore.kernel.org/lkml/20230602-vorzeichen-praktikum-f17931692301@brauner/ > > --- > > fs/ceph/inode.c | 20 ++++++++++++-------- > > 1 file changed, 12 insertions(+), 8 deletions(-) > > > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c > > index bcd9b506ec3b..ca438d1353b2 100644 > > --- a/fs/ceph/inode.c > > +++ b/fs/ceph/inode.c > > @@ -2052,31 +2052,35 @@ int __ceph_setattr(struct mnt_idmap *idmap, struct inode *inode, > > dout("setattr %p issued %s\n", inode, ceph_cap_string(issued)); > > > > if (ia_valid & ATTR_UID) { > > + kuid_t fsuid = from_vfsuid(idmap, i_user_ns(inode), attr->ia_vfsuid); > > + > > dout("setattr %p uid %d -> %d\n", inode, > > from_kuid(&init_user_ns, inode->i_uid), > > from_kuid(&init_user_ns, attr->ia_uid)); > > if (issued & CEPH_CAP_AUTH_EXCL) { > > - inode->i_uid = attr->ia_uid; > > + inode->i_uid = fsuid; > > dirtied |= CEPH_CAP_AUTH_EXCL; > > } else if ((issued & CEPH_CAP_AUTH_SHARED) == 0 || > > - !uid_eq(attr->ia_uid, inode->i_uid)) { > > + !uid_eq(fsuid, inode->i_uid)) { > > req->r_args.setattr.uid = cpu_to_le32( > > - from_kuid(&init_user_ns, attr->ia_uid)); > > + from_kuid(&init_user_ns, fsuid)); > > mask |= CEPH_SETATTR_UID; > > release |= CEPH_CAP_AUTH_SHARED; > > } > > } > > if (ia_valid & ATTR_GID) { > > + kgid_t fsgid = from_vfsgid(idmap, i_user_ns(inode), attr->ia_vfsgid); > > + > > dout("setattr %p gid %d -> %d\n", inode, > > from_kgid(&init_user_ns, inode->i_gid), > > from_kgid(&init_user_ns, attr->ia_gid)); > > if (issued & CEPH_CAP_AUTH_EXCL) { > > - inode->i_gid = attr->ia_gid; > > + inode->i_gid = fsgid; > > dirtied |= CEPH_CAP_AUTH_EXCL; > > } else if ((issued & CEPH_CAP_AUTH_SHARED) == 0 || > > - !gid_eq(attr->ia_gid, inode->i_gid)) { > > + !gid_eq(fsgid, inode->i_gid)) { > > req->r_args.setattr.gid = cpu_to_le32( > > - from_kgid(&init_user_ns, attr->ia_gid)); > > + from_kgid(&init_user_ns, fsgid)); > > mask |= CEPH_SETATTR_GID; > > release |= CEPH_CAP_AUTH_SHARED; > > } > > @@ -2241,7 +2245,7 @@ int ceph_setattr(struct mnt_idmap *idmap, struct dentry *dentry, > > if (ceph_inode_is_shutdown(inode)) > > return -ESTALE; > > > > - err = setattr_prepare(&nop_mnt_idmap, dentry, attr); > > + err = setattr_prepare(idmap, dentry, attr); > > if (err != 0) > > return err; > > > > @@ -2256,7 +2260,7 @@ int ceph_setattr(struct mnt_idmap *idmap, struct dentry *dentry, > > err = __ceph_setattr(idmap, inode, attr); > > > > if (err >= 0 && (attr->ia_valid & ATTR_MODE)) > > - err = posix_acl_chmod(&nop_mnt_idmap, dentry, attr->ia_mode); > > + err = posix_acl_chmod(idmap, dentry, attr->ia_mode); > > > > return err; > > } Hi Xiubo, > > You should also do 'req->r_mnt_idmap = idmap;' for sync setattr request. > > the setattr will just cache the change locally in client side if the 'x' > caps are issued and returns directly or will set a sync setattr reqeust. Have done in v5: ("ceph: pass idmap to __ceph_setattr") https://lore.kernel.org/lkml/20230608154256.562906-8-aleksandr.mikhalitsyn@canonical.com/ Kind regards, Alex > > Thanks > > - Xiubo >
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c index bcd9b506ec3b..ca438d1353b2 100644 --- a/fs/ceph/inode.c +++ b/fs/ceph/inode.c @@ -2052,31 +2052,35 @@ int __ceph_setattr(struct mnt_idmap *idmap, struct inode *inode, dout("setattr %p issued %s\n", inode, ceph_cap_string(issued)); if (ia_valid & ATTR_UID) { + kuid_t fsuid = from_vfsuid(idmap, i_user_ns(inode), attr->ia_vfsuid); + dout("setattr %p uid %d -> %d\n", inode, from_kuid(&init_user_ns, inode->i_uid), from_kuid(&init_user_ns, attr->ia_uid)); if (issued & CEPH_CAP_AUTH_EXCL) { - inode->i_uid = attr->ia_uid; + inode->i_uid = fsuid; dirtied |= CEPH_CAP_AUTH_EXCL; } else if ((issued & CEPH_CAP_AUTH_SHARED) == 0 || - !uid_eq(attr->ia_uid, inode->i_uid)) { + !uid_eq(fsuid, inode->i_uid)) { req->r_args.setattr.uid = cpu_to_le32( - from_kuid(&init_user_ns, attr->ia_uid)); + from_kuid(&init_user_ns, fsuid)); mask |= CEPH_SETATTR_UID; release |= CEPH_CAP_AUTH_SHARED; } } if (ia_valid & ATTR_GID) { + kgid_t fsgid = from_vfsgid(idmap, i_user_ns(inode), attr->ia_vfsgid); + dout("setattr %p gid %d -> %d\n", inode, from_kgid(&init_user_ns, inode->i_gid), from_kgid(&init_user_ns, attr->ia_gid)); if (issued & CEPH_CAP_AUTH_EXCL) { - inode->i_gid = attr->ia_gid; + inode->i_gid = fsgid; dirtied |= CEPH_CAP_AUTH_EXCL; } else if ((issued & CEPH_CAP_AUTH_SHARED) == 0 || - !gid_eq(attr->ia_gid, inode->i_gid)) { + !gid_eq(fsgid, inode->i_gid)) { req->r_args.setattr.gid = cpu_to_le32( - from_kgid(&init_user_ns, attr->ia_gid)); + from_kgid(&init_user_ns, fsgid)); mask |= CEPH_SETATTR_GID; release |= CEPH_CAP_AUTH_SHARED; } @@ -2241,7 +2245,7 @@ int ceph_setattr(struct mnt_idmap *idmap, struct dentry *dentry, if (ceph_inode_is_shutdown(inode)) return -ESTALE; - err = setattr_prepare(&nop_mnt_idmap, dentry, attr); + err = setattr_prepare(idmap, dentry, attr); if (err != 0) return err; @@ -2256,7 +2260,7 @@ int ceph_setattr(struct mnt_idmap *idmap, struct dentry *dentry, err = __ceph_setattr(idmap, inode, attr); if (err >= 0 && (attr->ia_valid & ATTR_MODE)) - err = posix_acl_chmod(&nop_mnt_idmap, dentry, attr->ia_mode); + err = posix_acl_chmod(idmap, dentry, attr->ia_mode); return err; }