public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] 9p: fix access mode flags being ORed instead of replaced
@ 2026-04-02 10:03 Pierre Barre
  2026-04-02 10:10 ` Pierre Barre
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Pierre Barre @ 2026-04-02 10:03 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus; +Cc: linux_oss, v9fs, linux-kernel, stable, sandeen

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] 9p: fix access mode flags being ORed instead of replaced
  2026-04-02 10:03 [PATCH] 9p: fix access mode flags being ORed instead of replaced Pierre Barre
@ 2026-04-02 10:10 ` Pierre Barre
  2026-04-09  8:12 ` Pierre Barre
  2026-04-09 14:51 ` Christian Schoenebeck
  2 siblings, 0 replies; 4+ messages in thread
From: Pierre Barre @ 2026-04-02 10:10 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus
  Cc: Christian Schoenebeck, v9fs, linux-kernel, stable, sandeen

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] 9p: fix access mode flags being ORed instead of replaced
  2026-04-02 10:03 [PATCH] 9p: fix access mode flags being ORed instead of replaced Pierre Barre
  2026-04-02 10:10 ` Pierre Barre
@ 2026-04-09  8:12 ` Pierre Barre
  2026-04-09 14:51 ` Christian Schoenebeck
  2 siblings, 0 replies; 4+ messages in thread
From: Pierre Barre @ 2026-04-09  8:12 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus
  Cc: Christian Schoenebeck, v9fs, linux-kernel, stable, sandeen

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] 9p: fix access mode flags being ORed instead of replaced
  2026-04-02 10:03 [PATCH] 9p: fix access mode flags being ORed instead of replaced Pierre Barre
  2026-04-02 10:10 ` Pierre Barre
  2026-04-09  8:12 ` Pierre Barre
@ 2026-04-09 14:51 ` Christian Schoenebeck
  2 siblings, 0 replies; 4+ messages in thread
From: Christian Schoenebeck @ 2026-04-09 14:51 UTC (permalink / raw)
  To: ericvh, lucho, asmadeus, Pierre Barre; +Cc: v9fs, linux-kernel, stable, sandeen

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





^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-09 15:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-02 10:03 [PATCH] 9p: fix access mode flags being ORed instead of replaced Pierre Barre
2026-04-02 10:10 ` Pierre Barre
2026-04-09  8:12 ` Pierre Barre
2026-04-09 14:51 ` Christian Schoenebeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox