On Fri, 2021-08-13 at 14:03 +0100, Luis Henriques wrote: > Encryption is currently only supported on files/directories with layouts > where stripe_count=1. Forbid changing layouts when encryption is involved. > > Signed-off-by: Luis Henriques <lhenriques@xxxxxxx> > --- > Hi! > > While continuing looking into fscrypt, I realized we're not yet forbidding > different layouts on encrypted files. This patch tries to do just that. > > Regarding the setxattr, I've also made a change [1] to the MDS code so that it > also prevents layouts to be changed. This should make the changes to > ceph_sync_setxattr() redundant, but in practice it doesn't because if we encrypt > a directory and immediately after that we change that directory layout, the MDS > wouldn't yet have received the fscrypt_auth for that inode. So... yeah, an > alternative would be to propagate the fscrypt context immediately after > encrypting a directory. > > [1] https://github.com/luis-henrix/ceph/commit/601488ae798ecfa5ec81677d1ced02f7dd42aa10 > > Cheers, > -- > Luis > > fs/ceph/ioctl.c | 4 ++++ > fs/ceph/xattr.c | 6 ++++++ > 2 files changed, 10 insertions(+) > > diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c > index 477ecc667aee..42abfc564301 100644 > --- a/fs/ceph/ioctl.c > +++ b/fs/ceph/ioctl.c > @@ -294,6 +294,10 @@ static long ceph_set_encryption_policy(struct file *file, unsigned long arg) > struct inode *inode = file_inode(file); > struct ceph_inode_info *ci = ceph_inode(inode); > > + /* encrypted directories can't have striped layout */ > + if (ci->i_layout.stripe_count > 1) > + return -EOPNOTSUPP; > + Yes, I've been needing to add that for a while. I'm not sure EOPNOTSUPP is the right error code though. Maybe EINVAL instead? > ret = vet_mds_for_fscrypt(file); > if (ret) > return ret; > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c > index b175b3029dc0..7921cb34900c 100644 > --- a/fs/ceph/xattr.c > +++ b/fs/ceph/xattr.c > @@ -1051,6 +1051,12 @@ static int ceph_sync_setxattr(struct inode *inode, const char *name, > int op = CEPH_MDS_OP_SETXATTR; > int err; > > + /* encrypted directories/files can't have their layout changed */ > + if (IS_ENCRYPTED(inode) && > + (!strncmp(name, "ceph.file.layout", 16) || > + !strncmp(name, "ceph.dir.layout", 15))) > + return -EOPNOTSUPP; > + Yuck. What might be nicer is to just make ceph_vxattrcb_layout* return an error when the inode is encrypted? You can return negative error codes from the ->getxattr_cb ops, and that's probably the better place to check for this. > if (size > 0) { > /* copy value into pagelist */ > pagelist = ceph_pagelist_alloc(GFP_NOFS); > > Cheers, > -- > Luís -- Jeff Layton <jlayton@xxxxxxxxxx>
