Re: [PATCH v2] ceph: send the fscrypt_auth to MDS via request

[Xiubo Li <xiubli@xxxxxxxxxx>]

On 3/15/22 8:41 PM, Jeff Layton wrote:
> On Tue, 2022-03-15 at 17:37 +0800, xiubli@xxxxxxxxxx wrote:
> > From: Xiubo Li <xiubli@xxxxxxxxxx>
> >
> > Currently when creating new files or directories the kclient will
> > create a new inode and fill the fscrypt auth locally, without sending
> > it to MDS via requests. Then the MDS reply with it to empty too.
> > And the kclient will update it later together with the cap update
> > requests.
> >
> > It's buggy if just after the create requests succeeds but the kclient
> > crash and reboot, then in MDS side the fscrypt_auth will keep empty.
> >
> > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
> > ---
> >
> > V2:
> > - Fix the compile errors without CONFIG_FS_ENCRYPTION enabled.
> >
> >
> >
> >   fs/ceph/dir.c  | 43 +++++++++++++++++++++++++++++++++++++++++--
> >   fs/ceph/file.c | 17 ++++++++++++++++-
> >   2 files changed, 57 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > index 5ae5cb778389..8675898a4336 100644
> > --- a/fs/ceph/dir.c
> > +++ b/fs/ceph/dir.c
> > @@ -904,8 +904,22 @@ static int ceph_mknod(struct user_namespace *mnt_userns, struct inode *dir,
> >   		goto out_req;
> >   	}
> >   
> > -	if (S_ISREG(mode) && IS_ENCRYPTED(dir))
> > -		set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
> > +	if (IS_ENCRYPTED(dir)) {
> > +#ifdef CONFIG_FS_ENCRYPTION
> > +		struct ceph_inode_info *ci = ceph_inode(req->r_new_inode);
> > +
> > +		req->r_fscrypt_auth = kmemdup(ci->fscrypt_auth,
> > +					      ci->fscrypt_auth_len,
> > +					      GFP_KERNEL);
> > +		if (!req->r_fscrypt_auth) {
> > +			err = -ENOMEM;
> > +			goto out_req;
> > +		}
> > +#endif
> I thought ceph_as_ctx_to_req was supposed to populate this field. If
> that's not happening here then there is a bug in that codepath, and we
> should just fix that instead of doing a workaround like this.

I may miss this and will check it.

- Xiubo


> > +
> > +		if (S_ISREG(mode))
> > +			set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
> > +	}
> >   
> >   	req->r_dentry = dget(dentry);
> >   	req->r_num_caps = 2;
> > @@ -1008,6 +1022,18 @@ static int ceph_symlink(struct user_namespace *mnt_userns, struct inode *dir,
> >   	ihold(dir);
> >   
> >   	if (IS_ENCRYPTED(req->r_new_inode)) {
> > +#ifdef CONFIG_FS_ENCRYPTION
> > +		struct ceph_inode_info *ci = ceph_inode(req->r_new_inode);
> > +
> > +		req->r_fscrypt_auth = kmemdup(ci->fscrypt_auth,
> > +					      ci->fscrypt_auth_len,
> > +					      GFP_KERNEL);
> > +		if (!req->r_fscrypt_auth) {
> > +			err = -ENOMEM;
> > +			goto out_req;
> > +		}
> > +#endif
> > +
> >   		err = prep_encrypted_symlink_target(req, dest);
> >   		if (err)
> >   			goto out_req;
> > @@ -1081,6 +1107,19 @@ static int ceph_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
> >   		goto out_req;
> >   	}
> >   
> > +#ifdef CONFIG_FS_ENCRYPTION
> > +	if (IS_ENCRYPTED(dir)) {
> > +		struct ceph_inode_info *ci = ceph_inode(req->r_new_inode);
> > +
> > +		req->r_fscrypt_auth = kmemdup(ci->fscrypt_auth,
> > +					      ci->fscrypt_auth_len,
> > +					      GFP_KERNEL);
> > +		if (!req->r_fscrypt_auth) {
> > +			err = -ENOMEM;
> > +			goto out_req;
> > +		}
> > +	}
> > +#endif
> >   	req->r_dentry = dget(dentry);
> >   	req->r_num_caps = 2;
> >   	req->r_parent = dir;
> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > index 61ffbda5b934..70ac41d6e0d4 100644
> > --- a/fs/ceph/file.c
> > +++ b/fs/ceph/file.c
> > @@ -771,9 +771,24 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
> >   	req->r_args.open.mask = cpu_to_le32(mask);
> >   	req->r_parent = dir;
> >   	ihold(dir);
> > -	if (IS_ENCRYPTED(dir))
> > +	if (IS_ENCRYPTED(dir)) {
> >   		set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
> >   
> > +#ifdef CONFIG_FS_ENCRYPTION
> > +		if (new_inode) {
> > +			struct ceph_inode_info *ci = ceph_inode(new_inode);
> > +
> > +			req->r_fscrypt_auth = kmemdup(ci->fscrypt_auth,
> > +						      ci->fscrypt_auth_len,
> > +						      GFP_KERNEL);
> > +			if (!req->r_fscrypt_auth) {
> > +				err = -ENOMEM;
> > +				goto out_req;
> > +			}
> > +		}
> > +#endif
> > +	}
> > +
> >   	if (flags & O_CREAT) {
> >   		struct ceph_file_layout lo;
> >