[PATCH] 9pfs: fix deep path truncation in V9fsPath

[PATCH] 9pfs: fix deep path truncation in V9fsPath

[Jia Jia]

V9fsPath.size tracks the length of backend path data. Storing it in a
uint16_t truncates local backend paths longer than 65535 bytes, so later
path copies can end up much smaller than the string data they are
supposed to describe.

A guest can reach this with normal 9p filesystem operations by creating
and walking a sufficiently deep directory tree on the local backend. On
an ASan build, calling readdir() in that deep directory aborts the host
process with:

  ERROR: AddressSanitizer: heap-buffer-overflow
    #0 __interceptor_strrchr
    #1 g_path_get_dirname
    #2 local_lstat
    #3 v9fs_co_lstat
    #4 v9fs_getattr

Fix this by storing V9fsPath lengths in size_t.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3358
Cc: qemu-stable@nongnu.org
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
Runtime reproducer:
  confirmed on current master (11.0.50) with an x86_64 ASan build and a
  local 9p backend

  guest actions:
    - mount the 9p share
    - create a 260-level directory tree with 255-byte names
    - walk back to the deepest directory
    - call readdir()

  host abort:
    ERROR: AddressSanitizer: heap-buffer-overflow
      #0 __interceptor_strrchr
      #1 g_path_get_dirname
      #2 local_lstat
      #3 v9fs_co_lstat
      #4 v9fs_getattr

 fsdev/file-op-9p.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index b85c9934def..e8d0661c4b5 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -112,7 +112,7 @@ struct FsContext {
 };
 
 struct V9fsPath {
-    uint16_t size;
+    size_t size;
     char *data;
 };
 P9ARRAY_DECLARE_TYPE(V9fsPath);
-- 
2.34.1

Please ignore my [PATCH] 9pfs: fix deep path truncation in V9fsPath

[Jia Jia]

Hi,

Please ignore my [PATCH] 9pfs: fix deep path truncation in V9fsPath.

Sorry. This issue has already been submitted, and the existing GitLab
report already includes a concrete fix proposal:
https://gitlab.com/qemu-project/qemu/-/work_items/3358

Please ignore my patch, and sorry for the noise.

Thanks,
Jia Jia

Re: [PATCH] 9pfs: fix deep path truncation in V9fsPath

[Christian Schoenebeck]

On Tuesday, 28 April 2026 09:46:14 CEST Jia Jia wrote:
> V9fsPath.size tracks the length of backend path data. Storing it in a
> uint16_t truncates local backend paths longer than 65535 bytes, so later
> path copies can end up much smaller than the string data they are
> supposed to describe.
> 
> A guest can reach this with normal 9p filesystem operations by creating
> and walking a sufficiently deep directory tree on the local backend. On
> an ASan build, calling readdir() in that deep directory aborts the host
> process with:
> 
>   ERROR: AddressSanitizer: heap-buffer-overflow
>     #0 __interceptor_strrchr
>     #1 g_path_get_dirname
>     #2 local_lstat
>     #3 v9fs_co_lstat
>     #4 v9fs_getattr
> 
> Fix this by storing V9fsPath lengths in size_t.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3358
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> ---

Hi Jia, thanks for looking at this issue!

> Runtime reproducer:
>   confirmed on current master (11.0.50) with an x86_64 ASan build and a
>   local 9p backend
> 
>   guest actions:
>     - mount the 9p share
>     - create a 260-level directory tree with 255-byte names
>     - walk back to the deepest directory
>     - call readdir()
> 
>   host abort:
>     ERROR: AddressSanitizer: heap-buffer-overflow
>       #0 __interceptor_strrchr
>       #1 g_path_get_dirname
>       #2 local_lstat
>       #3 v9fs_co_lstat
>       #4 v9fs_getattr
> 
>  fsdev/file-op-9p.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index b85c9934def..e8d0661c4b5 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -112,7 +112,7 @@ struct FsContext {
>  };
> 
>  struct V9fsPath {
> -    uint16_t size;
> +    size_t size;
>      char *data;
>  };
>  P9ARRAY_DECLARE_TYPE(V9fsPath);

I fear it is not that simple. Just changing this data type would only move the 
problem and furthermore turn a small OOB into a giant OOB: g_vasprintf() 
(called by v9fs_path_sprintf()) has as return type gint -> int, and 
v9fs_path_sprintf() returns -1 on error, and this change would then implicitly 
cast -1 to a giant unsigned value.

AFAICS the only way for truly fixing this is by getting rid of dragging full 
paths around with FIDs in general, which would be a massive change though.

On short term I only see a possible mitigation: adding error handling to 
v9fs_path_sprintf(), and for FIDs where v9fs_fix_path() fails, making the fids 
inaccessible for good (i.e. immediately closing those FIDs).

/Christian

Re: [PATCH] 9pfs: fix deep path truncation in V9fsPath

[Jia Jia]

Hi Christian,

I agree. Just widening V9fsPath.size is not a real fix; given that
g_vasprintf() returns a signed value and v9fs_path_sprintf() can return -1,
my change could indeed make the failure case worse by turning that into a
huge unsigned size.

I also realized afterwards that this issue had already been reported and that
my patch was essentially repeating the same incomplete direction. That is why
I sent the follow-up asking people to ignore it:
https://lore.kernel.org/all/20260428082151.3234483-1-physicalmtea@gmail.com/

If I understood your suggestion correctly, the short-term mitigation would be
roughly:

  - make v9fs_path_sprintf() fail on both g_vasprintf() failure and overlong
    results;
  - propagate that error through callers such as local_name_to_path();
  - make v9fs_fix_path() return an error as well;
  - and if a live fid path cannot be rebuilt during rename/fixup, clunk or
    otherwise invalidate that fid immediately instead of leaving it reachable
    with invalid path state.

So this is not just a local check in v9fs_path_sprintf(), but also error
propagation plus fid invalidation on fixup failure.

Sorry for the noise, and thanks for the clarification.

Thanks,
Jia Jia

Re: [PATCH] 9pfs: fix deep path truncation in V9fsPath

[Christian Schoenebeck]

On Thursday, 30 April 2026 14:52:41 CEST Jia Jia wrote:
[...]
> If I understood your suggestion correctly, the short-term mitigation would
> be roughly:
> 
>   - make v9fs_path_sprintf() fail on both g_vasprintf() failure and overlong
> results;
>   - propagate that error through callers such as local_name_to_path();
>   - make v9fs_fix_path() return an error as well;
>   - and if a live fid path cannot be rebuilt during rename/fixup, clunk or
>     otherwise invalidate that fid immediately instead of leaving it
> reachable with invalid path state.

Yes, that's roughly what I had in mind as short-term mitigation. Additionally 
I would suggest calling error_report_once() somewhere, as it would be an 
unusual incident that should be logged.

> So this is not just a local check in v9fs_path_sprintf(), but also error
> propagation plus fid invalidation on fixup failure.

Right, unfortunately I don't see an easier way to address this. E.g. one might 
think to just add a max. path length check when creating a new file/dir, which 
would fix the reported vector, but then it could still be triggered by moving 
a dir into a subdir, and the moved dir could have a deep directory structure, 
so one simple path length check would not be sufficient.

/Christian