[PATCH] hw/9pfs: fix heap-buffer-overflow in v9fs_complete_rename

[PATCH] hw/9pfs: fix heap-buffer-overflow in v9fs_complete_rename

[Christian Schoenebeck]

From: Oliver Chang <ochang@google.com>

When `v9fs_complete_rename` is called with `newdirfid == -1`, it attempts to
derive the directory name from `fidp->path.data` using `g_path_get_dirname`.
This logic assumes that `fidp->path.data` always contains a null-terminated
string representing a pathname.

While this assumption holds for the 'local' backend, the 'synth' backend stores
a `V9fsSynthNode *` pointer directly in the `V9fsPath.data` buffer. When using
'synth', `g_path_get_dirname` treats this pointer as a string, often resulting
in a short string like ".".

The subsequent call to `v9fs_co_name_to_path` invokes `synth_name_to_path`,
which expects `dir_path.data` to contain a `V9fsSynthNode *`. It attempts to
read 8 bytes (on 64-bit) from the buffer. If `g_path_get_dirname` returned a
short string, this results in a heap-buffer-overflow read.

Fix this by checking for the `V9FS_PATHNAME_FSCONTEXT` flag in the export
flags. This flag indicates that the backend supports string-based pathnames. If
it is not set (as in the 'synth' backend), return `-EOPNOTSUPP` to prevent
invalid memory access.

