[PATCH] 9p: fix access mode flags being ORed instead of replaced

[PATCH] 9p: fix access mode flags being ORed instead of replaced

[Pierre Barre]

Since commit 1f3e4142c0eb ("9p: convert to the new mount API"),
v9fs_apply_options() applies parsed mount flags with |= onto flags
already set by v9fs_session_init(). For 9P2000.L, session_init sets
V9FS_ACCESS_CLIENT as the default, so when the user mounts with
"access=user", both bits end up set. Access mode checks compare
against exact values, so having both bits set matches neither mode.

This causes v9fs_fid_lookup() to fall through to the default switch
case, using INVALID_UID (nobody/65534) instead of current_fsuid()
for all fid lookups. Root is then unable to chown or perform other
privileged operations.

Fix by clearing the access mask before applying the user's choice.

Fixes: 1f3e4142c0eb ("9p: convert to the new mount API")
Signed-off-by: Pierre Barre <pierre@barre.sh>
---
 fs/9p/v9fs.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 057487efaaeb..05a5e1c4df35 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -413,7 +413,11 @@ static void v9fs_apply_options(struct v9fs_session_info *v9ses,
        /*
         * Note that we must |= flags here as session_init already
         * set basic flags. This adds in flags from parsed options.
+        * Access flags are mutually exclusive, so clear any access
+        * bits set by session_init before applying the user's choice.
         */
+       if (ctx->session_opts.flags & V9FS_ACCESS_MASK)
+               v9ses->flags &= ~V9FS_ACCESS_MASK;
        v9ses->flags |= ctx->session_opts.flags;
 #ifdef CONFIG_9P_FSCACHE
        v9ses->cachetag = ctx->session_opts.cachetag;
--
2.51.0

Re: [PATCH] 9p: fix access mode flags being ORed instead of replaced

[Pierre Barre]

To reproduce: mount a 9P2000.L filesystem with access=user on kernel 6.19+:

# mount -t 9p -o trans=tcp,port=5564,version=9p2000.L,access=user 127.0.0.1 /mnt/9p

Then as root:

  touch /mnt/9p/test
  chown root:root /mnt/9p/test
  
# chown: changing ownership of '/mnt/9p/test': Operation not permitted

Tracing the server side confirms the attach arrives with uid=65534 (nobody) instead of 0 (root).

On Thu, Apr 2, 2026, at 12:03, Pierre Barre wrote:
> Since commit 1f3e4142c0eb ("9p: convert to the new mount API"),
> v9fs_apply_options() applies parsed mount flags with |= onto flags
> already set by v9fs_session_init(). For 9P2000.L, session_init sets
> V9FS_ACCESS_CLIENT as the default, so when the user mounts with
> "access=user", both bits end up set. Access mode checks compare
> against exact values, so having both bits set matches neither mode.
>
> This causes v9fs_fid_lookup() to fall through to the default switch
> case, using INVALID_UID (nobody/65534) instead of current_fsuid()
> for all fid lookups. Root is then unable to chown or perform other
> privileged operations.
>
> Fix by clearing the access mask before applying the user's choice.
>
> Fixes: 1f3e4142c0eb ("9p: convert to the new mount API")
> Signed-off-by: Pierre Barre <pierre@barre.sh>
> ---
>  fs/9p/v9fs.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index 057487efaaeb..05a5e1c4df35 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -413,7 +413,11 @@ static void v9fs_apply_options(struct 
> v9fs_session_info *v9ses,
>         /*
>          * Note that we must |= flags here as session_init already
>          * set basic flags. This adds in flags from parsed options.
> +        * Access flags are mutually exclusive, so clear any access
> +        * bits set by session_init before applying the user's choice.
>          */
> +       if (ctx->session_opts.flags & V9FS_ACCESS_MASK)
> +               v9ses->flags &= ~V9FS_ACCESS_MASK;
>         v9ses->flags |= ctx->session_opts.flags;
>  #ifdef CONFIG_9P_FSCACHE
>         v9ses->cachetag = ctx->session_opts.cachetag;
> --
> 2.51.0

Re: [PATCH] 9p: fix access mode flags being ORed instead of replaced

[Pierre Barre]

Hi,

Friendly ping on this, any thoughts or feedback?

Thanks!

On Thu, Apr 2, 2026, at 12:03, Pierre Barre wrote:
> Since commit 1f3e4142c0eb ("9p: convert to the new mount API"),
> v9fs_apply_options() applies parsed mount flags with |= onto flags
> already set by v9fs_session_init(). For 9P2000.L, session_init sets
> V9FS_ACCESS_CLIENT as the default, so when the user mounts with
> "access=user", both bits end up set. Access mode checks compare
> against exact values, so having both bits set matches neither mode.
>
> This causes v9fs_fid_lookup() to fall through to the default switch
> case, using INVALID_UID (nobody/65534) instead of current_fsuid()
> for all fid lookups. Root is then unable to chown or perform other
> privileged operations.
>
> Fix by clearing the access mask before applying the user's choice.
>
> Fixes: 1f3e4142c0eb ("9p: convert to the new mount API")
> Signed-off-by: Pierre Barre <pierre@barre.sh>
> ---
>  fs/9p/v9fs.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index 057487efaaeb..05a5e1c4df35 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -413,7 +413,11 @@ static void v9fs_apply_options(struct 
> v9fs_session_info *v9ses,
>         /*
>          * Note that we must |= flags here as session_init already
>          * set basic flags. This adds in flags from parsed options.
> +        * Access flags are mutually exclusive, so clear any access
> +        * bits set by session_init before applying the user's choice.
>          */
> +       if (ctx->session_opts.flags & V9FS_ACCESS_MASK)
> +               v9ses->flags &= ~V9FS_ACCESS_MASK;
>         v9ses->flags |= ctx->session_opts.flags;
>  #ifdef CONFIG_9P_FSCACHE
>         v9ses->cachetag = ctx->session_opts.cachetag;
> --
> 2.51.0

Re: [PATCH] 9p: fix access mode flags being ORed instead of replaced

[Christian Schoenebeck]

On Thursday, 2 April 2026 12:03:12 CEST Pierre Barre wrote:
> Since commit 1f3e4142c0eb ("9p: convert to the new mount API"),
> v9fs_apply_options() applies parsed mount flags with |= onto flags
> already set by v9fs_session_init(). For 9P2000.L, session_init sets
> V9FS_ACCESS_CLIENT as the default, so when the user mounts with
> "access=user", both bits end up set. Access mode checks compare
> against exact values, so having both bits set matches neither mode.
> 
> This causes v9fs_fid_lookup() to fall through to the default switch
> case, using INVALID_UID (nobody/65534) instead of current_fsuid()
> for all fid lookups. Root is then unable to chown or perform other
> privileged operations.
> 
> Fix by clearing the access mask before applying the user's choice.
> 
> Fixes: 1f3e4142c0eb ("9p: convert to the new mount API")
> Signed-off-by: Pierre Barre <pierre@barre.sh>
> ---
>  fs/9p/v9fs.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index 057487efaaeb..05a5e1c4df35 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -413,7 +413,11 @@ static void v9fs_apply_options(struct v9fs_session_info
> *v9ses, /*
>          * Note that we must |= flags here as session_init already
>          * set basic flags. This adds in flags from parsed options.
> +        * Access flags are mutually exclusive, so clear any access
> +        * bits set by session_init before applying the user's choice.

That phrase is a bit suboptimal, because V9FS_ACCESS_ANY is actually a bit
combination of single, user and client. But OK, I currently don't have a
better phrase for it since the access fields have to be replaced altogether.

As for the actual behaviour change; makes sense to me:

Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com>

>          */
> +       if (ctx->session_opts.flags & V9FS_ACCESS_MASK)
> +               v9ses->flags &= ~V9FS_ACCESS_MASK;
>         v9ses->flags |= ctx->session_opts.flags;
>  #ifdef CONFIG_9P_FSCACHE
>         v9ses->cachetag = ctx->session_opts.cachetag;
> --
> 2.51.0

Re: [PATCH] 9p: fix access mode flags being ORed instead of replaced

[Dominique Martinet]

Christian Schoenebeck wrote on Thu, Apr 09, 2026 at 04:51:07PM +0200:
> > diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> > index 057487efaaeb..05a5e1c4df35 100644
> > --- a/fs/9p/v9fs.c
> > +++ b/fs/9p/v9fs.c
> > @@ -413,7 +413,11 @@ static void v9fs_apply_options(struct v9fs_session_info
> > *v9ses, /*
> >          * Note that we must |= flags here as session_init already
> >          * set basic flags. This adds in flags from parsed options.
> > +        * Access flags are mutually exclusive, so clear any access
> > +        * bits set by session_init before applying the user's choice.
> 
> That phrase is a bit suboptimal, because V9FS_ACCESS_ANY is actually a bit
> combination of single, user and client. But OK, I currently don't have a
> better phrase for it since the access fields have to be replaced altogether.

Yeah, it's not so much that they're mutually exclusive that we need to
clear the default value before applying the settings.
The key distinction here is that it's not "any access bits set" -- if it
were arbitrary values then it wouldn't be acceptable to just or the
flags out, you could risk e.g. creating a client with 2000L but setting
another protocol in the flags and there'd be no end of things to check.

Something like this?
           * Default access flags must be cleared if session options
	   * change them to avoid mangling the setting.

> As for the actual behaviour change; makes sense to me:

Yes, that works for me.

Note your patch converted tabs to space and couldn't be applied -- if
you have to send patches by web mail please attach them instead, that'll
be easier to apply than a corrupted patch, even if some tooling like
sashiko won't run (it doesn't run on corrupted patches anyway..)


If you're fine with my wording for the comment I'll modify it in place,
otherwise we have a few more days to discuss this before I sent to Linus
for 7.1-rc1
(and I'm really sorry for my lack of reactivity, I'm sure I'll miss some
patches anyway...)

-- 
Dominique Martinet | Asmadeus

Re: [PATCH] 9p: fix access mode flags being ORed instead of replaced

[Pierre Barre]

> Note your patch converted tabs to space and couldn't be applied -- if
> you have to send patches by web mail please attach them instead, that'll
> be easier to apply than a corrupted patch, even if some tooling like
> sashiko won't run (it doesn't run on corrupted patches anyway..)

Argh, I'll just commit to using git-send-email from now on.

> If you're fine with my wording for the comment I'll modify it in place,

Sounds great, thank you.

Pierre.

On Thu, Apr 16, 2026, at 04:53, Dominique Martinet wrote:
> Christian Schoenebeck wrote on Thu, Apr 09, 2026 at 04:51:07PM +0200:
>> > diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
>> > index 057487efaaeb..05a5e1c4df35 100644
>> > --- a/fs/9p/v9fs.c
>> > +++ b/fs/9p/v9fs.c
>> > @@ -413,7 +413,11 @@ static void v9fs_apply_options(struct v9fs_session_info
>> > *v9ses, /*
>> >          * Note that we must |= flags here as session_init already
>> >          * set basic flags. This adds in flags from parsed options.
>> > +        * Access flags are mutually exclusive, so clear any access
>> > +        * bits set by session_init before applying the user's choice.
>> 
>> That phrase is a bit suboptimal, because V9FS_ACCESS_ANY is actually a bit
>> combination of single, user and client. But OK, I currently don't have a
>> better phrase for it since the access fields have to be replaced altogether.
>
> Yeah, it's not so much that they're mutually exclusive that we need to
> clear the default value before applying the settings.
> The key distinction here is that it's not "any access bits set" -- if it
> were arbitrary values then it wouldn't be acceptable to just or the
> flags out, you could risk e.g. creating a client with 2000L but setting
> another protocol in the flags and there'd be no end of things to check.
>
> Something like this?
>            * Default access flags must be cleared if session options
> 	   * change them to avoid mangling the setting.
>
>> As for the actual behaviour change; makes sense to me:
>
> Yes, that works for me.
>
> Note your patch converted tabs to space and couldn't be applied -- if
> you have to send patches by web mail please attach them instead, that'll
> be easier to apply than a corrupted patch, even if some tooling like
> sashiko won't run (it doesn't run on corrupted patches anyway..)
>
>
> If you're fine with my wording for the comment I'll modify it in place,
> otherwise we have a few more days to discuss this before I sent to Linus
> for 7.1-rc1
> (and I'm really sorry for my lack of reactivity, I'm sure I'll miss some
> patches anyway...)
>
> -- 
> Dominique Martinet | Asmadeus