[PATCH v2] erofs-utils: mount: fix flag-clearing bug and missing error check in parse_flagopts
Gao Xiang
hsiangkao at linux.alibaba.com
Mon Mar 2 12:58:12 AEDT 2026
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 at users.noreply.github.com>
It seems the real email is:
Reported-by: Robert Rose <robert.rose at mailbox.org>
Otherwise it looks good to me, will apply:
Reviewed-by: Gao Xiang <hsiangkao at linux.alibaba.com>
Thanks,
Gao Xiang
> Closes: https://github.com/NixOS/nixpkgs/issues/494653
> Signed-off-By: Yifan Zhao <yifan.yfzhao at 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, '.');
More information about the Linux-erofs
mailing list