All of lore.kernel.org
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH v3 0/5] test_dummy_encryption fixes and cleanups
@ 2022-05-13 23:16 ` Eric Biggers
  0 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, Jaegeuk Kim

This series cleans up and fixes the way that ext4 and f2fs handle the
test_dummy_encryption mount option.

Some patches from v2 were already applied to the fscrypt and f2fs trees.
This series just includes the remaining patches.  Patches 1-2 can be
applied to the ext4 tree now.  The remaining patches will need to wait
until their prerequisites get merged via the fscrypt tree.

Changed from v2 (besides the omitted patches as mentioned above):
    - Split the parse_apply_sb_mount_options() fix into its own patch
    - Fixed a couple bugs in
      "ext4: fix up test_dummy_encryption handling for new mount API"

Eric Biggers (5):
  ext4: fix memory leak in parse_apply_sb_mount_options()
  ext4: only allow test_dummy_encryption when supported
  ext4: fix up test_dummy_encryption handling for new mount API
  f2fs: use the updated test_dummy_encryption helper functions
  fscrypt: remove fscrypt_set_test_dummy_encryption()

 fs/crypto/policy.c      |  13 ---
 fs/ext4/ext4.h          |   6 --
 fs/ext4/super.c         | 180 +++++++++++++++++++++++-----------------
 fs/f2fs/super.c         |  29 +++++--
 include/linux/fscrypt.h |   2 -
 5 files changed, 124 insertions(+), 106 deletions(-)

-- 
2.36.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 0/5] test_dummy_encryption fixes and cleanups
@ 2022-05-13 23:16 ` Eric Biggers
  0 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Jeff Layton, Lukas Czerner, Theodore Ts'o, Jaegeuk Kim

This series cleans up and fixes the way that ext4 and f2fs handle the
test_dummy_encryption mount option.

Some patches from v2 were already applied to the fscrypt and f2fs trees.
This series just includes the remaining patches.  Patches 1-2 can be
applied to the ext4 tree now.  The remaining patches will need to wait
until their prerequisites get merged via the fscrypt tree.

Changed from v2 (besides the omitted patches as mentioned above):
    - Split the parse_apply_sb_mount_options() fix into its own patch
    - Fixed a couple bugs in
      "ext4: fix up test_dummy_encryption handling for new mount API"

Eric Biggers (5):
  ext4: fix memory leak in parse_apply_sb_mount_options()
  ext4: only allow test_dummy_encryption when supported
  ext4: fix up test_dummy_encryption handling for new mount API
  f2fs: use the updated test_dummy_encryption helper functions
  fscrypt: remove fscrypt_set_test_dummy_encryption()

 fs/crypto/policy.c      |  13 ---
 fs/ext4/ext4.h          |   6 --
 fs/ext4/super.c         | 180 +++++++++++++++++++++++-----------------
 fs/f2fs/super.c         |  29 +++++--
 include/linux/fscrypt.h |   2 -
 5 files changed, 124 insertions(+), 106 deletions(-)

-- 
2.36.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH v3 1/5] ext4: fix memory leak in parse_apply_sb_mount_options()
  2022-05-13 23:16 ` Eric Biggers
@ 2022-05-13 23:16   ` Eric Biggers
  -1 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, stable, Jaegeuk Kim

From: Eric Biggers <ebiggers@google.com>

If processing the on-disk mount options fails after any memory was
allocated in the ext4_fs_context, e.g. s_qf_names, then this memory is
leaked.  Fix this by calling ext4_fc_free() instead of kfree() directly.

Reproducer:

    mkfs.ext4 -F /dev/vdc
    tune2fs /dev/vdc -E mount_opts=usrjquota=file
    echo clear > /sys/kernel/debug/kmemleak
    mount /dev/vdc /vdc
    echo scan > /sys/kernel/debug/kmemleak
    sleep 5
    echo scan > /sys/kernel/debug/kmemleak
    cat /sys/kernel/debug/kmemleak

Fixes: 7edfd85b1ffd ("ext4: Completely separate options parsing and sb setup")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/super.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 1466fbdbc8e34..60fa2f2623e07 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2625,8 +2625,10 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
 	ret = ext4_apply_options(fc, sb);
 
 out_free:
-	kfree(s_ctx);
-	kfree(fc);
+	if (fc) {
+		ext4_fc_free(fc);
+		kfree(fc);
+	}
 	kfree(s_mount_opts);
 	return ret;
 }
-- 
2.36.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 1/5] ext4: fix memory leak in parse_apply_sb_mount_options()
@ 2022-05-13 23:16   ` Eric Biggers
  0 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Jeff Layton, Lukas Czerner, Theodore Ts'o, Jaegeuk Kim, stable

From: Eric Biggers <ebiggers@google.com>

If processing the on-disk mount options fails after any memory was
allocated in the ext4_fs_context, e.g. s_qf_names, then this memory is
leaked.  Fix this by calling ext4_fc_free() instead of kfree() directly.

Reproducer:

    mkfs.ext4 -F /dev/vdc
    tune2fs /dev/vdc -E mount_opts=usrjquota=file
    echo clear > /sys/kernel/debug/kmemleak
    mount /dev/vdc /vdc
    echo scan > /sys/kernel/debug/kmemleak
    sleep 5
    echo scan > /sys/kernel/debug/kmemleak
    cat /sys/kernel/debug/kmemleak

Fixes: 7edfd85b1ffd ("ext4: Completely separate options parsing and sb setup")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/super.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 1466fbdbc8e34..60fa2f2623e07 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2625,8 +2625,10 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
 	ret = ext4_apply_options(fc, sb);
 
 out_free:
-	kfree(s_ctx);
-	kfree(fc);
+	if (fc) {
+		ext4_fc_free(fc);
+		kfree(fc);
+	}
 	kfree(s_mount_opts);
 	return ret;
 }
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH v3 2/5] ext4: only allow test_dummy_encryption when supported
  2022-05-13 23:16 ` Eric Biggers
@ 2022-05-13 23:16   ` Eric Biggers
  -1 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, Jaegeuk Kim

From: Eric Biggers <ebiggers@google.com>

Make the test_dummy_encryption mount option require that the encrypt
feature flag be already enabled on the filesystem, rather than
automatically enabling it.  Practically, this means that "-O encrypt"
will need to be included in MKFS_OPTIONS when running xfstests with the
test_dummy_encryption mount option.  (ext4/053 also needs an update.)

Moreover, as long as the preconditions for test_dummy_encryption are
being tightened anyway, take the opportunity to start rejecting it when
!CONFIG_FS_ENCRYPTION rather than ignoring it.

The motivation for requiring the encrypt feature flag is that:

- Having the filesystem auto-enable feature flags is problematic, as it
  bypasses the usual sanity checks.  The specific issue which came up
  recently is that in kernel versions where ext4 supports casefold but
  not encrypt+casefold (v5.1 through v5.10), the kernel will happily add
  the encrypt flag to a filesystem that has the casefold flag, making it
  unmountable -- but only for subsequent mounts, not the initial one.
  This confused the casefold support detection in xfstests, causing
  generic/556 to fail rather than be skipped.

- The xfstests-bld test runners (kvm-xfstests et al.) already use the
  required mkfs flag, so they will not be affected by this change.  Only
  users of test_dummy_encryption alone will be affected.  But, this
  option has always been for testing only, so it should be fine to
  require that the few users of this option update their test scripts.

- f2fs already requires it (for its equivalent feature flag).

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/super.c | 59 +++++++++++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 60fa2f2623e07..001915df62c07 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2427,11 +2427,12 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param)
 		ctx->spec |= EXT4_SPEC_DUMMY_ENCRYPTION;
 		ctx->test_dummy_enc_arg = kmemdup_nul(param->string, param->size,
 						      GFP_KERNEL);
+		return 0;
 #else
 		ext4_msg(NULL, KERN_WARNING,
-			 "Test dummy encryption mount option ignored");
+			 "test_dummy_encryption option not supported");
+		return -EINVAL;
 #endif
-		return 0;
 	case Opt_dax:
 	case Opt_dax_type:
 #ifdef CONFIG_FS_DAX
@@ -2788,12 +2789,43 @@ static int ext4_check_quota_consistency(struct fs_context *fc,
 #endif
 }
 
+static int ext4_check_test_dummy_encryption(const struct fs_context *fc,
+					    struct super_block *sb)
+{
+	const struct ext4_fs_context *ctx = fc->fs_private;
+	const struct ext4_sb_info *sbi = EXT4_SB(sb);
+
+	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION) ||
+	    !(ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION))
+		return 0;
+
+	if (!ext4_has_feature_encrypt(sb)) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "test_dummy_encryption requires encrypt feature");
+		return -EINVAL;
+	}
+	/*
+	 * This mount option is just for testing, and it's not worthwhile to
+	 * implement the extra complexity (e.g. RCU protection) that would be
+	 * needed to allow it to be set or changed during remount.  We do allow
+	 * it to be specified during remount, but only if there is no change.
+	 */
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
+	    !DUMMY_ENCRYPTION_ENABLED(sbi)) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "Can't set test_dummy_encryption on remount");
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int ext4_check_opt_consistency(struct fs_context *fc,
 				      struct super_block *sb)
 {
 	struct ext4_fs_context *ctx = fc->fs_private;
 	struct ext4_sb_info *sbi = fc->s_fs_info;
 	int is_remount = fc->purpose == FS_CONTEXT_FOR_RECONFIGURE;
+	int err;
 
 	if ((ctx->opt_flags & MOPT_NO_EXT2) && IS_EXT2_SB(sb)) {
 		ext4_msg(NULL, KERN_ERR,
@@ -2823,20 +2855,9 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
 				 "for blocksize < PAGE_SIZE");
 	}
 
-#ifdef CONFIG_FS_ENCRYPTION
-	/*
-	 * This mount option is just for testing, and it's not worthwhile to
-	 * implement the extra complexity (e.g. RCU protection) that would be
-	 * needed to allow it to be set or changed during remount.  We do allow
-	 * it to be specified during remount, but only if there is no change.
-	 */
-	if ((ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION) &&
-	    is_remount && !sbi->s_dummy_enc_policy.policy) {
-		ext4_msg(NULL, KERN_WARNING,
-			 "Can't set test_dummy_encryption on remount");
-		return -1;
-	}
-#endif
+	err = ext4_check_test_dummy_encryption(fc, sb);
+	if (err)
+		return err;
 
 	if ((ctx->spec & EXT4_SPEC_DATAJ) && is_remount) {
 		if (!sbi->s_journal) {
@@ -5281,12 +5302,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 		goto failed_mount_wq;
 	}
 
-	if (DUMMY_ENCRYPTION_ENABLED(sbi) && !sb_rdonly(sb) &&
-	    !ext4_has_feature_encrypt(sb)) {
-		ext4_set_feature_encrypt(sb);
-		ext4_commit_super(sb);
-	}
-
 	/*
 	 * Get the # of file system overhead blocks from the
 	 * superblock if present.
-- 
2.36.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 2/5] ext4: only allow test_dummy_encryption when supported
@ 2022-05-13 23:16   ` Eric Biggers
  0 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Jeff Layton, Lukas Czerner, Theodore Ts'o, Jaegeuk Kim