Co-authored-by: CodeMender <codemender-patching@google.com>
Fixes: https://issues.oss-fuzz.com/issues/477990727
---
 hw/9pfs/9p.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 6fbe604ce8..546e70f75c 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3310,9 +3310,16 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp,
             goto out;
         }
     } else {
-        char *dir_name = g_path_get_dirname(fidp->path.data);
+        char *dir_name;
         V9fsPath dir_path;
 
+        if (!(s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT)) {
+            /* path renaming is only supported for path based fid */
+            err = -EOPNOTSUPP;
+            goto out;
+        }
+
+        dir_name = g_path_get_dirname(fidp->path.data);
         v9fs_path_init(&dir_path);
         v9fs_path_sprintf(&dir_path, "%s", dir_name);
         g_free(dir_name);
-- 

A.I. generated patch (was: [PATCH] hw/9pfs: fix heap-buffer-overflow in v9fs_complete_rename)

[Christian Schoenebeck]

On Friday, 13 February 2026 10:56:05 CET Christian Schoenebeck wrote:
> From: Oliver Chang <ochang@google.com>
> 
> When `v9fs_complete_rename` is called with `newdirfid == -1`, it attempts to
> derive the directory name from `fidp->path.data` using
> `g_path_get_dirname`. This logic assumes that `fidp->path.data` always
> contains a null-terminated string representing a pathname.
> 
> While this assumption holds for the 'local' backend, the 'synth' backend
> stores a `V9fsSynthNode *` pointer directly in the `V9fsPath.data` buffer.
> When using 'synth', `g_path_get_dirname` treats this pointer as a string,
> often resulting in a short string like ".".
> 
> The subsequent call to `v9fs_co_name_to_path` invokes `synth_name_to_path`,
> which expects `dir_path.data` to contain a `V9fsSynthNode *`. It attempts to
> read 8 bytes (on 64-bit) from the buffer. If `g_path_get_dirname` returned
> a short string, this results in a heap-buffer-overflow read.
> 
> Fix this by checking for the `V9FS_PATHNAME_FSCONTEXT` flag in the export
> flags. This flag indicates that the backend supports string-based pathnames.
> If it is not set (as in the 'synth' backend), return `-EOPNOTSUPP` to
> prevent invalid memory access.
> 
> Co-authored-by: CodeMender <codemender-patching@google.com>
> Fixes: https://issues.oss-fuzz.com/issues/477990727
> ---

Oliver, IIUIC your patch was auto generated by A.I. and AFAICS QEMU's current
policies forbid any A.I. generated contributions in general:

https://www.qemu.org/docs/master/devel/code-provenance.html#use-of-ai-generated-content

It is also missing a Signed-off-by: tag BTW.

Technically the patch should better be generalized anyway, at least by moving
the V9FS_PATHNAME_FSCONTEXT check to the beginning of the function, as
renaming is only possible with pathname-based fs drivers.

Peter, would that be sane enough or would such a v2 patch considered as
"derived from A.I content" as well?

/Christian

>  hw/9pfs/9p.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 6fbe604ce8..546e70f75c 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3310,9 +3310,16 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU
> *pdu, V9fsFidState *fidp, goto out;
>          }
>      } else {
> -        char *dir_name = g_path_get_dirname(fidp->path.data);
> +        char *dir_name;
>          V9fsPath dir_path;
> 
> +        if (!(s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT)) {
> +            /* path renaming is only supported for path based fid */
> +            err = -EOPNOTSUPP;
> +            goto out;
> +        }
> +
> +        dir_name = g_path_get_dirname(fidp->path.data);
>          v9fs_path_init(&dir_path);
>          v9fs_path_sprintf(&dir_path, "%s", dir_name);
>          g_free(dir_name);

Re: A.I. generated patch (was: [PATCH] hw/9pfs: fix heap-buffer-overflow in v9fs_complete_rename)

[Daniel P. Berrangé]

On Fri, Feb 13, 2026 at 01:25:57PM +0100, Christian Schoenebeck wrote:
> On Friday, 13 February 2026 10:56:05 CET Christian Schoenebeck wrote:
> > From: Oliver Chang <ochang@google.com>
> > 
> > When `v9fs_complete_rename` is called with `newdirfid == -1`, it attempts to
> > derive the directory name from `fidp->path.data` using
> > `g_path_get_dirname`. This logic assumes that `fidp->path.data` always
> > contains a null-terminated string representing a pathname.
> > 
> > While this assumption holds for the 'local' backend, the 'synth' backend
> > stores a `V9fsSynthNode *` pointer directly in the `V9fsPath.data` buffer.
> > When using 'synth', `g_path_get_dirname` treats this pointer as a string,
> > often resulting in a short string like ".".
> > 
> > The subsequent call to `v9fs_co_name_to_path` invokes `synth_name_to_path`,
> > which expects `dir_path.data` to contain a `V9fsSynthNode *`. It attempts to
> > read 8 bytes (on 64-bit) from the buffer. If `g_path_get_dirname` returned
> > a short string, this results in a heap-buffer-overflow read.
> > 
> > Fix this by checking for the `V9FS_PATHNAME_FSCONTEXT` flag in the export
> > flags. This flag indicates that the backend supports string-based pathnames.
> > If it is not set (as in the 'synth' backend), return `-EOPNOTSUPP` to
> > prevent invalid memory access.
> > 
> > Co-authored-by: CodeMender <codemender-patching@google.com>
> > Fixes: https://issues.oss-fuzz.com/issues/477990727
> > ---
> 
> Oliver, IIUIC your patch was auto generated by A.I. and AFAICS QEMU's current
> policies forbid any A.I. generated contributions in general:
> 
> https://www.qemu.org/docs/master/devel/code-provenance.html#use-of-ai-generated-content

This issue was previously reported to qemu-security where we have already
rejected inclusion of the patch on the basis that it was created using
AI tools which is not in compliance with our policy.

> It is also missing a Signed-off-by: tag BTW.

NB, signing off an AI (co-)authored patch is something we don't consider
valid per the policy.

> Technically the patch should better be generalized anyway, at least by moving
> the V9FS_PATHNAME_FSCONTEXT check to the beginning of the function, as
> renaming is only possible with pathname-based fs drivers.
> 
> Peter, would that be sane enough or would such a v2 patch considered as
> "derived from A.I content" as well?

Throw away the existing AI tainted patch, and write a clean patch
without reference to the original. It'll be a v1 patch at that
point.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|