| Message ID | 20200914191707.380444-9-jlayton@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Mon, Sep 14, 2020 at 03:16:59PM -0400, Jeff Layton wrote: > + case Opt_test_dummy_encryption: > + kfree(fsopt->test_dummy_encryption); > +#ifdef CONFIG_FS_ENCRYPTION > + fsopt->test_dummy_encryption = param->string; > + param->string = NULL; > + fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC; > +#else > + warnfc(fc, "FS encryption not supported: test_dummy_encryption mount option ignored"); > +#endif Seems that the kfree() should go in the CONFIG_FS_ENCRYPTION=y block. Also, is there much reason to have the CEPH_MOUNT_OPT_TEST_DUMMY_ENC flag instead of just checking fsopt->test_dummy_encryption != NULL? > +#ifdef CONFIG_FS_ENCRYPTION > +static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc, > + struct ceph_mount_options *fsopt) > +{ > + struct ceph_fs_client *fsc = sb->s_fs_info; > + > + if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) { > + substring_t arg = { }; > + > + /* > + * No changing encryption context on remount. Note that > + * fscrypt_set_test_dummy_encryption will validate the version > + * string passed in (if any). > + */ > + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy) > + return -EEXIST; Maybe show an error message here, with errorfc()? See the message that ext4_set_test_dummy_encryption() shows. > + > + /* Ewwwwwwww */ > + if (fsc->mount_options->test_dummy_encryption) { > + arg.from = fsc->mount_options->test_dummy_encryption; > + arg.to = arg.from + strlen(arg.from) - 1; > + } We should probably make fscrypt_set_test_dummy_encryption() take a 'const char *' to avoid having to create a substring_t here. > + return fscrypt_set_test_dummy_encryption(sb, &arg, &fsc->dummy_enc_policy); Likewise, maybe show an error message if this fails. > + } else { > + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && fsc->dummy_enc_policy.policy) > + return -EEXIST; If remount on ceph behaves as "don't change options that aren't specified", similar to ext4, then there's no need for this hunk here. > static int ceph_reconfigure_fc(struct fs_context *fc) > { > + int err; > struct ceph_parse_opts_ctx *pctx = fc->fs_private; > struct ceph_mount_options *fsopt = pctx->opts; > - struct ceph_fs_client *fsc = ceph_sb_to_client(fc->root->d_sb); > + struct super_block *sb = fc->root->d_sb; > + struct ceph_fs_client *fsc = ceph_sb_to_client(sb); > > if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS) > ceph_set_mount_opt(fsc, ASYNC_DIROPS); > else > ceph_clear_mount_opt(fsc, ASYNC_DIROPS); > > - sync_filesystem(fc->root->d_sb); > + err = ceph_set_test_dummy_encryption(sb, fc, fsopt); > + if (err) > + return err; > + > + sync_filesystem(sb); > return 0; > } Seems that ceph_set_test_dummy_encryption() should go at the beginning, since otherwise it can fail after something was already changed. - Eric
On Mon, 2020-09-14 at 18:23 -0700, Eric Biggers wrote: > On Mon, Sep 14, 2020 at 03:16:59PM -0400, Jeff Layton wrote: > > + case Opt_test_dummy_encryption: > > + kfree(fsopt->test_dummy_encryption); > > +#ifdef CONFIG_FS_ENCRYPTION > > + fsopt->test_dummy_encryption = param->string; > > + param->string = NULL; > > + fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC; > > +#else > > + warnfc(fc, "FS encryption not supported: test_dummy_encryption mount option ignored"); > > +#endif > > Seems that the kfree() should go in the CONFIG_FS_ENCRYPTION=y block. > > Also, is there much reason to have the CEPH_MOUNT_OPT_TEST_DUMMY_ENC flag > instead of just checking fsopt->test_dummy_encryption != NULL? > Yes. That distinguishes between the case where someone has used -o test_dummy_encryption and -o test_dummy_encryption=v2. > > +#ifdef CONFIG_FS_ENCRYPTION > > +static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc, > > + struct ceph_mount_options *fsopt) > > +{ > > + struct ceph_fs_client *fsc = sb->s_fs_info; > > + > > + if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) { > > + substring_t arg = { }; > > + > > + /* > > + * No changing encryption context on remount. Note that > > + * fscrypt_set_test_dummy_encryption will validate the version > > + * string passed in (if any). > > + */ > > + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy) > > + return -EEXIST; > > Maybe show an error message here, with errorfc()? > See the message that ext4_set_test_dummy_encryption() shows. > Good idea. I've rolled in error messages similar to the ones in ext4. > > + > > + /* Ewwwwwwww */ > > + if (fsc->mount_options->test_dummy_encryption) { > > + arg.from = fsc->mount_options->test_dummy_encryption; > > + arg.to = arg.from + strlen(arg.from) - 1; > > + } > > We should probably make fscrypt_set_test_dummy_encryption() take a > 'const char *' to avoid having to create a substring_t here. > Yes, please. I didn't want to do that with most of the current fscrypt- enabled fs using the old mount API. Some of them may need to copy the argument so that it's properly terminated. We could also just add a wrapper that turns the const char * into a substring_t before calling fscrypt_set_test_dummy_encryption. > > + return fscrypt_set_test_dummy_encryption(sb, &arg, &fsc->dummy_enc_policy); > > Likewise, maybe show an error message if this fails. > > > + } else { > > + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && fsc->dummy_enc_policy.policy) > > + return -EEXIST; > > If remount on ceph behaves as "don't change options that aren't specified", > similar to ext4, then there's no need for this hunk here. > Good point. > > static int ceph_reconfigure_fc(struct fs_context *fc) > > { > > + int err; > > struct ceph_parse_opts_ctx *pctx = fc->fs_private; > > struct ceph_mount_options *fsopt = pctx->opts; > > - struct ceph_fs_client *fsc = ceph_sb_to_client(fc->root->d_sb); > > + struct super_block *sb = fc->root->d_sb; > > + struct ceph_fs_client *fsc = ceph_sb_to_client(sb); > > > > if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS) > > ceph_set_mount_opt(fsc, ASYNC_DIROPS); > > else > > ceph_clear_mount_opt(fsc, ASYNC_DIROPS); > > > > - sync_filesystem(fc->root->d_sb); > > + err = ceph_set_test_dummy_encryption(sb, fc, fsopt); > > + if (err) > > + return err; > > + > > + sync_filesystem(sb); > > return 0; > > } > > Seems that ceph_set_test_dummy_encryption() should go at the beginning, since > otherwise it can fail after something was already changed. > Good catch. Fixed. -- Jeff Layton <jlayton@kernel.org>
diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c index 74f07d44dbe9..879d9a0d3751 100644 --- a/fs/ceph/crypto.c +++ b/fs/ceph/crypto.c @@ -29,16 +29,15 @@ static bool ceph_crypt_empty_dir(struct inode *inode) return ci->i_rsubdirs + ci->i_rfiles == 1; } -static const union fscrypt_context * -ceph_get_dummy_context(struct super_block *sb) +static const union fscrypt_policy *ceph_get_dummy_policy(struct super_block *sb) { - return ceph_sb_to_client(sb)->dummy_enc_ctx.ctx; + return ceph_sb_to_client(sb)->dummy_enc_policy.policy; } static struct fscrypt_operations ceph_fscrypt_ops = { .get_context = ceph_crypt_get_context, .set_context = ceph_crypt_set_context, - .get_dummy_context = ceph_get_dummy_context, + .get_dummy_policy = ceph_get_dummy_policy, .empty_dir = ceph_crypt_empty_dir, .max_namelen = NAME_MAX, }; diff --git a/fs/ceph/super.c b/fs/ceph/super.c index 055180218224..eefdea360c50 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -45,6 +45,7 @@ static void ceph_put_super(struct super_block *s) struct ceph_fs_client *fsc = ceph_sb_to_client(s); dout("put_super\n"); + fscrypt_free_dummy_policy(&fsc->dummy_enc_policy); ceph_mdsc_close_sessions(fsc->mdsc); } @@ -160,6 +161,7 @@ enum { Opt_quotadf, Opt_copyfrom, Opt_wsync, + Opt_test_dummy_encryption, }; enum ceph_recover_session_mode { @@ -198,6 +200,8 @@ static const struct fs_parameter_spec ceph_mount_parameters[] = { fsparam_u32 ("rsize", Opt_rsize), fsparam_string ("snapdirname", Opt_snapdirname), fsparam_string ("source", Opt_source), + fsparam_flag ("test_dummy_encryption", Opt_test_dummy_encryption), + fsparam_string ("test_dummy_encryption", Opt_test_dummy_encryption), fsparam_u32 ("wsize", Opt_wsize), fsparam_flag_no ("wsync", Opt_wsync), {} @@ -456,6 +460,16 @@ static int ceph_parse_mount_param(struct fs_context *fc, else fsopt->flags |= CEPH_MOUNT_OPT_ASYNC_DIROPS; break; + case Opt_test_dummy_encryption: + kfree(fsopt->test_dummy_encryption); +#ifdef CONFIG_FS_ENCRYPTION + fsopt->test_dummy_encryption = param->string; + param->string = NULL; + fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC; +#else + warnfc(fc, "FS encryption not supported: test_dummy_encryption mount option ignored"); +#endif + break; default: BUG(); } @@ -475,6 +489,7 @@ static void destroy_mount_options(struct ceph_mount_options *args) kfree(args->mds_namespace); kfree(args->server_path); kfree(args->fscache_uniq); + kfree(args->test_dummy_encryption); kfree(args); } @@ -582,6 +597,8 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root) if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS) seq_puts(m, ",nowsync"); + fscrypt_show_test_dummy_encryption(m, ',', root->d_sb); + if (fsopt->wsize != CEPH_MAX_WRITE_SIZE) seq_printf(m, ",wsize=%u", fsopt->wsize); if (fsopt->rsize != CEPH_MAX_READ_SIZE) @@ -964,6 +981,43 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc, return ERR_PTR(err); } +#ifdef CONFIG_FS_ENCRYPTION +static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc, + struct ceph_mount_options *fsopt) +{ + struct ceph_fs_client *fsc = sb->s_fs_info; + + if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) { + substring_t arg = { }; + + /* + * No changing encryption context on remount. Note that + * fscrypt_set_test_dummy_encryption will validate the version + * string passed in (if any). + */ + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy) + return -EEXIST; + + /* Ewwwwwwww */ + if (fsc->mount_options->test_dummy_encryption) { + arg.from = fsc->mount_options->test_dummy_encryption; + arg.to = arg.from + strlen(arg.from) - 1; + } + return fscrypt_set_test_dummy_encryption(sb, &arg, &fsc->dummy_enc_policy); + } else { + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && fsc->dummy_enc_policy.policy) + return -EEXIST; + } + return 0; +} +#else +static inline int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc, + struct ceph_mount_options *fsopt) +{ + return 0; +} +#endif + static int ceph_set_super(struct super_block *s, struct fs_context *fc) { struct ceph_fs_client *fsc = s->s_fs_info; @@ -985,12 +1039,12 @@ static int ceph_set_super(struct super_block *s, struct fs_context *fc) s->s_time_min = 0; s->s_time_max = U32_MAX; - ret = ceph_fscrypt_set_ops(s); - if (ret) - goto out; + ceph_fscrypt_set_ops(s); - ret = set_anon_super_fc(s, fc); - if (ret != 0) + ret = ceph_set_test_dummy_encryption(s, fc, fsc->mount_options); + if (!ret) + ret = set_anon_super_fc(s, fc); + if (ret) fsc->sb = NULL; return ret; } @@ -1136,16 +1190,22 @@ static void ceph_free_fc(struct fs_context *fc) static int ceph_reconfigure_fc(struct fs_context *fc) { + int err; struct ceph_parse_opts_ctx *pctx = fc->fs_private; struct ceph_mount_options *fsopt = pctx->opts; - struct ceph_fs_client *fsc = ceph_sb_to_client(fc->root->d_sb); + struct super_block *sb = fc->root->d_sb; + struct ceph_fs_client *fsc = ceph_sb_to_client(sb); if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS) ceph_set_mount_opt(fsc, ASYNC_DIROPS); else ceph_clear_mount_opt(fsc, ASYNC_DIROPS); - sync_filesystem(fc->root->d_sb); + err = ceph_set_test_dummy_encryption(sb, fc, fsopt); + if (err) + return err; + + sync_filesystem(sb); return 0; } diff --git a/fs/ceph/super.h b/fs/ceph/super.h index cc39cc36de77..11032b30a14f 100644 --- a/fs/ceph/super.h +++ b/fs/ceph/super.h @@ -17,6 +17,7 @@ #include <linux/posix_acl.h> #include <linux/refcount.h> #include <linux/security.h> +#include <linux/fscrypt.h> #include <linux/ceph/libceph.h> @@ -44,6 +45,7 @@ #define CEPH_MOUNT_OPT_NOQUOTADF (1<<13) /* no root dir quota in statfs */ #define CEPH_MOUNT_OPT_NOCOPYFROM (1<<14) /* don't use RADOS 'copy-from' op */ #define CEPH_MOUNT_OPT_ASYNC_DIROPS (1<<15) /* allow async directory ops */ +#define CEPH_MOUNT_OPT_TEST_DUMMY_ENC (1<<16) /* enable dummy encryption (for testing) */ #define CEPH_MOUNT_OPT_DEFAULT \ (CEPH_MOUNT_OPT_DCACHE | \ @@ -96,6 +98,7 @@ struct ceph_mount_options { char *mds_namespace; /* default NULL */ char *server_path; /* default NULL (means "/") */ char *fscache_uniq; /* default NULL */ + char *test_dummy_encryption; /* default NULL */ }; struct ceph_fs_client { @@ -135,9 +138,11 @@ struct ceph_fs_client { #ifdef CONFIG_CEPH_FSCACHE struct fscache_cookie *fscache; #endif +#ifdef CONFIG_FS_ENCRYPTION + struct fscrypt_dummy_policy dummy_enc_policy; +#endif }; - /* * File i/o capability. This tracks shared state with the metadata * server that allows us to cache or writeback attributes or to read
Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/ceph/crypto.c | 7 ++--- fs/ceph/super.c | 74 +++++++++++++++++++++++++++++++++++++++++++----- fs/ceph/super.h | 7 ++++- 3 files changed, 76 insertions(+), 12 deletions(-)