[PATCH] erofs: reject unknown option if it is not supported

[PATCH] erofs: reject unknown option if it is not supported

[Hongbo Li]

Some options are supported depending on different compiling config,
and these option will not fail during mount if they are not
supported. This is very weird, so we can reject them if they are
not supported.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 fs/erofs/super.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index cadec6b1b554..c1c350c6fbf4 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -374,16 +374,26 @@ static const struct constant_table erofs_dax_param_enums[] = {
 };
 
 static const struct fs_parameter_spec erofs_fs_parameters[] = {
+#ifdef CONFIG_EROFS_FS_XATTR
 	fsparam_flag_no("user_xattr",	Opt_user_xattr),
+#endif
+#ifdef CONFIG_EROFS_FS_POSIX_ACL
 	fsparam_flag_no("acl",		Opt_acl),
+#endif
+#ifdef CONFIG_EROFS_FS_ZIP
 	fsparam_enum("cache_strategy",	Opt_cache_strategy,
 		     erofs_param_cache_strategy),
+#endif
 	fsparam_flag("dax",             Opt_dax),
 	fsparam_enum("dax",		Opt_dax_enum, erofs_dax_param_enums),
 	fsparam_string("device",	Opt_device),
+#ifdef CONFIG_EROFS_FS_ONDEMAND
 	fsparam_string("fsid",		Opt_fsid),
 	fsparam_string("domain_id",	Opt_domain_id),
+#endif
+#ifdef CONFIG_EROFS_FS_BACKED_BY_FILE
 	fsparam_flag_no("directio",	Opt_directio),
+#endif
 	{}
 };
 
@@ -424,33 +434,27 @@ static int erofs_fc_parse_param(struct fs_context *fc,
 		return opt;
 
 	switch (opt) {
-	case Opt_user_xattr:
 #ifdef CONFIG_EROFS_FS_XATTR
+	case Opt_user_xattr:
 		if (result.boolean)
 			set_opt(&sbi->opt, XATTR_USER);
 		else
 			clear_opt(&sbi->opt, XATTR_USER);
-#else
-		errorfc(fc, "{,no}user_xattr options not supported");
-#endif
 		break;
-	case Opt_acl:
+#endif
 #ifdef CONFIG_EROFS_FS_POSIX_ACL
+	case Opt_acl:
 		if (result.boolean)
 			set_opt(&sbi->opt, POSIX_ACL);
 		else
 			clear_opt(&sbi->opt, POSIX_ACL);
-#else
-		errorfc(fc, "{,no}acl options not supported");
-#endif
 		break;
-	case Opt_cache_strategy:
+#endif
 #ifdef CONFIG_EROFS_FS_ZIP
+	case Opt_cache_strategy:
 		sbi->opt.cache_strategy = result.uint_32;
-#else
-		errorfc(fc, "compression not supported, cache_strategy ignored");
-#endif
 		break;
+#endif
 	case Opt_dax:
 		if (!erofs_fc_set_dax_mode(fc, EROFS_MOUNT_DAX_ALWAYS))
 			return -EINVAL;
@@ -491,22 +495,15 @@ static int erofs_fc_parse_param(struct fs_context *fc,
 		if (!sbi->domain_id)
 			return -ENOMEM;
 		break;
-#else
-	case Opt_fsid:
-	case Opt_domain_id:
-		errorfc(fc, "%s option not supported", erofs_fs_parameters[opt].name);
-		break;
 #endif
-	case Opt_directio:
 #ifdef CONFIG_EROFS_FS_BACKED_BY_FILE
+	case Opt_directio:
 		if (result.boolean)
 			set_opt(&sbi->opt, DIRECT_IO);
 		else
 			clear_opt(&sbi->opt, DIRECT_IO);
-#else
-		errorfc(fc, "%s option not supported", erofs_fs_parameters[opt].name);
-#endif
 		break;
+#endif
 	}
 	return 0;
 }
-- 
2.22.0

Re: [PATCH] erofs: reject unknown option if it is not supported

[Gao Xiang]

On Mon, Apr 28, 2025 at 02:25:45PM +0000, Hongbo Li wrote:
> Some options are supported depending on different compiling config,
> and these option will not fail during mount if they are not
> supported. This is very weird, so we can reject them if they are
> not supported.
> 

If it's an invalid option, we should reject it immediately.

But for unsupported options, I don't think we always error
out. e.g. for some options like (acl, noacl) ext4 will just
ignore if ACL is unsupported.

I think EROFS should follows that, otherwise users might use
"noacl" to disable ACL explicitly, but it will fail unexpectedly
if unsupported.

But I agree that for "fsid", "domain_id" and "directio", we
could error out instead.

> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  fs/erofs/super.c | 39 ++++++++++++++++++---------------------
>  1 file changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index cadec6b1b554..c1c350c6fbf4 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -374,16 +374,26 @@ static const struct constant_table erofs_dax_param_enums[] = {
>  };
>  
>  static const struct fs_parameter_spec erofs_fs_parameters[] = {
> +#ifdef CONFIG_EROFS_FS_XATTR
>  	fsparam_flag_no("user_xattr",	Opt_user_xattr),
> +#endif

Another thing is that I'm not sure if "user_xattr" option is really
needed, we might just kill this option since all recent fses don't
have such configuration and user_xattrs should be supported by default.

Thanks,
Gao Xiang