From: Eric Biggers <ebiggers@google.com>

Make the test_dummy_encryption mount option require that the encrypt
feature flag be already enabled on the filesystem, rather than
automatically enabling it.  Practically, this means that "-O encrypt"
will need to be included in MKFS_OPTIONS when running xfstests with the
test_dummy_encryption mount option.  (ext4/053 also needs an update.)

Moreover, as long as the preconditions for test_dummy_encryption are
being tightened anyway, take the opportunity to start rejecting it when
!CONFIG_FS_ENCRYPTION rather than ignoring it.

The motivation for requiring the encrypt feature flag is that:

- Having the filesystem auto-enable feature flags is problematic, as it
  bypasses the usual sanity checks.  The specific issue which came up
  recently is that in kernel versions where ext4 supports casefold but
  not encrypt+casefold (v5.1 through v5.10), the kernel will happily add
  the encrypt flag to a filesystem that has the casefold flag, making it
  unmountable -- but only for subsequent mounts, not the initial one.
  This confused the casefold support detection in xfstests, causing
  generic/556 to fail rather than be skipped.

- The xfstests-bld test runners (kvm-xfstests et al.) already use the
  required mkfs flag, so they will not be affected by this change.  Only
  users of test_dummy_encryption alone will be affected.  But, this
  option has always been for testing only, so it should be fine to
  require that the few users of this option update their test scripts.

- f2fs already requires it (for its equivalent feature flag).

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/super.c | 59 +++++++++++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 60fa2f2623e07..001915df62c07 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2427,11 +2427,12 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param)
 		ctx->spec |= EXT4_SPEC_DUMMY_ENCRYPTION;
 		ctx->test_dummy_enc_arg = kmemdup_nul(param->string, param->size,
 						      GFP_KERNEL);
+		return 0;
 #else
 		ext4_msg(NULL, KERN_WARNING,
-			 "Test dummy encryption mount option ignored");
+			 "test_dummy_encryption option not supported");
+		return -EINVAL;
 #endif
-		return 0;
 	case Opt_dax:
 	case Opt_dax_type:
 #ifdef CONFIG_FS_DAX
@@ -2788,12 +2789,43 @@ static int ext4_check_quota_consistency(struct fs_context *fc,
 #endif
 }
 
