| Message ID | 20210120182847.644850-18-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph+fscrypt: context, filename and symlink support | expand |
Jeff Layton <jlayton@kernel.org> writes: > Most of the ioctls, we gate on the 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. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/ceph/ioctl.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c > index 6e061bf62ad4..832909f3eb1b 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,29 @@ 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 = __ceph_lookup_mds_session(mdsc, i); > + > + if (!s) > + continue; > + if (test_bit(CEPHFS_FEATURE_ALTERNATE_NAME, &s->s_features)) > + ret = 0; And another one, I believe...? We need this here: ceph_put_mds_session(s); Also, isn't this logic broken? Shouldn't we walk through all the sessions and return 0 only if they all have that feature bit set? Cheers,
On Thu, 2021-01-28 at 12:22 +0000, Luis Henriques wrote: > Jeff Layton <jlayton@kernel.org> writes: > > > Most of the ioctls, we gate on the 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. > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/ceph/ioctl.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 61 insertions(+) > > > > diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c > > index 6e061bf62ad4..832909f3eb1b 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,29 @@ 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 = __ceph_lookup_mds_session(mdsc, i); > > + > > + if (!s) > > + continue; > > + if (test_bit(CEPHFS_FEATURE_ALTERNATE_NAME, &s->s_features)) > > + ret = 0; > > And another one, I believe...? We need this here: > > ceph_put_mds_session(s); > Well spotted. Though since we hold the mutex over the whole thing, I probably should change this to just not take references at all. I'll fix that. > Also, isn't this logic broken? Shouldn't we walk through all the sessions > and return 0 only if they all have that feature bit set? > Tough call. AFAIU, we're not guaranteed to have a session with all of the available MDS's at any given time. I figured we'd have one and we'd assume that all of the others would be similar. I'm not sure if that's a safe assumption or not though. Let me do some asking around... Thanks!
Jeff Layton <jlayton@kernel.org> writes: > On Thu, 2021-01-28 at 12:22 +0000, Luis Henriques wrote: >> Jeff Layton <jlayton@kernel.org> writes: >> >> > Most of the ioctls, we gate on the 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. >> > >> > Signed-off-by: Jeff Layton <jlayton@kernel.org> >> > --- >> > fs/ceph/ioctl.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 61 insertions(+) >> > >> > diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c >> > index 6e061bf62ad4..832909f3eb1b 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,29 @@ 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 = __ceph_lookup_mds_session(mdsc, i); >> > + >> > + if (!s) >> > + continue; >> > + if (test_bit(CEPHFS_FEATURE_ALTERNATE_NAME, &s->s_features)) >> > + ret = 0; >> >> And another one, I believe...? We need this here: >> >> ceph_put_mds_session(s); >> > > Well spotted. Though since we hold the mutex over the whole thing, I > probably should change this to just not take references at all. I'll fix > that. > >> Also, isn't this logic broken? Shouldn't we walk through all the sessions >> and return 0 only if they all have that feature bit set? >> > > Tough call. > > AFAIU, we're not guaranteed to have a session with all of the available > MDS's at any given time. I figured we'd have one and we'd assume that > all of the others would be similar. > > I'm not sure if that's a safe assumption or not though. Let me do some > asking around... Yeah, you're probably right. All the sessions should have the same features set. Cheers,
diff --git a/fs/ceph/ioctl.c b/fs/ceph/ioctl.c index 6e061bf62ad4..832909f3eb1b 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,29 @@ 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 = __ceph_lookup_mds_session(mdsc, i); + + if (!s) + continue; + if (test_bit(CEPHFS_FEATURE_ALTERNATE_NAME, &s->s_features)) + ret = 0; + break; + } + mutex_unlock(&mdsc->mutex); + 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 +311,45 @@ 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: + ret = vet_mds_for_fscrypt(file); + if (ret) + return ret; + return fscrypt_ioctl_set_policy(file, (const void __user *)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;
Most of the ioctls, we gate on the 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. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/ceph/ioctl.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)