linux-ext4 - [PATCH v2 2/3] ext4: fix the parsing of empty string mount parameters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240307160225.23841-3-lhenriques@suse.de>
Date: Thu,  7 Mar 2024 16:02:24 +0000
From: Luis Henriques <lhenriques@...e.de>
To: Theodore Ts'o <tytso@....edu>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Christian Brauner <brauner@...nel.org>,
	Jan Kara <jack@...e.cz>,
	Miklos Szeredi <miklos@...redi.hu>,
	Amir Goldstein <amir73il@...il.com>
Cc: linux-ext4@...r.kernel.org,
	linux-fsdevel@...r.kernel.org,
	linux-unionfs@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Luis Henriques <lhenriques@...e.de>
Subject: [PATCH v2 2/3] ext4: fix the parsing of empty string mount parameters

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@...nel.org>
Signed-off-by: Luis Henriques <lhenriques@...e.de>
---
 fs/ext4/super.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

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) {