Re: [RFC PATCH v7 12/24] ceph: add fscrypt ioctls

[Xiubo Li <xiubli@redhat.com>]

On 7/8/21 7:26 PM, Jeff Layton wrote:
> On Thu, 2021-07-08 at 15:30 +0800, Xiubo Li wrote:
>> On 6/25/21 9:58 PM, Jeff Layton wrote:
>>> We gate most of the ioctls on MDS feature support. The exception is the
>>> key removal and status functions that we still want to work if the MDS's
>>> were to (inexplicably) lose the feature.
>>>
>>> For the set_policy ioctl, we take Fcx caps to ensure that nothing can
>>> create files in the directory while the ioctl is running. That should
>>> be enough to ensure that the "empty_dir" check is reliable.
>>>
>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>> ---
>>>    fs/ceph/ioctl.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 83 insertions(+)
>>>
>>> diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c
>>> index 6e061bf62ad4..477ecc667aee 100644
>>> --- a/fs/ceph/ioctl.c
>>> +++ b/fs/ceph/ioctl.c
>>> @@ -6,6 +6,7 @@
>>>    #include "mds_client.h"
>>>    #include "ioctl.h"
>>>    #include <linux/ceph/striper.h>
>>> +#include <linux/fscrypt.h>
>>>    
>>>    /*
>>>     * ioctls
>>> @@ -268,8 +269,54 @@ static long ceph_ioctl_syncio(struct file *file)
>>>    	return 0;
>>>    }
>>>    
>>> +static int vet_mds_for_fscrypt(struct file *file)
>>> +{
>>> +	int i, ret = -EOPNOTSUPP;
>>> +	struct ceph_mds_client	*mdsc = ceph_sb_to_mdsc(file_inode(file)->i_sb);
>>> +
>>> +	mutex_lock(&mdsc->mutex);
>>> +	for (i = 0; i < mdsc->max_sessions; i++) {
>>> +		struct ceph_mds_session *s = mdsc->sessions[i];
>>> +
>>> +		if (!s)
>>> +			continue;
>>> +		if (test_bit(CEPHFS_FEATURE_ALTERNATE_NAME, &s->s_features))
>>> +			ret = 0;
>>> +		break;
>>> +	}
>>> +	mutex_unlock(&mdsc->mutex);
>>> +	return ret;
>>> +}
>>> +
>>> +static long ceph_set_encryption_policy(struct file *file, unsigned long arg)
>>> +{
>>> +	int ret, got = 0;
>>> +	struct inode *inode = file_inode(file);
>>> +	struct ceph_inode_info *ci = ceph_inode(inode);
>>> +
>>> +	ret = vet_mds_for_fscrypt(file);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	/*
>>> +	 * Ensure we hold these caps so that we _know_ that the rstats check
>>> +	 * in the empty_dir check is reliable.
>>> +	 */
>>> +	ret = ceph_get_caps(file, CEPH_CAP_FILE_SHARED, 0, -1, &got);
>> In the commit comment said it will host the Fsx, but here it is only
>> trying to hold the Fs. Will the Fx really needed ?
>>
> No. What we're interested in here is that the directory remains empty
> while we're encrypting it. If we hold Fs caps, then no one else can
> modify the directory, so this is enough to ensure that.

Yeah, this is what I thought.

Thanks


>>
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = fscrypt_ioctl_set_policy(file, (const void __user *)arg);
>>> +	if (got)
>>> +		ceph_put_cap_refs(ci, got);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>>    long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>>>    {
>>> +	int ret;
>>> +
>>>    	dout("ioctl file %p cmd %u arg %lu\n", file, cmd, arg);
>>>    	switch (cmd) {
>>>    	case CEPH_IOC_GET_LAYOUT:
>>> @@ -289,6 +336,42 @@ long ceph_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>>>    
>>>    	case CEPH_IOC_SYNCIO:
>>>    		return ceph_ioctl_syncio(file);
>>> +
>>> +	case FS_IOC_SET_ENCRYPTION_POLICY:
>>> +		return ceph_set_encryption_policy(file, arg);
>>> +
>>> +	case FS_IOC_GET_ENCRYPTION_POLICY:
>>> +		ret = vet_mds_for_fscrypt(file);
>>> +		if (ret)
>>> +			return ret;
>>> +		return fscrypt_ioctl_get_policy(file, (void __user *)arg);
>>> +
>>> +	case FS_IOC_GET_ENCRYPTION_POLICY_EX:
>>> +		ret = vet_mds_for_fscrypt(file);
>>> +		if (ret)
>>> +			return ret;
>>> +		return fscrypt_ioctl_get_policy_ex(file, (void __user *)arg);
>>> +
>>> +	case FS_IOC_ADD_ENCRYPTION_KEY:
>>> +		ret = vet_mds_for_fscrypt(file);
>>> +		if (ret)
>>> +			return ret;
>>> +		return fscrypt_ioctl_add_key(file, (void __user *)arg);
>>> +
>>> +	case FS_IOC_REMOVE_ENCRYPTION_KEY:
>>> +		return fscrypt_ioctl_remove_key(file, (void __user *)arg);
>>> +
>>> +	case FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS:
>>> +		return fscrypt_ioctl_remove_key_all_users(file, (void __user *)arg);
>>> +
>>> +	case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
>>> +		return fscrypt_ioctl_get_key_status(file, (void __user *)arg);
>>> +
>>> +	case FS_IOC_GET_ENCRYPTION_NONCE:
>>> +		ret = vet_mds_for_fscrypt(file);
>>> +		if (ret)
>>> +			return ret;
>>> +		return fscrypt_ioctl_get_nonce(file, (void __user *)arg);
>>>    	}
>>>    
>>>    	return -ENOTTY;