| Message ID | 20240307160225.23841-3-lhenriques@suse.de |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | fs_parser: handle parameters that can be empty and don't have a value | expand |
hi, Luis Henriques,
we noticed in
https://lore.kernel.org/all/20240307160225.23841-1-lhenriques@suse.de/
it was mentioned this patch is for "ext4/053 fstest failure",
however, in our tests, 053 can pass on parent but fail on this commit.
12dbddcebcb8e fs_parser: add helper to define parameters with string and flag types
2de45c422fe6a ext4: fix the parsing of empty string mount parameters
12dbddcebcb8e3e1 2de45c422fe6ae4f64d35df99cd
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 xfstests.ext4.053.fail
not sure if there is a xfstests patch works with this patch? Thanks!
below report just FYI.
Hello,
kernel test robot noticed "xfstests.ext4.053.fail" on:
commit: 2de45c422fe6ae4f64d35df99cdaf2c6fee2a5ac ("[PATCH v2 2/3] ext4: fix the parsing of empty string mount parameters")
url: https://github.com/intel-lab-lkp/linux/commits/Luis-Henriques/fs_parser-add-helper-to-define-parameters-with-string-and-flag-types/20240308-104759
base: https://git.kernel.org/cgit/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/all/20240307160225.23841-3-lhenriques@suse.de/
patch subject: [PATCH v2 2/3] ext4: fix the parsing of empty string mount parameters
in testcase: xfstests
version: xfstests-x86_64-9b6df9a0-1_20240318
with following parameters:
disk: 4HDD
fs: ext4
test: ext4-053
compiler: gcc-12
test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202403251019.50cab6c8-oliver.sang@intel.com
2024-03-22 09:38:10 export TEST_DIR=/fs/sdb1
2024-03-22 09:38:10 export TEST_DEV=/dev/sdb1
2024-03-22 09:38:10 export FSTYP=ext4
2024-03-22 09:38:10 export SCRATCH_MNT=/fs/scratch
2024-03-22 09:38:10 mkdir /fs/scratch -p
2024-03-22 09:38:10 export SCRATCH_DEV=/dev/sdb4
2024-03-22 09:38:10 echo ext4/053
2024-03-22 09:38:10 ./check -E tests/exclude/ext4 ext4/053
FSTYP -- ext4
PLATFORM -- Linux/x86_64 lkp-skl-d05 6.8.0-rc3-00019-g2de45c422fe6 #1 SMP PREEMPT_DYNAMIC Sat Mar 16 03:20:13 CST 2024
MKFS_OPTIONS -- -F /dev/sdb4
MOUNT_OPTIONS -- -o acl,user_xattr /dev/sdb4 /fs/scratch
ext4/053 [failed, exit status 1]- output mismatch (see /lkp/benchmarks/xfstests/results//ext4/053.out.bad)
--- tests/ext4/053.out 2024-03-18 16:30:59.000000000 +0000
+++ /lkp/benchmarks/xfstests/results//ext4/053.out.bad 2024-03-22 09:44:06.379217460 +0000
@@ -1,2 +1,3 @@
QA output created by 053
Silence is golden.
+SHOULD FAIL mounting ext4 "test_dummy_encryption=" (mount unexpectedly succeeded) FAILED
...
(Run 'diff -u /lkp/benchmarks/xfstests/tests/ext4/053.out /lkp/benchmarks/xfstests/results//ext4/053.out.bad' to see the entire diff)
Ran: ext4/053
Failures: ext4/053
Failed 1 of 1 tests
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240325/202403251019.50cab6c8-oliver.sang@intel.com
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 0f931d0c227d..5a2f178f8fe9 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1724,10 +1724,6 @@ static const struct constant_table ext4_param_dax[] = { {} }; -/* String parameter that allows empty argument */ -#define fsparam_string_empty(NAME, OPT) \ - __fsparam(fs_param_is_string, NAME, OPT, fs_param_can_be_empty, NULL) - /* * Mount option specification * We don't use fsparam_flag_no because of the way we set the @@ -1768,9 +1764,9 @@ static const struct fs_parameter_spec ext4_param_specs[] = { fsparam_enum ("data", Opt_data, ext4_param_data), fsparam_enum ("data_err", Opt_data_err, ext4_param_data_err), - fsparam_string_empty + fsparam_string_or_flag ("usrjquota", Opt_usrjquota), - fsparam_string_empty + fsparam_string_or_flag ("grpjquota", Opt_grpjquota), fsparam_enum ("jqfmt", Opt_jqfmt, ext4_param_jqfmt), fsparam_flag ("grpquota", Opt_grpquota), @@ -1814,9 +1810,8 @@ static const struct fs_parameter_spec ext4_param_specs[] = { fsparam_u32 ("fc_debug_max_replay", Opt_fc_debug_max_replay), #endif fsparam_u32 ("max_dir_size_kb", Opt_max_dir_size_kb), - fsparam_flag ("test_dummy_encryption", - Opt_test_dummy_encryption), - fsparam_string ("test_dummy_encryption", + fsparam_string_or_flag + ("test_dummy_encryption", Opt_test_dummy_encryption), fsparam_flag ("inlinecrypt", Opt_inlinecrypt), fsparam_flag ("nombcache", Opt_nombcache), @@ -2183,15 +2178,15 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param) switch (token) { #ifdef CONFIG_QUOTA case Opt_usrjquota: - if (!*param->string) - return unnote_qf_name(fc, USRQUOTA); - else + if ((param->type == fs_value_is_string) && + (*param->string)) return note_qf_name(fc, USRQUOTA, param); + return unnote_qf_name(fc, USRQUOTA); case Opt_grpjquota: - if (!*param->string) - return unnote_qf_name(fc, GRPQUOTA); - else + if ((param->type == fs_value_is_string) && + (*param->string)) return note_qf_name(fc, GRPQUOTA, param); + return unnote_qf_name(fc, GRPQUOTA); #endif case Opt_sb: if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
This patch fixes the usage of mount parameters that are defined as strings but which can be empty. Currently, only 'usrjquota' and 'grpjquota' parameters are in this situation for ext4. But since userspace can pass them in as 'flag' types (when they don't have a value), the parsing will fail because a 'string' type is assumed. This issue is fixed by using the new helper fsparam_string_or_flag() and by also taking the parameter type into account. While there, also remove the now unused fsparam_string_empty() macro and change the 'test_dummy_encryption' parameter to also use the new fs_parser helper. Suggested-by: Christian Brauner <brauner@kernel.org> Signed-off-by: Luis Henriques <lhenriques@suse.de> --- fs/ext4/super.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)