2022-07-13 09:13:08

by Xiubo Li

[permalink] [raw]
Subject: [PATCH] ceph: fix up test_dummy_encryption handling for new mount API

From: Xiubo Li <[email protected]>

From Eric the "fscrypt_set_test_dummy_encryption()" will be removed
in the next circle. Switch it to new APIs.

Cc: Eric Biggers <[email protected]>
Cc: Jeff Layton <[email protected]>
Signed-off-by: Xiubo Li <[email protected]>
---
fs/ceph/crypto.c | 4 +--
fs/ceph/super.c | 94 ++++++++++++++++++++++++------------------------
fs/ceph/super.h | 5 ++-
3 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 7e0c48e12554..5b807f8f4c69 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -127,7 +127,7 @@ static bool ceph_crypt_empty_dir(struct inode *inode)

static const union fscrypt_policy *ceph_get_dummy_policy(struct super_block *sb)
{
- return ceph_sb_to_client(sb)->dummy_enc_policy.policy;
+ return ceph_sb_to_client(sb)->fsc_dummy_enc_policy.policy;
}

static struct fscrypt_operations ceph_fscrypt_ops = {
@@ -144,7 +144,7 @@ void ceph_fscrypt_set_ops(struct super_block *sb)

void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc)
{
- fscrypt_free_dummy_policy(&fsc->dummy_enc_policy);
+ fscrypt_free_dummy_policy(&fsc->fsc_dummy_enc_policy);
}

int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index fa59a804b32c..4ac4a90755a2 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -591,10 +591,16 @@ static int ceph_parse_mount_param(struct fs_context *fc,
break;
case Opt_test_dummy_encryption:
#ifdef CONFIG_FS_ENCRYPTION
- kfree(fsopt->test_dummy_encryption);
- fsopt->test_dummy_encryption = param->string;
- param->string = NULL;
- fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC;
+ fscrypt_free_dummy_policy(&fsopt->dummy_enc_policy);
+ ret = fscrypt_parse_test_dummy_encryption(param,
+ &fsopt->dummy_enc_policy);
+ if (ret == -EINVAL) {
+ warnfc(fc, "Value of option \"%s\" is unrecognized",
+ param->key);
+ } else if (ret == -EEXIST) {
+ warnfc(fc, "Conflicting test_dummy_encryption options");
+ ret = -EINVAL;
+ }
#else
warnfc(fc,
"FS encryption not supported: test_dummy_encryption mount option ignored");
@@ -620,7 +626,7 @@ static void destroy_mount_options(struct ceph_mount_options *args)
kfree(args->server_path);
kfree(args->fscache_uniq);
kfree(args->mon_addr);
- kfree(args->test_dummy_encryption);
+ fscrypt_free_dummy_policy(&args->dummy_enc_policy);
kfree(args);
}

@@ -1080,51 +1086,47 @@ static struct dentry *open_root_dentry(struct ceph_fs_client *fsc,
return root;
}

-#ifdef CONFIG_FS_ENCRYPTION
-static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
- struct ceph_mount_options *fsopt)
+static int ceph_apply_test_dummy_encryption(struct super_block *sb,
+ struct fs_context *fc,
+ struct ceph_mount_options *fsopt)
{
- /*
- * No changing encryption context on remount. Note that
- * fscrypt_set_test_dummy_encryption will validate the version
- * string passed in (if any).
- */
- if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) {
- struct ceph_fs_client *fsc = sb->s_fs_info;
- int err = 0;
+ struct ceph_fs_client *fsc = sb->s_fs_info;
+ int err;

- if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy) {
- errorfc(fc, "Can't set test_dummy_encryption on remount");
- return -EEXIST;
- }
+ if (!fscrypt_is_dummy_policy_set(&fsopt->dummy_enc_policy))
+ return 0;

- err = fscrypt_set_test_dummy_encryption(sb,
- fsc->mount_options->test_dummy_encryption,
- &fsc->dummy_enc_policy);
- if (err) {
- if (err == -EEXIST)
- errorfc(fc, "Can't change test_dummy_encryption on remount");
- else if (err == -EINVAL)
- errorfc(fc, "Value of option \"%s\" is unrecognized",
- fsc->mount_options->test_dummy_encryption);
- else
- errorfc(fc, "Error processing option \"%s\" [%d]",
- fsc->mount_options->test_dummy_encryption, err);
- return err;
- }
- warnfc(fc, "test_dummy_encryption mode enabled");
+ /* No changing encryption context on remount. */
+ if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
+ !fscrypt_is_dummy_policy_set(&fsc->fsc_dummy_enc_policy)) {
+ if (fscrypt_dummy_policies_equal(&fsopt->dummy_enc_policy,
+ &fsc->fsc_dummy_enc_policy))
+ return 0;
+ errorfc(fc, "Can't set test_dummy_encryption on remount");
+ return -EINVAL;
}
+
+ /* Also make sure fsopt doesn't contain a conflicting value. */
+ if (fscrypt_is_dummy_policy_set(&fsc->fsc_dummy_enc_policy)) {
+ if (fscrypt_dummy_policies_equal(&fsopt->dummy_enc_policy,
+ &fsc->fsc_dummy_enc_policy))
+ return 0;
+ errorfc(fc, "Conflicting test_dummy_encryption options");
+ return -EINVAL;
+ }
+
+ fsc->fsc_dummy_enc_policy = fsopt->dummy_enc_policy;
+ memset(&fsopt->dummy_enc_policy, 0, sizeof(fsopt->dummy_enc_policy));
+
+ err = fscrypt_add_test_dummy_key(sb, &fsc->fsc_dummy_enc_policy);
+ if (err) {
+ errorfc(fc, "Error adding test dummy encryption key, %d", err);
+ return err;
+ }
+
+ warnfc(fc, "test_dummy_encryption mode enabled");
return 0;
}
-#else
-static inline int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
- struct ceph_mount_options *fsopt)
-{
- if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC)
- warnfc(fc, "test_dummy_encryption mode ignored");
- return 0;
-}
-#endif