+static int ext4_check_test_dummy_encryption(const struct fs_context *fc,
+					    struct super_block *sb)
+{
+	const struct ext4_fs_context *ctx = fc->fs_private;
+	const struct ext4_sb_info *sbi = EXT4_SB(sb);
+
+	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION) ||
+	    !(ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION))
+		return 0;
+
+	if (!ext4_has_feature_encrypt(sb)) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "test_dummy_encryption requires encrypt feature");
+		return -EINVAL;
+	}
+	/*
+	 * This mount option is just for testing, and it's not worthwhile to
+	 * implement the extra complexity (e.g. RCU protection) that would be
+	 * needed to allow it to be set or changed during remount.  We do allow
+	 * it to be specified during remount, but only if there is no change.
+	 */
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
+	    !DUMMY_ENCRYPTION_ENABLED(sbi)) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "Can't set test_dummy_encryption on remount");
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int ext4_check_opt_consistency(struct fs_context *fc,
 				      struct super_block *sb)
 {
 	struct ext4_fs_context *ctx = fc->fs_private;
 	struct ext4_sb_info *sbi = fc->s_fs_info;
 	int is_remount = fc->purpose == FS_CONTEXT_FOR_RECONFIGURE;
+	int err;
 
 	if ((ctx->opt_flags & MOPT_NO_EXT2) && IS_EXT2_SB(sb)) {
 		ext4_msg(NULL, KERN_ERR,
@@ -2823,20 +2855,9 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
 				 "for blocksize < PAGE_SIZE");
 	}
 
-#ifdef CONFIG_FS_ENCRYPTION
-	/*
-	 * This mount option is just for testing, and it's not worthwhile to
-	 * implement the extra complexity (e.g. RCU protection) that would be
-	 * needed to allow it to be set or changed during remount.  We do allow
-	 * it to be specified during remount, but only if there is no change.
-	 */
-	if ((ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION) &&
-	    is_remount && !sbi->s_dummy_enc_policy.policy) {
-		ext4_msg(NULL, KERN_WARNING,
-			 "Can't set test_dummy_encryption on remount");
-		return -1;
-	}
-#endif
+	err = ext4_check_test_dummy_encryption(fc, sb);
+	if (err)
+		return err;
 
 	if ((ctx->spec & EXT4_SPEC_DATAJ) && is_remount) {
 		if (!sbi->s_journal) {
@@ -5281,12 +5302,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 		goto failed_mount_wq;
 	}
 
-	if (DUMMY_ENCRYPTION_ENABLED(sbi) && !sb_rdonly(sb) &&
-	    !ext4_has_feature_encrypt(sb)) {
-		ext4_set_feature_encrypt(sb);
-		ext4_commit_super(sb);
-	}
-
 	/*
 	 * Get the # of file system overhead blocks from the
 	 * superblock if present.
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH v3 3/5] ext4: fix up test_dummy_encryption handling for new mount API
  2022-05-13 23:16 ` Eric Biggers
@ 2022-05-13 23:16   ` Eric Biggers
  -1 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, Jaegeuk Kim

From: Eric Biggers <ebiggers@google.com>

Since ext4 was converted to the new mount API, the test_dummy_encryption
mount option isn't being handled entirely correctly, because the needed
fscrypt_set_test_dummy_encryption() helper function combines
parsing/checking/applying into one function.  That doesn't work well
with the new mount API, which split these into separate steps.

This was sort of okay anyway, due to the parsing logic that was copied
from fscrypt_set_test_dummy_encryption() into ext4_parse_param(),
combined with an additional check in ext4_check_test_dummy_encryption().
However, these overlooked the case of changing the value of
test_dummy_encryption on remount, which isn't allowed but ext4 wasn't
detecting until ext4_apply_options() when it's too late to fail.
Another bug is that if test_dummy_encryption was specified multiple
times with an argument, memory was leaked.

Fix this up properly by using the new helper functions that allow
splitting up the parse/check/apply steps for test_dummy_encryption.

Fixes: cebe85d570cf ("ext4: switch to the new mount api")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/ext4.h  |   6 ---
 fs/ext4/super.c | 133 ++++++++++++++++++++++++++----------------------
 2 files changed, 71 insertions(+), 68 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index a743b1e3b89ec..f6d6661817b63 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1440,12 +1440,6 @@ struct ext4_super_block {
 
 #ifdef __KERNEL__
 
-#ifdef CONFIG_FS_ENCRYPTION
-#define DUMMY_ENCRYPTION_ENABLED(sbi) ((sbi)->s_dummy_enc_policy.policy != NULL)
-#else
-#define DUMMY_ENCRYPTION_ENABLED(sbi) (0)
-#endif
-
 /* Number of quota types we support */
 #define EXT4_MAXQUOTAS 3
 
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 001915df62c07..5a75da164418c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -87,7 +87,7 @@ static struct inode *ext4_get_journal_inode(struct super_block *sb,
 static int ext4_validate_options(struct fs_context *fc);
 static int ext4_check_opt_consistency(struct fs_context *fc,
 				      struct super_block *sb);
-static int ext4_apply_options(struct fs_context *fc, struct super_block *sb);
+static void ext4_apply_options(struct fs_context *fc, struct super_block *sb);
 static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param);
 static int ext4_get_tree(struct fs_context *fc);
 static int ext4_reconfigure(struct fs_context *fc);
@@ -1989,31 +1989,12 @@ ext4_sb_read_encoding(const struct ext4_super_block *es)
 }
 #endif
 
-static int ext4_set_test_dummy_encryption(struct super_block *sb, char *arg)
-{
-#ifdef CONFIG_FS_ENCRYPTION
-	struct ext4_sb_info *sbi = EXT4_SB(sb);
-	int err;
-
-	err = fscrypt_set_test_dummy_encryption(sb, arg,
-						&sbi->s_dummy_enc_policy);
-	if (err) {
-		ext4_msg(sb, KERN_WARNING,
-			 "Error while setting test dummy encryption [%d]", err);
-		return err;
-	}
-	ext4_msg(sb, KERN_WARNING, "Test dummy encryption mode enabled");
-#endif
-	return 0;
-}
-
 #define EXT4_SPEC_JQUOTA			(1 <<  0)
 #define EXT4_SPEC_JQFMT				(1 <<  1)
 #define EXT4_SPEC_DATAJ				(1 <<  2)
 #define EXT4_SPEC_SB_BLOCK			(1 <<  3)
 #define EXT4_SPEC_JOURNAL_DEV			(1 <<  4)
 #define EXT4_SPEC_JOURNAL_IOPRIO		(1 <<  5)
-#define EXT4_SPEC_DUMMY_ENCRYPTION		(1 <<  6)
 #define EXT4_SPEC_s_want_extra_isize		(1 <<  7)
 #define EXT4_SPEC_s_max_batch_time		(1 <<  8)
 #define EXT4_SPEC_s_min_batch_time		(1 <<  9)
@@ -2030,7 +2011,7 @@ static int ext4_set_test_dummy_encryption(struct super_block *sb, char *arg)
 
 struct ext4_fs_context {
 	char		*s_qf_names[EXT4_MAXQUOTAS];
-	char		*test_dummy_enc_arg;
+	struct fscrypt_dummy_policy dummy_enc_policy;
 	int		s_jquota_fmt;	/* Format of quota to use */
 #ifdef CONFIG_EXT4_DEBUG
 	int s_fc_debug_max_replay;
@@ -2072,7 +2053,7 @@ static void ext4_fc_free(struct fs_context *fc)
 	for (i = 0; i < EXT4_MAXQUOTAS; i++)
 		kfree(ctx->s_qf_names[i]);
 
-	kfree(ctx->test_dummy_enc_arg);
+	fscrypt_free_dummy_policy(&ctx->dummy_enc_policy);
 	kfree(ctx);
 }
 
@@ -2148,6 +2129,29 @@ static int unnote_qf_name(struct fs_context *fc, int qtype)
 }
 #endif
 
+static int ext4_parse_test_dummy_encryption(const struct fs_parameter *param,
+					    struct ext4_fs_context *ctx)
+{
+	int err;
+
+	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION)) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "test_dummy_encryption option not supported");
+		return -EINVAL;
+	}
+	err = fscrypt_parse_test_dummy_encryption(param,
+						  &ctx->dummy_enc_policy);
+	if (err == -EINVAL) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "Value of option \"%s\" is unrecognized", param->key);
+	} else if (err == -EEXIST) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "Conflicting test_dummy_encryption options");
+		return -EINVAL;
+	}
+	return err;
+}
+
 #define EXT4_SET_CTX(name)						\
 static inline void ctx_set_##name(struct ext4_fs_context *ctx,		\
 				  unsigned long flag)			\
@@ -2410,29 +2414,7 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param)
 		ctx->spec |= EXT4_SPEC_JOURNAL_IOPRIO;
 		return 0;
 	case Opt_test_dummy_encryption:
-#ifdef CONFIG_FS_ENCRYPTION
-		if (param->type == fs_value_is_flag) {
-			ctx->spec |= EXT4_SPEC_DUMMY_ENCRYPTION;
-			ctx->test_dummy_enc_arg = NULL;
-			return 0;
-		}
-		if (*param->string &&
-		    !(!strcmp(param->string, "v1") ||
-		      !strcmp(param->string, "v2"))) {
-			ext4_msg(NULL, KERN_WARNING,
-				 "Value of option \"%s\" is unrecognized",
-				 param->key);
-			return -EINVAL;
-		}
-		ctx->spec |= EXT4_SPEC_DUMMY_ENCRYPTION;
-		ctx->test_dummy_enc_arg = kmemdup_nul(param->string, param->size,
-						      GFP_KERNEL);
-		return 0;
-#else
-		ext4_msg(NULL, KERN_WARNING,
-			 "test_dummy_encryption option not supported");
-		return -EINVAL;
-#endif
+		return ext4_parse_test_dummy_encryption(param, ctx);
 	case Opt_dax:
 	case Opt_dax_type:
 #ifdef CONFIG_FS_DAX
@@ -2623,7 +2605,8 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
 	if (s_ctx->spec & EXT4_SPEC_JOURNAL_IOPRIO)
 		m_ctx->journal_ioprio = s_ctx->journal_ioprio;
 
-	ret = ext4_apply_options(fc, sb);
+	ext4_apply_options(fc, sb);
+	ret = 0;
 
 out_free:
 	if (fc) {
@@ -2794,9 +2777,9 @@ static int ext4_check_test_dummy_encryption(const struct fs_context *fc,
 {
 	const struct ext4_fs_context *ctx = fc->fs_private;
 	const struct ext4_sb_info *sbi = EXT4_SB(sb);
+	int err;
 
-	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION) ||
-	    !(ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION))
+	if (!fscrypt_is_dummy_policy_set(&ctx->dummy_enc_policy))
 		return 0;
 
 	if (!ext4_has_feature_encrypt(sb)) {
@@ -2810,13 +2793,46 @@ static int ext4_check_test_dummy_encryption(const struct fs_context *fc,
 	 * needed to allow it to be set or changed during remount.  We do allow
 	 * it to be specified during remount, but only if there is no change.
 	 */
-	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
-	    !DUMMY_ENCRYPTION_ENABLED(sbi)) {
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		if (fscrypt_dummy_policies_equal(&sbi->s_dummy_enc_policy,
+						 &ctx->dummy_enc_policy))
+			return 0;
 		ext4_msg(NULL, KERN_WARNING,
-			 "Can't set test_dummy_encryption on remount");
+			 "Can't set or change test_dummy_encryption on remount");
 		return -EINVAL;
 	}
-	return 0;
+	/* Also make sure s_mount_opts didn't contain a conflicting value. */
+	if (fscrypt_is_dummy_policy_set(&sbi->s_dummy_enc_policy)) {
+		if (fscrypt_dummy_policies_equal(&sbi->s_dummy_enc_policy,
+						 &ctx->dummy_enc_policy))
+			return 0;
+		ext4_msg(NULL, KERN_WARNING,
+			 "Conflicting test_dummy_encryption options");
+		return -EINVAL;
+	}
+	/*
+	 * fscrypt_add_test_dummy_key() technically changes the super_block, so
+	 * technically it should be delayed until ext4_apply_options() like the
+	 * other changes.  But since we never get here for remounts (see above),
+	 * and this is the last chance to report errors, we do it here.
+	 */
+	err = fscrypt_add_test_dummy_key(sb, &ctx->dummy_enc_policy);
+	if (err)
+		ext4_msg(NULL, KERN_WARNING,
+			 "Error adding test dummy encryption key [%d]", err);
+	return err;
+}
+
+static void ext4_apply_test_dummy_encryption(struct ext4_fs_context *ctx,
+					     struct super_block *sb)
+{
+	if (!fscrypt_is_dummy_policy_set(&ctx->dummy_enc_policy) ||
+	    /* if already set, it was already verified to be the same */
+	    fscrypt_is_dummy_policy_set(&EXT4_SB(sb)->s_dummy_enc_policy))
+		return;
+	EXT4_SB(sb)->s_dummy_enc_policy = ctx->dummy_enc_policy;
+	memset(&ctx->dummy_enc_policy, 0, sizeof(ctx->dummy_enc_policy));
+	ext4_msg(sb, KERN_WARNING, "Test dummy encryption mode enabled");
 }
 
 static int ext4_check_opt_consistency(struct fs_context *fc,
@@ -2903,11 +2919,10 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
 	return ext4_check_quota_consistency(fc, sb);
 }
 
