Re: [PATCH 3/3] mount_service: call the new fsmount ms_flags helpers in the right order

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Jun 01, 2026 at 06:53:25PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Codex noticed a discrepancy between the new fsmount code that Bernd
> wrote and my port of the even newer mount service code to use the
> helpers that Bernd wrote.  Specifically, set_fsconfig_ms_flags only
> clears bits from MS_FLAGS if there's no corresponding MOUNT_ATTR_ flag,
> whereas ms_flags_to_mount_attrs always clears them.
> 
> In other words, set_fsconfig_ms_flags MUST be called before
> ms_flags_to_mount_attrs or we can lose the 'ro' option.  Fix this.
> 
> Fixes: 7211953256526f ("mount_service: use the fsmount API helpers from mount_fsmount.c")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  lib/mount_fsmount.c  |    1 +
>  util/mount_service.c |   12 +++++++++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/lib/mount_fsmount.c b/lib/mount_fsmount.c
> index 83768f0b8d193b..7bd772d139808e 100644
> --- a/lib/mount_fsmount.c
> +++ b/lib/mount_fsmount.c
> @@ -35,6 +35,7 @@
>  #define MOUNT_ATTR_NOSYMFOLLOW  0x00200000
>  #endif
>  
> +/* Must be called after set_fsconfig_ms_flags */
>  unsigned long ms_flags_to_mount_attrs(unsigned long ms_flags,
>  				      unsigned int *mount_attrs)
>  {
> diff --git a/util/mount_service.c b/util/mount_service.c
> index 360a78bab3bd3d..f1b61ace526275 100644
> --- a/util/mount_service.c
> +++ b/util/mount_service.c
> @@ -1410,14 +1410,12 @@ static int mount_service_fsopen_mount(struct mount_service *mo,
>  				      const struct stat *stbuf)
>  {
>  	char tmp[64];
> -	unsigned long ms_flags;
> +	unsigned long ms_flags = ntohl(oc->ms_flags);
>  	unsigned int attr_flags;
>  	int mfd;
>  	int error;
>  	int ret;
>  
> -	ms_flags = ms_flags_to_mount_attrs(ntohl(oc->ms_flags), &attr_flags);
> -
>  	ret = set_fsconfig_ms_flags(mo->fsopenfd, &ms_flags);
>  	if (ret) {
>  		error = errno;
> @@ -1478,6 +1476,14 @@ static int mount_service_fsopen_mount(struct mount_service *mo,
>  		goto fail_fsconfig;
>  	}
>  
> +	ms_flags = ms_flags_to_mount_attrs(ms_flags, &attr_flags);

NAK, this call should have been moved only to the other side of
set_fsconfig_ms_flags, otherwise the fallback is triggered for any MS_*
flags that could easily be translated into MOUNT_ATTR_* flags.

--D

> +	if (ms_flags != 0) {
> +		error = ENOTSUP;
> +		fprintf(stderr, "%s: unsupported mount flags encountered\n",
> +			mo->msgtag);
> +		goto fail_fsconfig;
> +	}
> +
>  	mfd = fsmount(mo->fsopenfd, FSMOUNT_CLOEXEC, attr_flags);
>  	if (mfd < 0) {
>  		error = errno;
> 
>