/*
* mount: join the ceph cluster, and open root directory.
@@ -1154,7 +1156,7 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc,
goto out;
}

- err = ceph_set_test_dummy_encryption(fsc->sb, fc, fsc->mount_options);
+ err = ceph_apply_test_dummy_encryption(fsc->sb, fc, fsc->mount_options);
if (err)
goto out;

@@ -1373,7 +1375,7 @@ static int ceph_reconfigure_fc(struct fs_context *fc)
struct super_block *sb = fc->root->d_sb;
struct ceph_fs_client *fsc = ceph_sb_to_client(sb);

- err = ceph_set_test_dummy_encryption(sb, fc, fsopt);
+ err = ceph_apply_test_dummy_encryption(sb, fc, fsopt);
if (err)
return err;

diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index bfc8bfcea799..5ea0ac6450dd 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -44,7 +44,6 @@
#define CEPH_MOUNT_OPT_ASYNC_DIROPS (1<<15) /* allow async directory ops */
#define CEPH_MOUNT_OPT_NOPAGECACHE (1<<16) /* bypass pagecache altogether */
#define CEPH_MOUNT_OPT_SPARSEREAD (1<<17) /* always do sparse reads */
-#define CEPH_MOUNT_OPT_TEST_DUMMY_ENC (1<<18) /* enable dummy encryption (for testing) */