-static int ext4_apply_options(struct fs_context *fc, struct super_block *sb)
+static void ext4_apply_options(struct fs_context *fc, struct super_block *sb)
 {
 	struct ext4_fs_context *ctx = fc->fs_private;
 	struct ext4_sb_info *sbi = fc->s_fs_info;
-	int ret = 0;
 
 	sbi->s_mount_opt &= ~ctx->mask_s_mount_opt;
 	sbi->s_mount_opt |= ctx->vals_s_mount_opt;
@@ -2943,11 +2958,7 @@ static int ext4_apply_options(struct fs_context *fc, struct super_block *sb)
 #endif
 
 	ext4_apply_quota_options(fc, sb);
-
-	if (ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION)
-		ret = ext4_set_test_dummy_encryption(sb, ctx->test_dummy_enc_arg);
-
-	return ret;
+	ext4_apply_test_dummy_encryption(ctx, sb);
 }
 
 
@@ -4669,9 +4680,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 	if (err < 0)
 		goto failed_mount;
 
-	err = ext4_apply_options(fc, sb);
-	if (err < 0)
-		goto failed_mount;
+	ext4_apply_options(fc, sb);
 
 #if IS_ENABLED(CONFIG_UNICODE)
 	if (ext4_has_feature_casefold(sb) && !sb->s_encoding) {
-- 
2.36.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 3/5] ext4: fix up test_dummy_encryption handling for new mount API
@ 2022-05-13 23:16   ` Eric Biggers
  0 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Jeff Layton, Lukas Czerner, Theodore Ts'o, Jaegeuk Kim

From: Eric Biggers <ebiggers@google.com>

Since ext4 was converted to the new mount API, the test_dummy_encryption
mount option isn't being handled entirely correctly, because the needed
fscrypt_set_test_dummy_encryption() helper function combines
parsing/checking/applying into one function.  That doesn't work well
with the new mount API, which split these into separate steps.

This was sort of okay anyway, due to the parsing logic that was copied
from fscrypt_set_test_dummy_encryption() into ext4_parse_param(),
combined with an additional check in ext4_check_test_dummy_encryption().
However, these overlooked the case of changing the value of
test_dummy_encryption on remount, which isn't allowed but ext4 wasn't
detecting until ext4_apply_options() when it's too late to fail.
Another bug is that if test_dummy_encryption was specified multiple
times with an argument, memory was leaked.

Fix this up properly by using the new helper functions that allow
splitting up the parse/check/apply steps for test_dummy_encryption.

Fixes: cebe85d570cf ("ext4: switch to the new mount api")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/ext4.h  |   6 ---
 fs/ext4/super.c | 133 ++++++++++++++++++++++++++----------------------
 2 files changed, 71 insertions(+), 68 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index a743b1e3b89ec..f6d6661817b63 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1440,12 +1440,6 @@ struct ext4_super_block {
 
 #ifdef __KERNEL__
 
-#ifdef CONFIG_FS_ENCRYPTION
-#define DUMMY_ENCRYPTION_ENABLED(sbi) ((sbi)->s_dummy_enc_policy.policy != NULL)
-#else
-#define DUMMY_ENCRYPTION_ENABLED(sbi) (0)
-#endif
-
 /* Number of quota types we support */
 #define EXT4_MAXQUOTAS 3
 
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 001915df62c07..5a75da164418c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -87,7 +87,7 @@ static struct inode *ext4_get_journal_inode(struct super_block *sb,
 static int ext4_validate_options(struct fs_context *fc);
 static int ext4_check_opt_consistency(struct fs_context *fc,
 				      struct super_block *sb);
-static int ext4_apply_options(struct fs_context *fc, struct super_block *sb);
+static void ext4_apply_options(struct fs_context *fc, struct super_block *sb);
 static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param);
 static int ext4_get_tree(struct fs_context *fc);
 static int ext4_reconfigure(struct fs_context *fc);
@@ -1989,31 +1989,12 @@ ext4_sb_read_encoding(const struct ext4_super_block *es)
 }
 #endif
 
-static int ext4_set_test_dummy_encryption(struct super_block *sb, char *arg)
-{
-#ifdef CONFIG_FS_ENCRYPTION
-	struct ext4_sb_info *sbi = EXT4_SB(sb);
-	int err;
-
-	err = fscrypt_set_test_dummy_encryption(sb, arg,
-						&sbi->s_dummy_enc_policy);
-	if (err) {
-		ext4_msg(sb, KERN_WARNING,
-			 "Error while setting test dummy encryption [%d]", err);
-		return err;
-	}
-	ext4_msg(sb, KERN_WARNING, "Test dummy encryption mode enabled");
-#endif
-	return 0;
-}
-
 #define EXT4_SPEC_JQUOTA			(1 <<  0)
 #define EXT4_SPEC_JQFMT				(1 <<  1)
 #define EXT4_SPEC_DATAJ				(1 <<  2)
 #define EXT4_SPEC_SB_BLOCK			(1 <<  3)
 #define EXT4_SPEC_JOURNAL_DEV			(1 <<  4)
 #define EXT4_SPEC_JOURNAL_IOPRIO		(1 <<  5)
-#define EXT4_SPEC_DUMMY_ENCRYPTION		(1 <<  6)
 #define EXT4_SPEC_s_want_extra_isize		(1 <<  7)
 #define EXT4_SPEC_s_max_batch_time		(1 <<  8)
 #define EXT4_SPEC_s_min_batch_time		(1 <<  9)
@@ -2030,7 +2011,7 @@ static int ext4_set_test_dummy_encryption(struct super_block *sb, char *arg)
 
 struct ext4_fs_context {
 	char		*s_qf_names[EXT4_MAXQUOTAS];
-	char		*test_dummy_enc_arg;
+	struct fscrypt_dummy_policy dummy_enc_policy;
 	int		s_jquota_fmt;	/* Format of quota to use */
 #ifdef CONFIG_EXT4_DEBUG
 	int s_fc_debug_max_replay;
@@ -2072,7 +2053,7 @@ static void ext4_fc_free(struct fs_context *fc)
 	for (i = 0; i < EXT4_MAXQUOTAS; i++)
 		kfree(ctx->s_qf_names[i]);
 
-	kfree(ctx->test_dummy_enc_arg);
+	fscrypt_free_dummy_policy(&ctx->dummy_enc_policy);
 	kfree(ctx);
 }
 
@@ -2148,6 +2129,29 @@ static int unnote_qf_name(struct fs_context *fc, int qtype)
 }
 #endif
 
+static int ext4_parse_test_dummy_encryption(const struct fs_parameter *param,
+					    struct ext4_fs_context *ctx)
+{
+	int err;
+
+	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION)) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "test_dummy_encryption option not supported");
+		return -EINVAL;
+	}
+	err = fscrypt_parse_test_dummy_encryption(param,
+						  &ctx->dummy_enc_policy);
+	if (err == -EINVAL) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "Value of option \"%s\" is unrecognized", param->key);
+	} else if (err == -EEXIST) {
+		ext4_msg(NULL, KERN_WARNING,
+			 "Conflicting test_dummy_encryption options");
+		return -EINVAL;
+	}
+	return err;
+}
+
 #define EXT4_SET_CTX(name)						\
 static inline void ctx_set_##name(struct ext4_fs_context *ctx,		\
 				  unsigned long flag)			\
@@ -2410,29 +2414,7 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param)
 		ctx->spec |= EXT4_SPEC_JOURNAL_IOPRIO;
 		return 0;
 	case Opt_test_dummy_encryption:
-#ifdef CONFIG_FS_ENCRYPTION
-		if (param->type == fs_value_is_flag) {
-			ctx->spec |= EXT4_SPEC_DUMMY_ENCRYPTION;
-			ctx->test_dummy_enc_arg = NULL;
-			return 0;
-		}
-		if (*param->string &&
-		    !(!strcmp(param->string, "v1") ||
-		      !strcmp(param->string, "v2"))) {
-			ext4_msg(NULL, KERN_WARNING,
-				 "Value of option \"%s\" is unrecognized",
-				 param->key);
-			return -EINVAL;
-		}
-		ctx->spec |= EXT4_SPEC_DUMMY_ENCRYPTION;
-		ctx->test_dummy_enc_arg = kmemdup_nul(param->string, param->size,
-						      GFP_KERNEL);
-		return 0;
-#else
-		ext4_msg(NULL, KERN_WARNING,
-			 "test_dummy_encryption option not supported");
-		return -EINVAL;
-#endif
+		return ext4_parse_test_dummy_encryption(param, ctx);
 	case Opt_dax:
 	case Opt_dax_type:
 #ifdef CONFIG_FS_DAX
@@ -2623,7 +2605,8 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
 	if (s_ctx->spec & EXT4_SPEC_JOURNAL_IOPRIO)
 		m_ctx->journal_ioprio = s_ctx->journal_ioprio;
 
-	ret = ext4_apply_options(fc, sb);
+	ext4_apply_options(fc, sb);
+	ret = 0;
 
 out_free:
 	if (fc) {
@@ -2794,9 +2777,9 @@ static int ext4_check_test_dummy_encryption(const struct fs_context *fc,
 {
 	const struct ext4_fs_context *ctx = fc->fs_private;
 	const struct ext4_sb_info *sbi = EXT4_SB(sb);
+	int err;
 
-	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION) ||
-	    !(ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION))
+	if (!fscrypt_is_dummy_policy_set(&ctx->dummy_enc_policy))
 		return 0;
 
 	if (!ext4_has_feature_encrypt(sb)) {
@@ -2810,13 +2793,46 @@ static int ext4_check_test_dummy_encryption(const struct fs_context *fc,
 	 * needed to allow it to be set or changed during remount.  We do allow
 	 * it to be specified during remount, but only if there is no change.
 	 */
-	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE &&
-	    !DUMMY_ENCRYPTION_ENABLED(sbi)) {
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		if (fscrypt_dummy_policies_equal(&sbi->s_dummy_enc_policy,
+						 &ctx->dummy_enc_policy))
+			return 0;
 		ext4_msg(NULL, KERN_WARNING,
-			 "Can't set test_dummy_encryption on remount");
+			 "Can't set or change test_dummy_encryption on remount");
 		return -EINVAL;
 	}
-	return 0;
+	/* Also make sure s_mount_opts didn't contain a conflicting value. */
+	if (fscrypt_is_dummy_policy_set(&sbi->s_dummy_enc_policy)) {
+		if (fscrypt_dummy_policies_equal(&sbi->s_dummy_enc_policy,
+						 &ctx->dummy_enc_policy))
+			return 0;
+		ext4_msg(NULL, KERN_WARNING,
+			 "Conflicting test_dummy_encryption options");
+		return -EINVAL;
+	}
+	/*
+	 * fscrypt_add_test_dummy_key() technically changes the super_block, so
+	 * technically it should be delayed until ext4_apply_options() like the
+	 * other changes.  But since we never get here for remounts (see above),
+	 * and this is the last chance to report errors, we do it here.
+	 */
+	err = fscrypt_add_test_dummy_key(sb, &ctx->dummy_enc_policy);
+	if (err)
+		ext4_msg(NULL, KERN_WARNING,
+			 "Error adding test dummy encryption key [%d]", err);
+	return err;
+}
+
+static void ext4_apply_test_dummy_encryption(struct ext4_fs_context *ctx,
+					     struct super_block *sb)
+{
+	if (!fscrypt_is_dummy_policy_set(&ctx->dummy_enc_policy) ||
+	    /* if already set, it was already verified to be the same */
+	    fscrypt_is_dummy_policy_set(&EXT4_SB(sb)->s_dummy_enc_policy))
+		return;
+	EXT4_SB(sb)->s_dummy_enc_policy = ctx->dummy_enc_policy;
+	memset(&ctx->dummy_enc_policy, 0, sizeof(ctx->dummy_enc_policy));
+	ext4_msg(sb, KERN_WARNING, "Test dummy encryption mode enabled");
 }
 
 static int ext4_check_opt_consistency(struct fs_context *fc,
@@ -2903,11 +2919,10 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
 	return ext4_check_quota_consistency(fc, sb);
 }
 
-static int ext4_apply_options(struct fs_context *fc, struct super_block *sb)
+static void ext4_apply_options(struct fs_context *fc, struct super_block *sb)
 {
 	struct ext4_fs_context *ctx = fc->fs_private;
 	struct ext4_sb_info *sbi = fc->s_fs_info;
-	int ret = 0;
 
 	sbi->s_mount_opt &= ~ctx->mask_s_mount_opt;
 	sbi->s_mount_opt |= ctx->vals_s_mount_opt;
@@ -2943,11 +2958,7 @@ static int ext4_apply_options(struct fs_context *fc, struct super_block *sb)
 #endif
 
 	ext4_apply_quota_options(fc, sb);
-
-	if (ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION)
-		ret = ext4_set_test_dummy_encryption(sb, ctx->test_dummy_enc_arg);
-
-	return ret;
+	ext4_apply_test_dummy_encryption(ctx, sb);
 }
 
 
@@ -4669,9 +4680,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 	if (err < 0)
 		goto failed_mount;
 
-	err = ext4_apply_options(fc, sb);
-	if (err < 0)
-		goto failed_mount;
+	ext4_apply_options(fc, sb);
 
 #if IS_ENABLED(CONFIG_UNICODE)
 	if (ext4_has_feature_casefold(sb) && !sb->s_encoding) {
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH v3 4/5] f2fs: use the updated test_dummy_encryption helper functions
  2022-05-13 23:16 ` Eric Biggers
