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

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] [day] [month] [year] [list]

Message-ID: <aA+bsw09PHTQWUXK@debian>
Date: Mon, 28 Apr 2025 23:16:03 +0800
From: Gao Xiang <xiang@...nel.org>
To: Hongbo Li <lihongbo22@...wei.com>
Cc: xiang@...nel.org, chao@...nel.org, zbestahu@...il.com,
	jefflexu@...ux.alibaba.com, dhavale@...gle.com,
	linux-erofs@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] erofs: reject unknown option if it is not supported

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@...wei.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