#define CEPH_MOUNT_OPT_DEFAULT \
(CEPH_MOUNT_OPT_DCACHE | \
@@ -101,7 +100,7 @@ struct ceph_mount_options {
char *server_path; /* default NULL (means "/") */
char *fscache_uniq; /* default NULL */
char *mon_addr;
- char *test_dummy_encryption; /* default NULL */
+ struct fscrypt_dummy_policy dummy_enc_policy;
};

#define CEPH_ASYNC_CREATE_CONFLICT_BITS 8
@@ -148,7 +147,7 @@ struct ceph_fs_client {
struct fscache_volume *fscache;
#endif
#ifdef CONFIG_FS_ENCRYPTION
- struct fscrypt_dummy_policy dummy_enc_policy;
+ struct fscrypt_dummy_policy fsc_dummy_enc_policy;
#endif
};

--
2.36.0.rc1


2022-07-13 11:06:37

by Xiubo Li

[permalink] [raw]
Subject: Re: [PATCH] ceph: fix up test_dummy_encryption handling for new mount API


On 7/13/22 6:42 PM, Jeff Layton wrote:
> On Wed, 2022-07-13 at 16:56 +0800, [email protected] wrote:
>> From: Xiubo Li <[email protected]>
>>
>> From Eric the "fscrypt_set_test_dummy_encryption()" will be removed
>> in the next circle. Switch it to new APIs.
>>
>> Cc: Eric Biggers <[email protected]>
>> Cc: Jeff Layton <[email protected]>
>> Signed-off-by: Xiubo Li <[email protected]>
>> ---
>> fs/ceph/crypto.c | 4 +--
>> fs/ceph/super.c | 94 ++++++++++++++++++++++++------------------------
>> fs/ceph/super.h | 5 ++-
>> 3 files changed, 52 insertions(+), 51 deletions(-)
>>
>> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
>> index 7e0c48e12554..5b807f8f4c69 100644
>> --- a/fs/ceph/crypto.c
>> +++ b/fs/ceph/crypto.c
>> @@ -127,7 +127,7 @@ static bool ceph_crypt_empty_dir(struct inode *inode)
>>
>> static const union fscrypt_policy *ceph_get_dummy_policy(struct super_block *sb)
>> {
>> - return ceph_sb_to_client(sb)->dummy_enc_policy.policy;
>> + return ceph_sb_to_client(sb)->fsc_dummy_enc_policy.policy;
>> }
>>
>> static struct fscrypt_operations ceph_fscrypt_ops = {
>> @@ -144,7 +144,7 @@ void ceph_fscrypt_set_ops(struct super_block *sb)
>>
>> void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc)
>> {
>> - fscrypt_free_dummy_policy(&fsc->dummy_enc_policy);
>> + fscrypt_free_dummy_policy(&fsc->fsc_dummy_enc_policy);
>> }
>>
>> int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
>> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
>> index fa59a804b32c..4ac4a90755a2 100644
>> --- a/fs/ceph/super.c
>> +++ b/fs/ceph/super.c
>> @@ -591,10 +591,16 @@ static int ceph_parse_mount_param(struct fs_context *fc,
>> break;
>> case Opt_test_dummy_encryption:
>> #ifdef CONFIG_FS_ENCRYPTION
>> - kfree(fsopt->test_dummy_encryption);
>> - fsopt->test_dummy_encryption = param->string;
>> - param->string = NULL;
>> - fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC;
>> + fscrypt_free_dummy_policy(&fsopt->dummy_enc_policy);
>> + ret = fscrypt_parse_test_dummy_encryption(param,
>> + &fsopt->dummy_enc_policy);
>> + if (ret == -EINVAL) {
>> + warnfc(fc, "Value of option \"%s\" is unrecognized",
>> + param->key);
>> + } else if (ret == -EEXIST) {
>> + warnfc(fc, "Conflicting test_dummy_encryption options");
>> + ret = -EINVAL;
>> + }
>> #else
>> warnfc(fc,
>> "FS encryption not supported: test_dummy_encryption mount option ignored");
>> @@ -620,7 +626,7 @@ static void destroy_mount_options(struct ceph_mount_options *args)
>> kfree(args->server_path);
>> kfree(args->fscache_uniq);
>> kfree(args->mon_addr);
>> - kfree(args->test_dummy_encryption);
>> + fscrypt_free_dummy_policy(&args->dummy_enc_policy);
>> kfree(args);
>> }
>>
>> @@ -1080,51 +1086,47 @@ static struct dentry *open_root_dentry(struct ceph_fs_client *fsc,
>> return root;
>> }
>>
>> -#ifdef CONFIG_FS_ENCRYPTION
>> -static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
>> - struct ceph_mount_options *fsopt)
>> +static int ceph_apply_test_dummy_encryption(struct super_block *sb,
>> + struct fs_context *fc,
>> + struct ceph_mount_options *fsopt)
>> {
>> - /*
>> - * No changing encryption context on remount. Note that
>> - * fscrypt_set_test_dummy_encryption will validate the version
>> - * string passed in (if any).
>> - */
>> - if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) {
>> - struct ceph_fs_client *fsc = sb->s_fs_info;
>> - int err = 0;
>> + struct ceph_fs_client *fsc = sb->s_fs_info;
>> + int err;
>>
>> - if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy) {
>> - errorfc(fc, "Can't set test_dummy_encryption on remount");
>> - return -EEXIST;
>> - }
>> + if (!fscrypt_is_dummy_policy_set(&fsopt->dummy_enc_policy))
>> + return 0;
>>
>> - err = fscrypt_set_test_dummy_encryption(sb,
>> - fsc->mount_options->test_dummy_encryption,
>> - &fsc->dummy_enc_policy);
>> - if (err) {
>> - if (err == -EEXIST)
>> - errorfc(fc, "Can't change test_dummy_encryption on remount");
>> - else if (err == -EINVAL)
>> - errorfc(fc, "Value of option \"%s\" is unrecognized",
>> - fsc->mount_options->test_dummy_encryption);
>> - else
>> - errorfc(fc, "Error processing option \"%s\" [%d]",
>> - fsc->mount_options->test_dummy_encryption, err);
>> - return err;
>> - }
>> - warnfc(fc, "test_dummy_encryption mode enabled");
>> + /* No changing encryption context on remount. */
>> + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
>> + !fscrypt_is_dummy_policy_set(&fsc->fsc_dummy_enc_policy)) {
>> + if (fscrypt_dummy_policies_equal(&fsopt->dummy_enc_policy,
>> + &fsc->fsc_dummy_enc_policy))
>> + return 0;
>> + errorfc(fc, "Can't set test_dummy_encryption on remount");
>> + return -EINVAL;
>> }
>> +
>> + /* Also make sure fsopt doesn't contain a conflicting value. */
>> + if (fscrypt_is_dummy_policy_set(&fsc->fsc_dummy_enc_policy)) {
>> + if (fscrypt_dummy_policies_equal(&fsopt->dummy_enc_policy,
>> + &fsc->fsc_dummy_enc_policy))
>> + return 0;
>> + errorfc(fc, "Conflicting test_dummy_encryption options");
>> + return -EINVAL;
>> + }
>> +
>> + fsc->fsc_dummy_enc_policy = fsopt->dummy_enc_policy;
>> + memset(&fsopt->dummy_enc_policy, 0, sizeof(fsopt->dummy_enc_policy));
>> +
>> + err = fscrypt_add_test_dummy_key(sb, &fsc->fsc_dummy_enc_policy);
>> + if (err) {
>> + errorfc(fc, "Error adding test dummy encryption key, %d", err);
>> + return err;
>> + }
>> +
>> + warnfc(fc, "test_dummy_encryption mode enabled");
>> return 0;
>> }
>> -#else
>> -static inline int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
>> - struct ceph_mount_options *fsopt)
>> -{
>> - if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC)
>> - warnfc(fc, "test_dummy_encryption mode ignored");
>> - return 0;
>> -}
>> -#endif
>>
>> /*
>> * mount: join the ceph cluster, and open root directory.
>> @@ -1154,7 +1156,7 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc,
>> goto out;
>> }
>>
>> - err = ceph_set_test_dummy_encryption(fsc->sb, fc, fsc->mount_options);
>> + err = ceph_apply_test_dummy_encryption(fsc->sb, fc, fsc->mount_options);
>> if (err)
>> goto out;
>>
>> @@ -1373,7 +1375,7 @@ static int ceph_reconfigure_fc(struct fs_context *fc)
>> struct super_block *sb = fc->root->d_sb;
>> struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
>>
>> - err = ceph_set_test_dummy_encryption(sb, fc, fsopt);
>> + err = ceph_apply_test_dummy_encryption(sb, fc, fsopt);
>> if (err)
>> return err;
>>
>> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
>> index bfc8bfcea799..5ea0ac6450dd 100644
>> --- a/fs/ceph/super.h
>> +++ b/fs/ceph/super.h
>> @@ -44,7 +44,6 @@
>> #define CEPH_MOUNT_OPT_ASYNC_DIROPS (1<<15) /* allow async directory ops */
>> #define CEPH_MOUNT_OPT_NOPAGECACHE (1<<16) /* bypass pagecache altogether */
>> #define CEPH_MOUNT_OPT_SPARSEREAD (1<<17) /* always do sparse reads */
>> -#define CEPH_MOUNT_OPT_TEST_DUMMY_ENC (1<<18) /* enable dummy encryption (for testing) */
>>
>> #define CEPH_MOUNT_OPT_DEFAULT \
>> (CEPH_MOUNT_OPT_DCACHE | \
>> @@ -101,7 +100,7 @@ struct ceph_mount_options {
>> char *server_path; /* default NULL (means "/") */
>> char *fscache_uniq; /* default NULL */
>> char *mon_addr;
>> - char *test_dummy_encryption; /* default NULL */
>> + struct fscrypt_dummy_policy dummy_enc_policy;
>> };
>>
>> #define CEPH_ASYNC_CREATE_CONFLICT_BITS 8
>> @@ -148,7 +147,7 @@ struct ceph_fs_client {
>> struct fscache_volume *fscache;
>> #endif
>> #ifdef CONFIG_FS_ENCRYPTION
>> - struct fscrypt_dummy_policy dummy_enc_policy;
>> + struct fscrypt_dummy_policy fsc_dummy_enc_policy;
>> #endif
>> };
>>
> LGTM
>
> Reviewed-by: Jeff Layton <[email protected]>

Thanks Jeff.

I will fold this into the previous commit.

-- Xiubo

2022-07-13 11:09:17

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] ceph: fix up test_dummy_encryption handling for new mount API