@ 2022-05-13 23:16   ` Eric Biggers
  -1 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, Jaegeuk Kim

From: Eric Biggers <ebiggers@google.com>

Switch f2fs over to the functions that are replacing
fscrypt_set_test_dummy_encryption().  Since f2fs hasn't been converted
to the new mount API yet, this doesn't really provide a benefit for
f2fs.  But it allows fscrypt_set_test_dummy_encryption() to be removed.

Also take the opportunity to eliminate an #ifdef.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/f2fs/super.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6f69491aa5731..c08cbe0dfcd85 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
 #include <linux/sched/mm.h>
 #include <linux/statfs.h>
 #include <linux/buffer_head.h>
@@ -492,9 +493,19 @@ static int f2fs_set_test_dummy_encryption(struct super_block *sb,
 					  bool is_remount)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(sb);
-#ifdef CONFIG_FS_ENCRYPTION
+	struct fs_parameter param = {
+		.type = fs_value_is_string,
+		.string = arg->from ? arg->from : "",
+	};
+	struct fscrypt_dummy_policy *policy =
+		&F2FS_OPTION(sbi).dummy_enc_policy;
 	int err;
 
+	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION)) {
+		f2fs_warn(sbi, "test_dummy_encryption option not supported");
+		return -EINVAL;
+	}
+
 	if (!f2fs_sb_has_encrypt(sbi)) {
 		f2fs_err(sbi, "Encrypt feature is off");
 		return -EINVAL;
@@ -506,12 +517,12 @@ static int f2fs_set_test_dummy_encryption(struct super_block *sb,
 	 * needed to allow it to be set or changed during remount.  We do allow
 	 * it to be specified during remount, but only if there is no change.
 	 */
-	if (is_remount && !F2FS_OPTION(sbi).dummy_enc_policy.policy) {
+	if (is_remount && !fscrypt_is_dummy_policy_set(policy)) {
 		f2fs_warn(sbi, "Can't set test_dummy_encryption on remount");
 		return -EINVAL;
 	}
-	err = fscrypt_set_test_dummy_encryption(
-		sb, arg->from, &F2FS_OPTION(sbi).dummy_enc_policy);
+
+	err = fscrypt_parse_test_dummy_encryption(&param, policy);
 	if (err) {
 		if (err == -EEXIST)
 			f2fs_warn(sbi,
@@ -524,12 +535,14 @@ static int f2fs_set_test_dummy_encryption(struct super_block *sb,
 				  opt, err);
 		return -EINVAL;
 	}
+	err = fscrypt_add_test_dummy_key(sb, policy);
+	if (err) {
+		f2fs_warn(sbi, "Error adding test dummy encryption key [%d]",
+			  err);
+		return err;
+	}
 	f2fs_warn(sbi, "Test dummy encryption mode enabled");
 	return 0;
-#else
-	f2fs_warn(sbi, "test_dummy_encryption option not supported");
-	return -EINVAL;
-#endif
 }
 
 #ifdef CONFIG_F2FS_FS_COMPRESSION
-- 
2.36.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 4/5] f2fs: use the updated test_dummy_encryption helper functions
@ 2022-05-13 23:16   ` Eric Biggers
  0 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Jeff Layton, Lukas Czerner, Theodore Ts'o, Jaegeuk Kim

