Received-SPF: pass (google.com: domain of linux-ext4+bounces-1562-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Luis Henriques <lhenriques@suse.de>
To: Theodore Ts'o <tytso@mit.edu>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	Jan Kara <jack@suse.cz>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Amir Goldstein <amir73il@gmail.com>
Cc: linux-ext4@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	linux-unionfs@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Luis Henriques <lhenriques@suse.de>
Subject: [PATCH v2 3/3] ovl: fix the parsing of empty string mount parameters
Date: Thu,  7 Mar 2024 16:02:25 +0000
Message-ID: <20240307160225.23841-4-lhenriques@suse.de>
In-Reply-To: <20240307160225.23841-1-lhenriques@suse.de>
References: <20240307160225.23841-1-lhenriques@suse.de>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

This patch fixes the usage of mount parameters that are defined as strings
but which can be empty.  Currently, only 'lowerdir' parameter is in this
situation for overlayfs.  But since userspace can pass it in as 'flag'
type (when it doesn'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.

Suggested-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Luis Henriques <lhenriques@suse.de>
---
 fs/overlayfs/params.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
index 112b4b12f825..6eb163ca4b92 100644
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -139,12 +139,8 @@ static int ovl_verity_mode_def(void)
 	return OVL_VERITY_OFF;
 }
 
-#define fsparam_string_empty(NAME, OPT) \
-	__fsparam(fs_param_is_string, NAME, OPT, fs_param_can_be_empty, NULL)
-
-
 const struct fs_parameter_spec ovl_parameter_spec[] = {
-	fsparam_string_empty("lowerdir",    Opt_lowerdir),
+	fsparam_string_or_flag("lowerdir",    Opt_lowerdir),
 	fsparam_string("lowerdir+",         Opt_lowerdir_add),
 	fsparam_string("datadir+",          Opt_datadir_add),
 	fsparam_string("upperdir",          Opt_upperdir),
@@ -424,12 +420,13 @@ static void ovl_reset_lowerdirs(struct ovl_fs_context *ctx)
  *     "/data1" and "/data2" as data lower layers. Any existing lower
  *     layers are replaced.
  */
-static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
+static int ovl_parse_param_lowerdir(struct fs_context *fc, struct fs_parameter *param)
 {
 	int err;
 	struct ovl_fs_context *ctx = fc->fs_private;
 	struct ovl_fs_context_layer *l;
 	char *dup = NULL, *iter;
+	const char *name = param->string;
 	ssize_t nr_lower, nr;
 	bool data_layer = false;
 
@@ -441,7 +438,7 @@ static int ovl_parse_param_lowerdir(const char *name, struct fs_context *fc)
 	/* drop all existing lower layers */
 	ovl_reset_lowerdirs(ctx);
 
-	if (!*name)
+	if ((param->type == fs_value_is_flag) || (!*name))
 		return 0;
 
 	if (*name == ':') {
@@ -572,7 +569,7 @@ static int ovl_parse_param(struct fs_context *fc, struct fs_parameter *param)
 
 	switch (opt) {
 	case Opt_lowerdir:
-		err = ovl_parse_param_lowerdir(param->string, fc);
+		err = ovl_parse_param_lowerdir(fc, param);
 		break;
 	case Opt_lowerdir_add:
 	case Opt_datadir_add: