Re: [PATCH v2] erofs-utils: mount: fix flag-clearing bug and missing error check in parse_flagopts

[Gao Xiang <hsiangkao@linux.alibaba.com>]

On 2026/3/1 20:12, Yifan Zhao wrote:
> The MS_* constants in glibc's <sys/mount.h> are defined as members of
> an anonymous enum whose underlying type is unsigned int (because the
> last member, MS_NOUSER, is initialised with '1U << 31').  Therefore
> ~MS_RDONLY, ~MS_NOSUID, etc. are unsigned int values that, when stored

The problem seems glibc refines these macros as enum.

> into a 'long flags' field, undergo zero-extension, not sign-extension.
> As a result every 'clearing' entry (rw, suid, dev, exec, async, atime,
> diratime, norelatime, loud) produced a positive long, so the
> opts[i].flags < 0 guard in erofsmount_parse_flagopts() was never true
> and the corresponding flags were set rather than cleared.
> 
> Fix by casting the operand to long before applying bitwise-NOT,
> ensuring the result is a negative long with the correct bit pattern.
> 
> Also add the missing return-value check for erofsmount_parse_flagopts()
> in the '-o' option handler.
> 
> Reported-By: rorosen <76747196+rorosen@users.noreply.github.com>

It seems the real email is:
Reported-by: Robert Rose <robert.rose@mailbox.org>

Otherwise it looks good to me, will apply:
Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

> Closes: https://github.com/NixOS/nixpkgs/issues/494653
> Signed-off-By: Yifan Zhao <yifan.yfzhao@foxmail.com>
> ---
>   mount/main.c | 27 +++++++++++++++------------
>   1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/mount/main.c b/mount/main.c
> index b04be5d..7c557bd 100644
> --- a/mount/main.c
> +++ b/mount/main.c
> @@ -203,15 +203,15 @@ static long erofsmount_parse_flagopts(char *s, long flags, char **more)
>   	} opts[] = {
>   		{"defaults", 0}, {"quiet", 0}, // NOPs
>   		{"user", 0}, {"nouser", 0}, // checked in fstab, ignored in -o
> -		{"ro", MS_RDONLY}, {"rw", ~MS_RDONLY},
> -		{"nosuid", MS_NOSUID}, {"suid", ~MS_NOSUID},
> -		{"nodev", MS_NODEV}, {"dev", ~MS_NODEV},
> -		{"noexec", MS_NOEXEC}, {"exec", ~MS_NOEXEC},
> -		{"sync", MS_SYNCHRONOUS}, {"async", ~MS_SYNCHRONOUS},
> -		{"noatime", MS_NOATIME}, {"atime", ~MS_NOATIME},
> -		{"norelatime", ~MS_RELATIME}, {"relatime", MS_RELATIME},
> -		{"nodiratime", MS_NODIRATIME}, {"diratime", ~MS_NODIRATIME},
> -		{"loud", ~MS_SILENT},
> +		{"ro", MS_RDONLY}, {"rw", ~(long)MS_RDONLY},
> +		{"nosuid", MS_NOSUID}, {"suid", ~(long)MS_NOSUID},
> +		{"nodev", MS_NODEV}, {"dev", ~(long)MS_NODEV},
> +		{"noexec", MS_NOEXEC}, {"exec", ~(long)MS_NOEXEC},
> +		{"sync", MS_SYNCHRONOUS}, {"async", ~(long)MS_SYNCHRONOUS},
> +		{"noatime", MS_NOATIME}, {"atime", ~(long)MS_NOATIME},
> +		{"norelatime", ~(long)MS_RELATIME}, {"relatime", MS_RELATIME},
> +		{"nodiratime", MS_NODIRATIME}, {"diratime", ~(long)MS_NODIRATIME},
> +		{"loud", ~(long)MS_SILENT},
>   		{"remount", MS_REMOUNT}, {"move", MS_MOVE},
>   		// mand dirsync rec iversion strictatime
>   	};
> @@ -281,6 +281,7 @@ static int erofsmount_parse_options(int argc, char **argv)
>   		{0, 0, 0, 0},
>   	};
>   	char *dot;
> +	long ret;
>   	int opt;
>   	int i;
>   
> @@ -305,9 +306,11 @@ static int erofsmount_parse_options(int argc, char **argv)
>   			break;
>   		case 'o':
>   			mountcfg.full_options = optarg;
> -			mountcfg.flags =
> -				erofsmount_parse_flagopts(optarg, mountcfg.flags,
> -							  &mountcfg.options);
> +			ret = erofsmount_parse_flagopts(optarg, mountcfg.flags,
> +								   &mountcfg.options);
> +			if (ret < 0)
> +				return (int)ret;
> +			mountcfg.flags = ret;
>   			break;
>   		case 't':
>   			dot = strchr(optarg, '.');