On Wed, 2022-07-13 at 16:56 +0800, [email protected] wrote:
> From: Xiubo Li <[email protected]>
>
> From Eric the "fscrypt_set_test_dummy_encryption()" will be removed
> in the next circle. Switch it to new APIs.
>
> Cc: Eric Biggers <[email protected]>
> Cc: Jeff Layton <[email protected]>
> Signed-off-by: Xiubo Li <[email protected]>
> ---
> fs/ceph/crypto.c | 4 +--
> fs/ceph/super.c | 94 ++++++++++++++++++++++++------------------------
> fs/ceph/super.h | 5 ++-
> 3 files changed, 52 insertions(+), 51 deletions(-)
>
> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> index 7e0c48e12554..5b807f8f4c69 100644
> --- a/fs/ceph/crypto.c
> +++ b/fs/ceph/crypto.c
> @@ -127,7 +127,7 @@ static bool ceph_crypt_empty_dir(struct inode *inode)
>
> static const union fscrypt_policy *ceph_get_dummy_policy(struct super_block *sb)
> {
> - return ceph_sb_to_client(sb)->dummy_enc_policy.policy;
> + return ceph_sb_to_client(sb)->fsc_dummy_enc_policy.policy;
> }
>
> static struct fscrypt_operations ceph_fscrypt_ops = {
> @@ -144,7 +144,7 @@ void ceph_fscrypt_set_ops(struct super_block *sb)
>
> void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc)
> {
> - fscrypt_free_dummy_policy(&fsc->dummy_enc_policy);
> + fscrypt_free_dummy_policy(&fsc->fsc_dummy_enc_policy);
> }
>
> int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index fa59a804b32c..4ac4a90755a2 100644
> --- a/fs/ceph/super.c
> +++ b/fs/ceph/super.c
> @@ -591,10 +591,16 @@ static int ceph_parse_mount_param(struct fs_context *fc,
> break;
> case Opt_test_dummy_encryption:
> #ifdef CONFIG_FS_ENCRYPTION
> - kfree(fsopt->test_dummy_encryption);
> - fsopt->test_dummy_encryption = param->string;
> - param->string = NULL;
> - fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC;
> + fscrypt_free_dummy_policy(&fsopt->dummy_enc_policy);
> + ret = fscrypt_parse_test_dummy_encryption(param,
> + &fsopt->dummy_enc_policy);
> + if (ret == -EINVAL) {
> + warnfc(fc, "Value of option \"%s\" is unrecognized",
> + param->key);
> + } else if (ret == -EEXIST) {
> + warnfc(fc, "Conflicting test_dummy_encryption options");
> + ret = -EINVAL;
> + }
> #else
> warnfc(fc,
> "FS encryption not supported: test_dummy_encryption mount option ignored");
> @@ -620,7 +626,7 @@ static void destroy_mount_options(struct ceph_mount_options *args)
> kfree(args->server_path);
> kfree(args->fscache_uniq);
> kfree(args->mon_addr);
> - kfree(args->test_dummy_encryption);
> + fscrypt_free_dummy_policy(&args->dummy_enc_policy);
> kfree(args);
> }
>
> @@ -1080,51 +1086,47 @@ static struct dentry *open_root_dentry(struct ceph_fs_client *fsc,
> return root;
> }
>
> -#ifdef CONFIG_FS_ENCRYPTION
> -static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
> - struct ceph_mount_options *fsopt)
> +static int ceph_apply_test_dummy_encryption(struct super_block *sb,
> + struct fs_context *fc,
> + struct ceph_mount_options *fsopt)
> {
> - /*
> - * No changing encryption context on remount. Note that
> - * fscrypt_set_test_dummy_encryption will validate the version
> - * string passed in (if any).
> - */
> - if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) {
> - struct ceph_fs_client *fsc = sb->s_fs_info;
> - int err = 0;
> + struct ceph_fs_client *fsc = sb->s_fs_info;
> + int err;
>
> - if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy) {
> - errorfc(fc, "Can't set test_dummy_encryption on remount");
> - return -EEXIST;
> - }
> + if (!fscrypt_is_dummy_policy_set(&fsopt->dummy_enc_policy))
> + return 0;
>
> - err = fscrypt_set_test_dummy_encryption(sb,
> - fsc->mount_options->test_dummy_encryption,
> - &fsc->dummy_enc_policy);
> - if (err) {
> - if (err == -EEXIST)
> - errorfc(fc, "Can't change test_dummy_encryption on remount");
> - else if (err == -EINVAL)
> - errorfc(fc, "Value of option \"%s\" is unrecognized",
> - fsc->mount_options->test_dummy_encryption);
> - else
> - errorfc(fc, "Error processing option \"%s\" [%d]",
> - fsc->mount_options->test_dummy_encryption, err);
> - return err;
> - }
> - warnfc(fc, "test_dummy_encryption mode enabled");
> + /* No changing encryption context on remount. */
> + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
> + !fscrypt_is_dummy_policy_set(&fsc->fsc_dummy_enc_policy)) {
> + if (fscrypt_dummy_policies_equal(&fsopt->dummy_enc_policy,
> + &fsc->fsc_dummy_enc_policy))
> + return 0;
> + errorfc(fc, "Can't set test_dummy_encryption on remount");
> + return -EINVAL;
> }
> +
> + /* Also make sure fsopt doesn't contain a conflicting value. */
> + if (fscrypt_is_dummy_policy_set(&fsc->fsc_dummy_enc_policy)) {
> + if (fscrypt_dummy_policies_equal(&fsopt->dummy_enc_policy,
> + &fsc->fsc_dummy_enc_policy))
> + return 0;
> + errorfc(fc, "Conflicting test_dummy_encryption options");
> + return -EINVAL;
> + }
> +
> + fsc->fsc_dummy_enc_policy = fsopt->dummy_enc_policy;
> + memset(&fsopt->dummy_enc_policy, 0, sizeof(fsopt->dummy_enc_policy));
> +
> + err = fscrypt_add_test_dummy_key(sb, &fsc->fsc_dummy_enc_policy);
> + if (err) {
> + errorfc(fc, "Error adding test dummy encryption key, %d", err);
> + return err;
> + }
> +
> + warnfc(fc, "test_dummy_encryption mode enabled");
> return 0;
> }
> -#else
> -static inline int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
> - struct ceph_mount_options *fsopt)
> -{
> - if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC)
> - warnfc(fc, "test_dummy_encryption mode ignored");
> - return 0;
> -}
> -#endif
>
> /*
> * mount: join the ceph cluster, and open root directory.
> @@ -1154,7 +1156,7 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc,
> goto out;
> }
>
> - err = ceph_set_test_dummy_encryption(fsc->sb, fc, fsc->mount_options);
> + err = ceph_apply_test_dummy_encryption(fsc->sb, fc, fsc->mount_options);
> if (err)
> goto out;
>
> @@ -1373,7 +1375,7 @@ static int ceph_reconfigure_fc(struct fs_context *fc)
> struct super_block *sb = fc->root->d_sb;
> struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
>
> - err = ceph_set_test_dummy_encryption(sb, fc, fsopt);
> + err = ceph_apply_test_dummy_encryption(sb, fc, fsopt);
> if (err)
> return err;
>
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index bfc8bfcea799..5ea0ac6450dd 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -44,7 +44,6 @@
> #define CEPH_MOUNT_OPT_ASYNC_DIROPS (1<<15) /* allow async directory ops */
> #define CEPH_MOUNT_OPT_NOPAGECACHE (1<<16) /* bypass pagecache altogether */
> #define CEPH_MOUNT_OPT_SPARSEREAD (1<<17) /* always do sparse reads */
> -#define CEPH_MOUNT_OPT_TEST_DUMMY_ENC (1<<18) /* enable dummy encryption (for testing) */
>
> #define CEPH_MOUNT_OPT_DEFAULT \
> (CEPH_MOUNT_OPT_DCACHE | \
> @@ -101,7 +100,7 @@ struct ceph_mount_options {
> char *server_path; /* default NULL (means "/") */
> char *fscache_uniq; /* default NULL */
> char *mon_addr;
> - char *test_dummy_encryption; /* default NULL */
> + struct fscrypt_dummy_policy dummy_enc_policy;
> };
>
> #define CEPH_ASYNC_CREATE_CONFLICT_BITS 8
> @@ -148,7 +147,7 @@ struct ceph_fs_client {
> struct fscache_volume *fscache;
> #endif
> #ifdef CONFIG_FS_ENCRYPTION
> - struct fscrypt_dummy_policy dummy_enc_policy;
> + struct fscrypt_dummy_policy fsc_dummy_enc_policy;
> #endif
> };
>

LGTM

Reviewed-by: Jeff Layton <[email protected]>

2022-07-13 13:19:17

by Xiubo Li

[permalink] [raw]
Subject: Re: [PATCH] ceph: fix up test_dummy_encryption handling for new mount API

Hi Jeff,

I am rebasing the 'wip-fscrypt' branch to the 'testing', and there is
one commit conflicts:

commit 81faddddbef409bcaa1aa3d89e59606cde94dab7
Author: Jeff Layton <[email protected]>
Date:?? Wed Dec 1 09:21:39 2021 -0500

??? ceph: add infrastructure for file encryption and decryption

??? ...and allow test_dummy_encryption to bypass content encryption
??? if mounted with test_dummy_encryption=clear.

??? Reviewed-by: Xiubo Li <[email protected]>
??? Signed-off-by: Jeff Layton <[email protected]>

And the conflicts are:

[xiubli@fedora ]$ cat fs/ceph/super.h.rej
--- fs/ceph/super.h
+++ fs/ceph/super.h
@@ -45,6 +45,7 @@
?#define CEPH_MOUNT_OPT_NOPAGECACHE???? (1<<16) /* bypass pagecache
altogether */
?#define CEPH_MOUNT_OPT_SPARSEREAD????? (1<<17) /* always do sparse
reads */
?#define CEPH_MOUNT_OPT_TEST_DUMMY_ENC? (1<<18) /* enable dummy
encryption (for testing) */
+#define CEPH_MOUNT_OPT_DUMMY_ENC_CLEAR (1<<19) /* don't actually
encrypt content */

?#define CEPH_MOUNT_OPT_DEFAULT??? ??? ??? \
???? (CEPH_MOUNT_OPT_DCACHE |??? ??? \

[xiubli@fedora ]$ cat fs/ceph/super.c.rej
--- fs/ceph/super.c
+++ fs/ceph/super.c
@@ -1098,6 +1098,14 @@ static int ceph_set_test_dummy_encryption(struct
super_block *sb, struct fs_cont
???? ??? ??? return -EEXIST;
???? ??? }

+??? ??? /* HACK: allow for cleartext "encryption" in files for testing */
+??? ??? if (fsc->mount_options->test_dummy_encryption &&
+ !strcmp(fsc->mount_options->test_dummy_encryption, "clear")) {
+??? ??? ??? fsopt->flags |= CEPH_MOUNT_OPT_DUMMY_ENC_CLEAR;
+ kfree(fsc->mount_options->test_dummy_encryption);
+??? ??? ??? fsc->mount_options->test_dummy_encryption = NULL;
+??? ??? }
+
???? ??? err = fscrypt_set_test_dummy_encryption(sb,
fsc->mount_options->test_dummy_encryption,
???? ??? ??? ??? ??? ??? ??? &fsc->dummy_enc_policy);

And I have pushed the tmp patches to
https://github.com/lxbsz/ceph-client/commits/lxb-wip-fscrypt3.

Please take a look is that okay ?

Thanks

-- Xiubo



On 7/13/22 6:42 PM, Jeff Layton wrote:
> On Wed, 2022-07-13 at 16:56 +0800, [email protected] wrote:
>> From: Xiubo Li <[email protected]>
>>
>> From Eric the "fscrypt_set_test_dummy_encryption()" will be removed
>> in the next circle. Switch it to new APIs.
>>
>> Cc: Eric Biggers <[email protected]>
>> Cc: Jeff Layton <[email protected]>
>> Signed-off-by: Xiubo Li <[email protected]>
>> ---
>> fs/ceph/crypto.c | 4 +--
>> fs/ceph/super.c | 94 ++++++++++++++++++++++++------------------------
>> fs/ceph/super.h | 5 ++-
>> 3 files changed, 52 insertions(+), 51 deletions(-)
>>
>> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
>> index 7e0c48e12554..5b807f8f4c69 100644
>> --- a/fs/ceph/crypto.c
>> +++ b/fs/ceph/crypto.c
>> @@ -127,7 +127,7 @@ static bool ceph_crypt_empty_dir(struct inode *inode)
>>
>> static const union fscrypt_policy *ceph_get_dummy_policy(struct super_block *sb)
>> {
>> - return ceph_sb_to_client(sb)->dummy_enc_policy.policy;
>> + return ceph_sb_to_client(sb)->fsc_dummy_enc_policy.policy;
>> }
>>
>> static struct fscrypt_operations ceph_fscrypt_ops = {
>> @@ -144,7 +144,7 @@ void ceph_fscrypt_set_ops(struct super_block *sb)
>>
>> void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc)
>> {
>> - fscrypt_free_dummy_policy(&fsc->dummy_enc_policy);
>> + fscrypt_free_dummy_policy(&fsc->fsc_dummy_enc_policy);
>> }
>>
>> int ceph_fscrypt_prepare_context(struct inode *dir, struct inode *inode,
>> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
>> index fa59a804b32c..4ac4a90755a2 100644
>> --- a/fs/ceph/super.c
>> +++ b/fs/ceph/super.c
>> @@ -591,10 +591,16 @@ static int ceph_parse_mount_param(struct fs_context *fc,
>> break;
>> case Opt_test_dummy_encryption:
>> #ifdef CONFIG_FS_ENCRYPTION
>> - kfree(fsopt->test_dummy_encryption);
>> - fsopt->test_dummy_encryption = param->string;
>> - param->string = NULL;
>> - fsopt->flags |= CEPH_MOUNT_OPT_TEST_DUMMY_ENC;
>> + fscrypt_free_dummy_policy(&fsopt->dummy_enc_policy);
>> + ret = fscrypt_parse_test_dummy_encryption(param,
>> + &fsopt->dummy_enc_policy);
>> + if (ret == -EINVAL) {
>> + warnfc(fc, "Value of option \"%s\" is unrecognized",
>> + param->key);
>> + } else if (ret == -EEXIST) {
>> + warnfc(fc, "Conflicting test_dummy_encryption options");
>> + ret = -EINVAL;
>> + }
>> #else
>> warnfc(fc,
>> "FS encryption not supported: test_dummy_encryption mount option ignored");
>> @@ -620,7 +626,7 @@ static void destroy_mount_options(struct ceph_mount_options *args)
>> kfree(args->server_path);
>> kfree(args->fscache_uniq);
>> kfree(args->mon_addr);
>> - kfree(args->test_dummy_encryption);
>> + fscrypt_free_dummy_policy(&args->dummy_enc_policy);
>> kfree(args);
>> }
>>
>> @@ -1080,51 +1086,47 @@ static struct dentry *open_root_dentry(struct ceph_fs_client *fsc,
>> return root;
>> }
>>
>> -#ifdef CONFIG_FS_ENCRYPTION
>> -static int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
>> - struct ceph_mount_options *fsopt)
>> +static int ceph_apply_test_dummy_encryption(struct super_block *sb,
>> + struct fs_context *fc,
>> + struct ceph_mount_options *fsopt)
>> {
>> - /*
>> - * No changing encryption context on remount. Note that
>> - * fscrypt_set_test_dummy_encryption will validate the version
>> - * string passed in (if any).
>> - */
>> - if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC) {
>> - struct ceph_fs_client *fsc = sb->s_fs_info;
>> - int err = 0;
>> + struct ceph_fs_client *fsc = sb->s_fs_info;
>> + int err;
>>
>> - if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && !fsc->dummy_enc_policy.policy) {
>> - errorfc(fc, "Can't set test_dummy_encryption on remount");
>> - return -EEXIST;
>> - }
>> + if (!fscrypt_is_dummy_policy_set(&fsopt->dummy_enc_policy))
>> + return 0;
>>
>> - err = fscrypt_set_test_dummy_encryption(sb,
>> - fsc->mount_options->test_dummy_encryption,
>> - &fsc->dummy_enc_policy);
>> - if (err) {
>> - if (err == -EEXIST)
>> - errorfc(fc, "Can't change test_dummy_encryption on remount");
>> - else if (err == -EINVAL)
>> - errorfc(fc, "Value of option \"%s\" is unrecognized",
>> - fsc->mount_options->test_dummy_encryption);
>> - else
>> - errorfc(fc, "Error processing option \"%s\" [%d]",
>> - fsc->mount_options->test_dummy_encryption, err);
>> - return err;
>> - }
>> - warnfc(fc, "test_dummy_encryption mode enabled");
>> + /* No changing encryption context on remount. */
>> + if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
>> + !fscrypt_is_dummy_policy_set(&fsc->fsc_dummy_enc_policy)) {
>> + if (fscrypt_dummy_policies_equal(&fsopt->dummy_enc_policy,
>> + &fsc->fsc_dummy_enc_policy))
>> + return 0;
>> + errorfc(fc, "Can't set test_dummy_encryption on remount");
>> + return -EINVAL;
>> }
>> +
>> + /* Also make sure fsopt doesn't contain a conflicting value. */
>> + if (fscrypt_is_dummy_policy_set(&fsc->fsc_dummy_enc_policy)) {
>> + if (fscrypt_dummy_policies_equal(&fsopt->dummy_enc_policy,
>> + &fsc->fsc_dummy_enc_policy))
>> + return 0;
>> + errorfc(fc, "Conflicting test_dummy_encryption options");
>> + return -EINVAL;
>> + }
>> +
>> + fsc->fsc_dummy_enc_policy = fsopt->dummy_enc_policy;
>> + memset(&fsopt->dummy_enc_policy, 0, sizeof(fsopt->dummy_enc_policy));
>> +
>> + err = fscrypt_add_test_dummy_key(sb, &fsc->fsc_dummy_enc_policy);
>> + if (err) {
>> + errorfc(fc, "Error adding test dummy encryption key, %d", err);
>> + return err;
>> + }
>> +
>> + warnfc(fc, "test_dummy_encryption mode enabled");
>> return 0;
>> }
>> -#else
>> -static inline int ceph_set_test_dummy_encryption(struct super_block *sb, struct fs_context *fc,
>> - struct ceph_mount_options *fsopt)
>> -{
>> - if (fsopt->flags & CEPH_MOUNT_OPT_TEST_DUMMY_ENC)
>> - warnfc(fc, "test_dummy_encryption mode ignored");
>> - return 0;
>> -}
>> -#endif
>>
>> /*
>> * mount: join the ceph cluster, and open root directory.
>> @@ -1154,7 +1156,7 @@ static struct dentry *ceph_real_mount(struct ceph_fs_client *fsc,
>> goto out;
>> }
>>
>> - err = ceph_set_test_dummy_encryption(fsc->sb, fc, fsc->mount_options);
>> + err = ceph_apply_test_dummy_encryption(fsc->sb, fc, fsc->mount_options);
>> if (err)
>> goto out;
>>
>> @@ -1373,7 +1375,7 @@ static int ceph_reconfigure_fc(struct fs_context *fc)
>> struct super_block *sb = fc->root->d_sb;
>> struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
>>
>> - err = ceph_set_test_dummy_encryption(sb, fc, fsopt);
>> + err = ceph_apply_test_dummy_encryption(sb, fc, fsopt);
>> if (err)
>> return err;
>>
>> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
>> index bfc8bfcea799..5ea0ac6450dd 100644
>> --- a/fs/ceph/super.h
>> +++ b/fs/ceph/super.h
>> @@ -44,7 +44,6 @@
>> #define CEPH_MOUNT_OPT_ASYNC_DIROPS (1<<15) /* allow async directory ops */
>> #define CEPH_MOUNT_OPT_NOPAGECACHE (1<<16) /* bypass pagecache altogether */
>> #define CEPH_MOUNT_OPT_SPARSEREAD (1<<17) /* always do sparse reads */
>> -#define CEPH_MOUNT_OPT_TEST_DUMMY_ENC (1<<18) /* enable dummy encryption (for testing) */
>>
>> #define CEPH_MOUNT_OPT_DEFAULT \
>> (CEPH_MOUNT_OPT_DCACHE | \
>> @@ -101,7 +100,7 @@ struct ceph_mount_options {
>> char *server_path; /* default NULL (means "/") */
>> char *fscache_uniq; /* default NULL */
>> char *mon_addr;
>> - char *test_dummy_encryption; /* default NULL */
>> + struct fscrypt_dummy_policy dummy_enc_policy;
>> };
>>
>> #define CEPH_ASYNC_CREATE_CONFLICT_BITS 8
>> @@ -148,7 +147,7 @@ struct ceph_fs_client {
>> struct fscache_volume *fscache;
>> #endif
>> #ifdef CONFIG_FS_ENCRYPTION
>> - struct fscrypt_dummy_policy dummy_enc_policy;
>> + struct fscrypt_dummy_policy fsc_dummy_enc_policy;
>> #endif
>> };
>>
> LGTM
>
> Reviewed-by: Jeff Layton <[email protected]>
>

2022-07-13 19:29:25

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] ceph: fix up test_dummy_encryption handling for new mount API

