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

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

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