[Please use reply-all, not reply!] On Thu, May 26, 2022 at 10:55:07AM +0200, Lukas Czerner wrote: > On Wed, May 25, 2022 at 09:04:12PM -0700, Eric Biggers wrote: > > From: Eric Biggers <ebiggers@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > --- > > Hi, thanks for the patch it looks good, exept maybe small consideration > below... > > > @@ -2673,11 +2656,11 @@ static int ext4_check_quota_consistency(struct fs_context *fc, > > static int ext4_check_test_dummy_encryption(const struct fs_context *fc, > > struct super_block *sb) > > { > > -#ifdef CONFIG_FS_ENCRYPTION > > const struct ext4_fs_context *ctx = fc->fs_private; > > const struct ext4_sb_info *sbi = EXT4_SB(sb); > > + int err; > > > > - if (!(ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION)) > > + if (!fscrypt_is_dummy_policy_set(&ctx->dummy_enc_policy)) > > how about > > if (!fscrypt_is_dummy_policy_set(&ctx->dummy_enc_policy) || > fscrypt_dummy_policies_equal(&sbi->s_dummy_enc_policy, > &ctx->dummy_enc_policy)) > return 0; > > and remove the two fscrypt_dummy_policies_equal checks below? > > But regardless whether you want to change it, you can add > > Reviewed-by: Lukas Czerner <lczerner@xxxxxxxxxx> > That would work, but I think the code I've proposed makes it a little more explicit what's going on. - Eric
![]() |