diff mbox series

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

Message ID 20210625135834.12934-13-jlayton@kernel.org (mailing list archive)
State Not Applicable
Headers show
Series ceph+fscrypt: context, filename and symlink support | expand

Commit Message

Jeff Layton June 25, 2021, 1:58 p.m. UTC 
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(+)

Comments

Xiubo Li July 8, 2021, 7:30 a.m. UTC | #1 
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 ?

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;
Jeff Layton July 8, 2021, 11:26 a.m. UTC | #2 
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.

> 
> 
> > +	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;
>
Xiubo Li July 8, 2021, 11:32 a.m. UTC | #3 
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;
diff mbox series

Patch

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);
+	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;