On Wed, 2022-07-13 at 20:55 +0800, Xiubo Li wrote:
> Hi Jeff,
>
> I am rebasing the 'wip-fscrypt' branch to the 'testing', and there is
> one commit conflicts:
>
> commit 81faddddbef409bcaa1aa3d89e59606cde94dab7
> Author: Jeff Layton <[email protected]>
> Date:?? Wed Dec 1 09:21:39 2021 -0500
>
> ??? ceph: add infrastructure for file encryption and decryption
>
> ??? ...and allow test_dummy_encryption to bypass content encryption
> ??? if mounted with test_dummy_encryption=clear.
>
> ??? Reviewed-by: Xiubo Li <[email protected]>
> ??? Signed-off-by: Jeff Layton <[email protected]>
>
> And the conflicts are:
>
> [xiubli@fedora ]$ cat fs/ceph/super.h.rej
> --- fs/ceph/super.h
> +++ fs/ceph/super.h
> @@ -45,6 +45,7 @@
> ?#define CEPH_MOUNT_OPT_NOPAGECACHE???? (1<<16) /* bypass pagecache
> altogether */
> ?#define CEPH_MOUNT_OPT_SPARSEREAD????? (1<<17) /* always do sparse
> reads */
> ?#define CEPH_MOUNT_OPT_TEST_DUMMY_ENC? (1<<18) /* enable dummy
> encryption (for testing) */
> +#define CEPH_MOUNT_OPT_DUMMY_ENC_CLEAR (1<<19) /* don't actually
> encrypt content */
>
> ?#define CEPH_MOUNT_OPT_DEFAULT??? ??? ??? \
> ???? (CEPH_MOUNT_OPT_DCACHE |??? ??? \
>
> [xiubli@fedora ]$ cat fs/ceph/super.c.rej
> --- fs/ceph/super.c
> +++ fs/ceph/super.c
> @@ -1098,6 +1098,14 @@ static int ceph_set_test_dummy_encryption(struct
> super_block *sb, struct fs_cont
> ???? ??? ??? return -EEXIST;
> ???? ??? }
>
> +??? ??? /* HACK: allow for cleartext "encryption" in files for testing */
> +??? ??? if (fsc->mount_options->test_dummy_encryption &&
> + !strcmp(fsc->mount_options->test_dummy_encryption, "clear")) {
> +??? ??? ??? fsopt->flags |= CEPH_MOUNT_OPT_DUMMY_ENC_CLEAR;
> + kfree(fsc->mount_options->test_dummy_encryption);
> +??? ??? ??? fsc->mount_options->test_dummy_encryption = NULL;
> +??? ??? }
> +
> ???? ??? err = fscrypt_set_test_dummy_encryption(sb,
> fsc->mount_options->test_dummy_encryption,
> ???? ??? ??? ??? ??? ??? ??? &fsc->dummy_enc_policy);
>
> And I have pushed the tmp patches to
> https://github.com/lxbsz/ceph-client/commits/lxb-wip-fscrypt3.
>
> Please take a look is that okay ?
>
> Thanks
>
> -- Xiubo
>
>

The result looks good to me. Thanks for fixing this up.

--
Jeff Layton <[email protected]>

2022-07-14 01:56:44

by Xiubo Li

[permalink] [raw]
Subject: Re: [PATCH] ceph: fix up test_dummy_encryption handling for new mount API


On 7/14/22 2:56 AM, Jeff Layton wrote:
> On Wed, 2022-07-13 at 20:55 +0800, Xiubo Li wrote:
>> Hi Jeff,
>>
>> I am rebasing the 'wip-fscrypt' branch to the 'testing', and there is
>> one commit conflicts:
>>
>> commit 81faddddbef409bcaa1aa3d89e59606cde94dab7
>> Author: Jeff Layton <[email protected]>
>> Date:?? Wed Dec 1 09:21:39 2021 -0500
>>
>> ??? ceph: add infrastructure for file encryption and decryption
>>
>> ??? ...and allow test_dummy_encryption to bypass content encryption
>> ??? if mounted with test_dummy_encryption=clear.
>>
>> ??? Reviewed-by: Xiubo Li <[email protected]>
>> ??? Signed-off-by: Jeff Layton <[email protected]>
>>
>> And the conflicts are:
>>
>> [xiubli@fedora ]$ cat fs/ceph/super.h.rej
>> --- fs/ceph/super.h
>> +++ fs/ceph/super.h
>> @@ -45,6 +45,7 @@
>> ?#define CEPH_MOUNT_OPT_NOPAGECACHE???? (1<<16) /* bypass pagecache
>> altogether */
>> ?#define CEPH_MOUNT_OPT_SPARSEREAD????? (1<<17) /* always do sparse
>> reads */
>> ?#define CEPH_MOUNT_OPT_TEST_DUMMY_ENC? (1<<18) /* enable dummy
>> encryption (for testing) */
>> +#define CEPH_MOUNT_OPT_DUMMY_ENC_CLEAR (1<<19) /* don't actually
>> encrypt content */
>>
>> ?#define CEPH_MOUNT_OPT_DEFAULT??? ??? ??? \
>> ???? (CEPH_MOUNT_OPT_DCACHE |??? ??? \
>>
>> [xiubli@fedora ]$ cat fs/ceph/super.c.rej
>> --- fs/ceph/super.c
>> +++ fs/ceph/super.c
>> @@ -1098,6 +1098,14 @@ static int ceph_set_test_dummy_encryption(struct
>> super_block *sb, struct fs_cont
>> ???? ??? ??? return -EEXIST;
>> ???? ??? }
>>
>> +??? ??? /* HACK: allow for cleartext "encryption" in files for testing */
>> +??? ??? if (fsc->mount_options->test_dummy_encryption &&
>> + !strcmp(fsc->mount_options->test_dummy_encryption, "clear")) {
>> +??? ??? ??? fsopt->flags |= CEPH_MOUNT_OPT_DUMMY_ENC_CLEAR;
>> + kfree(fsc->mount_options->test_dummy_encryption);
>> +??? ??? ??? fsc->mount_options->test_dummy_encryption = NULL;
>> +??? ??? }
>> +
>> ???? ??? err = fscrypt_set_test_dummy_encryption(sb,
>> fsc->mount_options->test_dummy_encryption,
>> ???? ??? ??? ??? ??? ??? ??? &fsc->dummy_enc_policy);
>>
>> And I have pushed the tmp patches to
>> https://github.com/lxbsz/ceph-client/commits/lxb-wip-fscrypt3.
>>
>> Please take a look is that okay ?
>>
>> Thanks
>>
>> -- Xiubo
>>
>>
> The result looks good to me. Thanks for fixing this up.

Cool. Thanks Jeff!


>