From: Eric Biggers <ebiggers@google.com>

Switch f2fs over to the functions that are replacing
fscrypt_set_test_dummy_encryption().  Since f2fs hasn't been converted
to the new mount API yet, this doesn't really provide a benefit for
f2fs.  But it allows fscrypt_set_test_dummy_encryption() to be removed.

Also take the opportunity to eliminate an #ifdef.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/f2fs/super.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6f69491aa5731..c08cbe0dfcd85 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
 #include <linux/sched/mm.h>
 #include <linux/statfs.h>
 #include <linux/buffer_head.h>
@@ -492,9 +493,19 @@ static int f2fs_set_test_dummy_encryption(struct super_block *sb,
 					  bool is_remount)
 {
 	struct f2fs_sb_info *sbi = F2FS_SB(sb);
-#ifdef CONFIG_FS_ENCRYPTION
+	struct fs_parameter param = {
+		.type = fs_value_is_string,
+		.string = arg->from ? arg->from : "",
+	};
+	struct fscrypt_dummy_policy *policy =
+		&F2FS_OPTION(sbi).dummy_enc_policy;
 	int err;
 
+	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION)) {
+		f2fs_warn(sbi, "test_dummy_encryption option not supported");
+		return -EINVAL;
+	}
+
 	if (!f2fs_sb_has_encrypt(sbi)) {
 		f2fs_err(sbi, "Encrypt feature is off");
 		return -EINVAL;
@@ -506,12 +517,12 @@ static int f2fs_set_test_dummy_encryption(struct super_block *sb,
 	 * needed to allow it to be set or changed during remount.  We do allow
 	 * it to be specified during remount, but only if there is no change.
 	 */
-	if (is_remount && !F2FS_OPTION(sbi).dummy_enc_policy.policy) {
+	if (is_remount && !fscrypt_is_dummy_policy_set(policy)) {
 		f2fs_warn(sbi, "Can't set test_dummy_encryption on remount");
 		return -EINVAL;
 	}
-	err = fscrypt_set_test_dummy_encryption(
-		sb, arg->from, &F2FS_OPTION(sbi).dummy_enc_policy);
+
+	err = fscrypt_parse_test_dummy_encryption(&param, policy);
 	if (err) {
 		if (err == -EEXIST)
 			f2fs_warn(sbi,
@@ -524,12 +535,14 @@ static int f2fs_set_test_dummy_encryption(struct super_block *sb,
 				  opt, err);
 		return -EINVAL;
 	}
+	err = fscrypt_add_test_dummy_key(sb, policy);
+	if (err) {
+		f2fs_warn(sbi, "Error adding test dummy encryption key [%d]",
+			  err);
+		return err;
+	}
 	f2fs_warn(sbi, "Test dummy encryption mode enabled");
 	return 0;
-#else
-	f2fs_warn(sbi, "test_dummy_encryption option not supported");
-	return -EINVAL;
-#endif
 }
 
 #ifdef CONFIG_F2FS_FS_COMPRESSION
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [f2fs-dev] [PATCH v3 5/5] fscrypt: remove fscrypt_set_test_dummy_encryption()
  2022-05-13 23:16 ` Eric Biggers
@ 2022-05-13 23:16   ` Eric Biggers
  -1 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, Jaegeuk Kim

From: Eric Biggers <ebiggers@google.com>

Now that all its callers have been converted to
fscrypt_parse_test_dummy_encryption() and fscrypt_add_test_dummy_key()
instead, fscrypt_set_test_dummy_encryption() can be removed.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/policy.c      | 13 -------------
 include/linux/fscrypt.h |  2 --
 2 files changed, 15 deletions(-)

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 5f858cee1e3b0..d0a8921577def 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -802,19 +802,6 @@ bool fscrypt_dummy_policies_equal(const struct fscrypt_dummy_policy *p1,
 }
 EXPORT_SYMBOL_GPL(fscrypt_dummy_policies_equal);
 
-/* Deprecated, do not use */
-int fscrypt_set_test_dummy_encryption(struct super_block *sb, const char *arg,
-				      struct fscrypt_dummy_policy *dummy_policy)
-{
-	struct fs_parameter param = {
-		.type = fs_value_is_string,
-		.string = arg ? (char *)arg : "",
-	};
-	return fscrypt_parse_test_dummy_encryption(&param, dummy_policy) ?:
-		fscrypt_add_test_dummy_key(sb, dummy_policy);
-}
-EXPORT_SYMBOL_GPL(fscrypt_set_test_dummy_encryption);
-
 /**
  * fscrypt_show_test_dummy_encryption() - show '-o test_dummy_encryption'
  * @seq: the seq_file to print the option to
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 099b881e63e49..11db6d61d4244 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -284,8 +284,6 @@ int fscrypt_parse_test_dummy_encryption(const struct fs_parameter *param,
 				    struct fscrypt_dummy_policy *dummy_policy);
 bool fscrypt_dummy_policies_equal(const struct fscrypt_dummy_policy *p1,
 				  const struct fscrypt_dummy_policy *p2);
-int fscrypt_set_test_dummy_encryption(struct super_block *sb, const char *arg,
-				struct fscrypt_dummy_policy *dummy_policy);
 void fscrypt_show_test_dummy_encryption(struct seq_file *seq, char sep,
 					struct super_block *sb);
 static inline bool
-- 
2.36.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 5/5] fscrypt: remove fscrypt_set_test_dummy_encryption()
@ 2022-05-13 23:16   ` Eric Biggers
  0 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-05-13 23:16 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Jeff Layton, Lukas Czerner, Theodore Ts'o, Jaegeuk Kim

From: Eric Biggers <ebiggers@google.com>

Now that all its callers have been converted to
fscrypt_parse_test_dummy_encryption() and fscrypt_add_test_dummy_key()
instead, fscrypt_set_test_dummy_encryption() can be removed.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/policy.c      | 13 -------------
 include/linux/fscrypt.h |  2 --
 2 files changed, 15 deletions(-)

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 5f858cee1e3b0..d0a8921577def 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -802,19 +802,6 @@ bool fscrypt_dummy_policies_equal(const struct fscrypt_dummy_policy *p1,
 }
 EXPORT_SYMBOL_GPL(fscrypt_dummy_policies_equal);
 
-/* Deprecated, do not use */
-int fscrypt_set_test_dummy_encryption(struct super_block *sb, const char *arg,
-				      struct fscrypt_dummy_policy *dummy_policy)
-{
-	struct fs_parameter param = {
-		.type = fs_value_is_string,
-		.string = arg ? (char *)arg : "",
-	};
-	return fscrypt_parse_test_dummy_encryption(&param, dummy_policy) ?:
-		fscrypt_add_test_dummy_key(sb, dummy_policy);
-}
-EXPORT_SYMBOL_GPL(fscrypt_set_test_dummy_encryption);
-
 /**
  * fscrypt_show_test_dummy_encryption() - show '-o test_dummy_encryption'
  * @seq: the seq_file to print the option to
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 099b881e63e49..11db6d61d4244 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -284,8 +284,6 @@ int fscrypt_parse_test_dummy_encryption(const struct fs_parameter *param,
 				    struct fscrypt_dummy_policy *dummy_policy);
 bool fscrypt_dummy_policies_equal(const struct fscrypt_dummy_policy *p1,
 				  const struct fscrypt_dummy_policy *p2);
-int fscrypt_set_test_dummy_encryption(struct super_block *sb, const char *arg,
-				struct fscrypt_dummy_policy *dummy_policy);
 void fscrypt_show_test_dummy_encryption(struct seq_file *seq, char sep,
 					struct super_block *sb);
 static inline bool
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH v3 1/5] ext4: fix memory leak in parse_apply_sb_mount_options()
  2022-05-13 23:16   ` Eric Biggers
@ 2022-05-14 12:09     ` Ritesh Harjani
  -1 siblings, 0 replies; 22+ messages in thread
From: Ritesh Harjani @ 2022-05-14 12:09 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel, Lukas Czerner,
	Jeff Layton, Theodore Ts'o, stable, Jaegeuk Kim

On 22/05/13 04:16PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> If processing the on-disk mount options fails after any memory was
> allocated in the ext4_fs_context, e.g. s_qf_names, then this memory is
> leaked.  Fix this by calling ext4_fc_free() instead of kfree() directly.

Thanks for splitting the patch. It becomes an easy backport.

>
> Reproducer:
>
>     mkfs.ext4 -F /dev/vdc
>     tune2fs /dev/vdc -E mount_opts=usrjquota=file
>     echo clear > /sys/kernel/debug/kmemleak
>     mount /dev/vdc /vdc
>     echo scan > /sys/kernel/debug/kmemleak
>     sleep 5
>     echo scan > /sys/kernel/debug/kmemleak
>     cat /sys/kernel/debug/kmemleak

Tested this and as you mentioned this patch fixes the memory leak with
s_qf_names in note_qf_name().

tune2fs 1.46.5 (30-Dec-2021)
Setting extended default mount options to 'usrjquota=file'
unreferenced object 0xffff8881126b9a50 (size 8):
  comm "mount", pid 1475, jiffies 4294829180 (age 48.670s)
  hex dump (first 8 bytes):
    66 69 6c 65 00 6b 6b a5                          file.kk.
  backtrace:
    [<ffffffff8153b09d>] __kmalloc_track_caller+0x17d/0x2f0
    [<ffffffff8149b7e8>] kmemdup_nul+0x28/0x70
    [<ffffffff81753a75>] note_qf_name.isra.0+0x95/0x180
    [<ffffffff817548a8>] ext4_parse_param+0xd48/0x11c0
    [<ffffffff8175a131>] ext4_fill_super+0x1cc1/0x6260
    [<ffffffff8155edce>] get_tree_bdev+0x24e/0x3a0
    [<ffffffff81740355>] ext4_get_tree+0x15/0x20
    [<ffffffff8155d3a2>] vfs_get_tree+0x52/0x140
    [<ffffffff815a2048>] path_mount+0x3f8/0xf30
    [<ffffffff815a2c52>] do_mount+0xd2/0xf0
    [<ffffffff815a2e4a>] __x64_sys_mount+0xca/0x110
    [<ffffffff82e6674b>] do_syscall_64+0x3b/0x90
    [<ffffffff8300007c>] entry_SYSCALL_64_after_hwframe+0x44/0xae


Feel free to add by -

Tested-by: Ritesh Harjani <ritesh.list@gmail.com>

-ritesh

>
> Fixes: 7edfd85b1ffd ("ext4: Completely separate options parsing and sb setup")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/ext4/super.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 1466fbdbc8e34..60fa2f2623e07 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2625,8 +2625,10 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
>  	ret = ext4_apply_options(fc, sb);
>
>  out_free:
> -	kfree(s_ctx);
> -	kfree(fc);
> +	if (fc) {
> +		ext4_fc_free(fc);
> +		kfree(fc);
> +	}
>  	kfree(s_mount_opts);
>  	return ret;
>  }
> --
> 2.36.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH v3 1/5] ext4: fix memory leak in parse_apply_sb_mount_options()
@ 2022-05-14 12:09     ` Ritesh Harjani
  0 siblings, 0 replies; 22+ messages in thread
From: Ritesh Harjani @ 2022-05-14 12:09 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Theodore Ts'o, Jeff Layton, stable, linux-f2fs-devel,
	linux-fscrypt, Jaegeuk Kim, Lukas Czerner, linux-ext4

On 22/05/13 04:16PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> If processing the on-disk mount options fails after any memory was
> allocated in the ext4_fs_context, e.g. s_qf_names, then this memory is
> leaked.  Fix this by calling ext4_fc_free() instead of kfree() directly.

Thanks for splitting the patch. It becomes an easy backport.

>
> Reproducer:
>
>     mkfs.ext4 -F /dev/vdc
>     tune2fs /dev/vdc -E mount_opts=usrjquota=file
>     echo clear > /sys/kernel/debug/kmemleak
>     mount /dev/vdc /vdc
>     echo scan > /sys/kernel/debug/kmemleak
>     sleep 5
>     echo scan > /sys/kernel/debug/kmemleak
>     cat /sys/kernel/debug/kmemleak

Tested this and as you mentioned this patch fixes the memory leak with
s_qf_names in note_qf_name().

tune2fs 1.46.5 (30-Dec-2021)
Setting extended default mount options to 'usrjquota=file'
unreferenced object 0xffff8881126b9a50 (size 8):
  comm "mount", pid 1475, jiffies 4294829180 (age 48.670s)
  hex dump (first 8 bytes):
    66 69 6c 65 00 6b 6b a5                          file.kk.
  backtrace:
    [<ffffffff8153b09d>] __kmalloc_track_caller+0x17d/0x2f0
    [<ffffffff8149b7e8>] kmemdup_nul+0x28/0x70
    [<ffffffff81753a75>] note_qf_name.isra.0+0x95/0x180
    [<ffffffff817548a8>] ext4_parse_param+0xd48/0x11c0
    [<ffffffff8175a131>] ext4_fill_super+0x1cc1/0x6260
    [<ffffffff8155edce>] get_tree_bdev+0x24e/0x3a0
    [<ffffffff81740355>] ext4_get_tree+0x15/0x20
    [<ffffffff8155d3a2>] vfs_get_tree+0x52/0x140
    [<ffffffff815a2048>] path_mount+0x3f8/0xf30
    [<ffffffff815a2c52>] do_mount+0xd2/0xf0
    [<ffffffff815a2e4a>] __x64_sys_mount+0xca/0x110
    [<ffffffff82e6674b>] do_syscall_64+0x3b/0x90
    [<ffffffff8300007c>] entry_SYSCALL_64_after_hwframe+0x44/0xae


Feel free to add by -

Tested-by: Ritesh Harjani <ritesh.list@gmail.com>

-ritesh

>
> Fixes: 7edfd85b1ffd ("ext4: Completely separate options parsing and sb setup")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/ext4/super.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 1466fbdbc8e34..60fa2f2623e07 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2625,8 +2625,10 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
>  	ret = ext4_apply_options(fc, sb);
>
>  out_free:
> -	kfree(s_ctx);
> -	kfree(fc);
> +	if (fc) {
> +		ext4_fc_free(fc);
> +		kfree(fc);
> +	}
>  	kfree(s_mount_opts);
>  	return ret;
>  }
> --
> 2.36.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/5] ext4: fix memory leak in parse_apply_sb_mount_options()
  2022-05-13 23:16   ` Eric Biggers
@ 2022-05-19  2:10     ` Theodore Ts'o
  -1 siblings, 0 replies; 22+ messages in thread
From: Theodore Ts'o @ 2022-05-19  2:10 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel, Jeff Layton,
	Lukas Czerner, Jaegeuk Kim, stable

On Fri, May 13, 2022 at 04:16:01PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> If processing the on-disk mount options fails after any memory was
> allocated in the ext4_fs_context, e.g. s_qf_names, then this memory is
> leaked.  Fix this by calling ext4_fc_free() instead of kfree() directly.
> 
> Reproducer:
> 
>     mkfs.ext4 -F /dev/vdc
>     tune2fs /dev/vdc -E mount_opts=usrjquota=file
>     echo clear > /sys/kernel/debug/kmemleak
>     mount /dev/vdc /vdc
>     echo scan > /sys/kernel/debug/kmemleak
>     sleep 5
>     echo scan > /sys/kernel/debug/kmemleak
>     cat /sys/kernel/debug/kmemleak
> 
> Fixes: 7edfd85b1ffd ("ext4: Completely separate options parsing and sb setup")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied.

					- Ted

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH v3 1/5] ext4: fix memory leak in parse_apply_sb_mount_options()
@ 2022-05-19  2:10     ` Theodore Ts'o
  0 siblings, 0 replies; 22+ messages in thread
From: Theodore Ts'o @ 2022-05-19  2:10 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Jeff Layton, stable, linux-f2fs-devel, linux-fscrypt,
	Lukas Czerner, Jaegeuk Kim, linux-ext4

On Fri, May 13, 2022 at 04:16:01PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> If processing the on-disk mount options fails after any memory was
> allocated in the ext4_fs_context, e.g. s_qf_names, then this memory is
> leaked.  Fix this by calling ext4_fc_free() instead of kfree() directly.
> 
> Reproducer:
> 
>     mkfs.ext4 -F /dev/vdc
>     tune2fs /dev/vdc -E mount_opts=usrjquota=file
>     echo clear > /sys/kernel/debug/kmemleak
>     mount /dev/vdc /vdc
>     echo scan > /sys/kernel/debug/kmemleak
>     sleep 5
>     echo scan > /sys/kernel/debug/kmemleak
>     cat /sys/kernel/debug/kmemleak
> 
> Fixes: 7edfd85b1ffd ("ext4: Completely separate options parsing and sb setup")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied.

					- Ted


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 2/5] ext4: only allow test_dummy_encryption when supported
  2022-05-13 23:16   ` Eric Biggers
@ 2022-05-19  2:11     ` Theodore Ts'o
  -1 siblings, 0 replies; 22+ messages in thread
From: Theodore Ts'o @ 2022-05-19  2:11 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel, Jeff Layton,
	Lukas Czerner, Jaegeuk Kim

On Fri, May 13, 2022 at 04:16:02PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Make the test_dummy_encryption mount option require that the encrypt
> feature flag be already enabled on the filesystem, rather than
> automatically enabling it.  Practically, this means that "-O encrypt"
> will need to be included in MKFS_OPTIONS when running xfstests with the
> test_dummy_encryption mount option.  (ext4/053 also needs an update.)
> 
> Moreover, as long as the preconditions for test_dummy_encryption are
> being tightened anyway, take the opportunity to start rejecting it when
> !CONFIG_FS_ENCRYPTION rather than ignoring it.
> 
> The motivation for requiring the encrypt feature flag is that:
> 
> - Having the filesystem auto-enable feature flags is problematic, as it
>   bypasses the usual sanity checks.  The specific issue which came up
>   recently is that in kernel versions where ext4 supports casefold but
>   not encrypt+casefold (v5.1 through v5.10), the kernel will happily add
>   the encrypt flag to a filesystem that has the casefold flag, making it
>   unmountable -- but only for subsequent mounts, not the initial one.
>   This confused the casefold support detection in xfstests, causing
>   generic/556 to fail rather than be skipped.
> 
> - The xfstests-bld test runners (kvm-xfstests et al.) already use the
>   required mkfs flag, so they will not be affected by this change.  Only
>   users of test_dummy_encryption alone will be affected.  But, this
>   option has always been for testing only, so it should be fine to
>   require that the few users of this option update their test scripts.
> 
> - f2fs already requires it (for its equivalent feature flag).
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied.

					- Ted

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH v3 2/5] ext4: only allow test_dummy_encryption when supported
@ 2022-05-19  2:11     ` Theodore Ts'o
  0 siblings, 0 replies; 22+ messages in thread
From: Theodore Ts'o @ 2022-05-19  2:11 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Jeff Layton, linux-f2fs-devel, linux-fscrypt, Lukas Czerner,
	Jaegeuk Kim, linux-ext4

On Fri, May 13, 2022 at 04:16:02PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Make the test_dummy_encryption mount option require that the encrypt
> feature flag be already enabled on the filesystem, rather than
> automatically enabling it.  Practically, this means that "-O encrypt"
> will need to be included in MKFS_OPTIONS when running xfstests with the
> test_dummy_encryption mount option.  (ext4/053 also needs an update.)
> 
> Moreover, as long as the preconditions for test_dummy_encryption are
> being tightened anyway, take the opportunity to start rejecting it when
> !CONFIG_FS_ENCRYPTION rather than ignoring it.
> 
> The motivation for requiring the encrypt feature flag is that:
> 
> - Having the filesystem auto-enable feature flags is problematic, as it
>   bypasses the usual sanity checks.  The specific issue which came up
>   recently is that in kernel versions where ext4 supports casefold but
>   not encrypt+casefold (v5.1 through v5.10), the kernel will happily add
>   the encrypt flag to a filesystem that has the casefold flag, making it
>   unmountable -- but only for subsequent mounts, not the initial one.
>   This confused the casefold support detection in xfstests, causing
>   generic/556 to fail rather than be skipped.
> 
> - The xfstests-bld test runners (kvm-xfstests et al.) already use the
>   required mkfs flag, so they will not be affected by this change.  Only
>   users of test_dummy_encryption alone will be affected.  But, this
>   option has always been for testing only, so it should be fine to
>   require that the few users of this option update their test scripts.
> 
> - f2fs already requires it (for its equivalent feature flag).
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied.

					- Ted


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH v3 4/5] f2fs: use the updated test_dummy_encryption helper functions
  2022-05-13 23:16   ` Eric Biggers
@ 2022-05-19 11:21     ` Chao Yu
  -1 siblings, 0 replies; 22+ messages in thread
From: Chao Yu @ 2022-05-19 11:21 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, Jaegeuk Kim

On 2022/5/14 7:16, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Switch f2fs over to the functions that are replacing
> fscrypt_set_test_dummy_encryption().  Since f2fs hasn't been converted
> to the new mount API yet, this doesn't really provide a benefit for
> f2fs.  But it allows fscrypt_set_test_dummy_encryption() to be removed.
> 
> Also take the opportunity to eliminate an #ifdef.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH v3 4/5] f2fs: use the updated test_dummy_encryption helper functions
@ 2022-05-19 11:21     ` Chao Yu
  0 siblings, 0 replies; 22+ messages in thread
From: Chao Yu @ 2022-05-19 11:21 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, Jaegeuk Kim

On 2022/5/14 7:16, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Switch f2fs over to the functions that are replacing
> fscrypt_set_test_dummy_encryption().  Since f2fs hasn't been converted
> to the new mount API yet, this doesn't really provide a benefit for
> f2fs.  But it allows fscrypt_set_test_dummy_encryption() to be removed.
> 
> Also take the opportunity to eliminate an #ifdef.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [f2fs-dev] [PATCH v3 5/5] fscrypt: remove fscrypt_set_test_dummy_encryption()
  2022-05-13 23:16   ` Eric Biggers
@ 2022-08-15 18:48     ` Eric Biggers
  -1 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-08-15 18:48 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, Jaegeuk Kim

On Fri, May 13, 2022 at 04:16:05PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Now that all its callers have been converted to
> fscrypt_parse_test_dummy_encryption() and fscrypt_add_test_dummy_key()
> instead, fscrypt_set_test_dummy_encryption() can be removed.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/crypto/policy.c      | 13 -------------
>  include/linux/fscrypt.h |  2 --
>  2 files changed, 15 deletions(-)

Now that all its prerequisites are upstream, I've applied this patch to
fscrypt.git#master for 6.1.

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 5/5] fscrypt: remove fscrypt_set_test_dummy_encryption()
@ 2022-08-15 18:48     ` Eric Biggers
  0 siblings, 0 replies; 22+ messages in thread
From: Eric Biggers @ 2022-08-15 18:48 UTC (permalink / raw)
  To: linux-fscrypt, linux-ext4, linux-f2fs-devel
  Cc: Lukas Czerner, Jeff Layton, Theodore Ts'o, Jaegeuk Kim

On Fri, May 13, 2022 at 04:16:05PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Now that all its callers have been converted to
> fscrypt_parse_test_dummy_encryption() and fscrypt_add_test_dummy_key()
> instead, fscrypt_set_test_dummy_encryption() can be removed.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/crypto/policy.c      | 13 -------------
>  include/linux/fscrypt.h |  2 --
>  2 files changed, 15 deletions(-)

Now that all its prerequisites are upstream, I've applied this patch to
fscrypt.git#master for 6.1.

- Eric

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2022-08-15 19:42 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13 23:16 [f2fs-dev] [PATCH v3 0/5] test_dummy_encryption fixes and cleanups Eric Biggers
2022-05-13 23:16 ` Eric Biggers
2022-05-13 23:16 ` [f2fs-dev] [PATCH v3 1/5] ext4: fix memory leak in parse_apply_sb_mount_options() Eric Biggers
2022-05-13 23:16   ` Eric Biggers
2022-05-14 12:09   ` [f2fs-dev] " Ritesh Harjani
2022-05-14 12:09     ` Ritesh Harjani
2022-05-19  2:10   ` Theodore Ts'o
2022-05-19  2:10     ` [f2fs-dev] " Theodore Ts'o
2022-05-13 23:16 ` [f2fs-dev] [PATCH v3 2/5] ext4: only allow test_dummy_encryption when supported Eric Biggers
2022-05-13 23:16   ` Eric Biggers
2022-05-19  2:11   ` Theodore Ts'o
2022-05-19  2:11     ` [f2fs-dev] " Theodore Ts'o
2022-05-13 23:16 ` [f2fs-dev] [PATCH v3 3/5] ext4: fix up test_dummy_encryption handling for new mount API Eric Biggers
2022-05-13 23:16   ` Eric Biggers
2022-05-13 23:16 ` [f2fs-dev] [PATCH v3 4/5] f2fs: use the updated test_dummy_encryption helper functions Eric Biggers
2022-05-13 23:16   ` Eric Biggers
2022-05-19 11:21   ` [f2fs-dev] " Chao Yu
2022-05-19 11:21     ` Chao Yu
2022-05-13 23:16 ` [f2fs-dev] [PATCH v3 5/5] fscrypt: remove fscrypt_set_test_dummy_encryption() Eric Biggers
2022-05-13 23:16   ` Eric Biggers
2022-08-15 18:48   ` [f2fs-dev] " Eric Biggers
2022-08-15 18:48     ` Eric Biggers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.