All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] hw/9pfs: add NULL check in v9fs_path_is_ancestor()
  2026-05-18 17:40 [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
@ 2026-05-18 17:35 ` Christian Schoenebeck
  2026-05-18 17:35 ` [PATCH 2/6] hw/9pfs: change V9fsPath.size to size_t and v9fs_path_sprintf() return type Christian Schoenebeck
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Christian Schoenebeck @ 2026-05-18 17:35 UTC (permalink / raw)
  To: qemu-devel, qemu-stable; +Cc: Greg Kurz, Jia Jia

Add NULL check for s1->data and s2->data before using them in
string operations. This prevents potential crashes when dealing
with uninitialized paths.

This is just a defensive measure. We are currently never passing
NULL to this function.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index e2713b9eee..e590c414ab 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -241,6 +241,9 @@ int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
  */
 static int v9fs_path_is_ancestor(V9fsPath *s1, V9fsPath *s2)
 {
+    if (!s1->data || !s2->data) {
+        return 0;
+    }
     if (!strncmp(s1->data, s2->data, s1->size - 1)) {
         if (s2->data[s1->size - 1] == '\0' || s2->data[s1->size - 1] == '/') {
             return 1;
-- 
2.47.3



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

* [PATCH 2/6] hw/9pfs: change V9fsPath.size to size_t and v9fs_path_sprintf() return type
  2026-05-18 17:40 [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
  2026-05-18 17:35 ` [PATCH 1/6] hw/9pfs: add NULL check in v9fs_path_is_ancestor() Christian Schoenebeck
@ 2026-05-18 17:35 ` Christian Schoenebeck
  2026-05-18 17:35 ` [PATCH 3/6] hw/9pfs: add error handling to v9fs_fix_path() Christian Schoenebeck
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Christian Schoenebeck @ 2026-05-18 17:35 UTC (permalink / raw)
  To: qemu-devel, qemu-stable; +Cc: Greg Kurz, Jia Jia

- Change V9fsPath.size from uint16_t to size_t to support paths larger
  than 65536 bytes.

- Change v9fs_path_sprintf() return type from void to int to allow error
  reporting.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 fsdev/file-op-9p.h |  2 +-
 hw/9pfs/9p.c       | 14 +++++++++++---
 hw/9pfs/9p.h       |  4 ++--
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index b85c9934de..e8d0661c4b 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);
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index e590c414ab..88894ec9d2 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -203,16 +203,24 @@ void v9fs_path_free(V9fsPath *path)
 }
 
 
-void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...)
+int v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...)
 {
     va_list ap;
+    int ret;
 
     v9fs_path_free(path);
 
     va_start(ap, fmt);
-    /* Bump the size for including terminating NULL */
-    path->size = g_vasprintf(&path->data, fmt, ap) + 1;
+    ret = g_vasprintf(&path->data, fmt, ap);
     va_end(ap);
+    if (ret < 0) {
+        error_report_once("9pfs: unusual path formatting failure; "
+                         "invalidating associated FID");
+        return -1;
+    }
+    /* Bump the size for including terminating NULL */
+    path->size = ret + 1;
+    return 0;
 }
 
 void v9fs_path_copy(V9fsPath *dst, const V9fsPath *src)
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 65cc45e344..b2df659b0e 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -456,8 +456,8 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu)
 void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu);
 void v9fs_path_init(V9fsPath *path);
 void v9fs_path_free(V9fsPath *path);
-void G_GNUC_PRINTF(2, 3) v9fs_path_sprintf(V9fsPath *path, const char *fmt,
-                                           ...);
+int G_GNUC_PRINTF(2, 3) v9fs_path_sprintf(V9fsPath *path, const char *fmt,
+                                          ...);
 void v9fs_path_copy(V9fsPath *dst, const V9fsPath *src);
 size_t v9fs_readdir_response_size(V9fsString *name);
 int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
-- 
2.47.3



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

* [PATCH 3/6] hw/9pfs: add error handling to v9fs_fix_path()
  2026-05-18 17:40 [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
  2026-05-18 17:35 ` [PATCH 1/6] hw/9pfs: add NULL check in v9fs_path_is_ancestor() Christian Schoenebeck
  2026-05-18 17:35 ` [PATCH 2/6] hw/9pfs: change V9fsPath.size to size_t and v9fs_path_sprintf() return type Christian Schoenebeck
@ 2026-05-18 17:35 ` Christian Schoenebeck
  2026-05-18 17:35 ` [PATCH 4/6] hw/9pfs: let callers of v9fs_path_sprintf() and v9fs_fix_path() handle errors Christian Schoenebeck
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Christian Schoenebeck @ 2026-05-18 17:35 UTC (permalink / raw)
  To: qemu-devel, qemu-stable; +Cc: Greg Kurz, Jia Jia

Update v9fs_fix_path() to return int and propagate errors from
v9fs_path_sprintf(). This allows callers to detect and handle
path formatting failures.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 88894ec9d2..d704de644f 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1417,13 +1417,15 @@ static void print_sg(struct iovec *sg, int cnt)
 }
 
 /* Will call this only for path name based fid */
-static void v9fs_fix_path(V9fsPath *dst, V9fsPath *src, int len)
+static int v9fs_fix_path(V9fsPath *dst, V9fsPath *src, int len)
 {
     V9fsPath str;
+    int ret;
     v9fs_path_init(&str);
     v9fs_path_copy(&str, dst);
-    v9fs_path_sprintf(dst, "%s%s", src->data, str.data + len);
+    ret = v9fs_path_sprintf(dst, "%s%s", src->data, str.data + len);
     v9fs_path_free(&str);
+    return ret;
 }
 
 static inline bool is_ro_export(FsContext *ctx)
-- 
2.47.3



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

* [PATCH 4/6] hw/9pfs: let callers of v9fs_path_sprintf() and v9fs_fix_path() handle errors
  2026-05-18 17:40 [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
                   ` (2 preceding siblings ...)
  2026-05-18 17:35 ` [PATCH 3/6] hw/9pfs: add error handling to v9fs_fix_path() Christian Schoenebeck
@ 2026-05-18 17:35 ` Christian Schoenebeck
  2026-05-18 17:36 ` [PATCH 5/6] tests/qtest/libqos: add qvirtqueue_reset_pool() for descriptor pool reset Christian Schoenebeck
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Christian Schoenebeck @ 2026-05-18 17:35 UTC (permalink / raw)
  To: qemu-devel, qemu-stable; +Cc: Greg Kurz, Jia Jia

This patch mitigates issues with very large absolute paths.

- Add error handling to all v9fs_path_sprintf() calls in
  local_name_to_path()

- Update callers of v9fs_fix_path() to check return values.

- When path formatting fails, clunk the affected FIDs to prevent use of
  invalid paths.

- Use g_autofree for temporary variables to simplify code.

Even though paths are usually limited to PATH_MAX (typically 4k) on guest,
this limitation can be circumvented by using *at() functions on guest and
creating very deep directory structures. This was a problem for QEMU 9p
server, as it currently tracks the absolute path for each FID internally
that always requires assembly of a (potentially ver large) absolute path.

A true long-term fix would be getting rid of storing an absolute path for
each FID internally. However that would likely be a massive change with
uncertain implications.

This patch therefore just mitigates the problem by immediately clunking
(i.e. closing) all FIDs whose path exceed a limit that we could handle.
As this only accounts to very unusual large absolute paths not ever been
reported on (sane) production machines, this is currently considered an
acceptable mitigation that should only (counter)affect malicious attempts.

Fixes: 2f008a8c97e2 ("hw/9pfs: Use the correct signed type ...")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3358
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p-local.c | 23 ++++++++++++++++-------
 hw/9pfs/9p.c       | 18 +++++++++++++-----
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 24cb1da90a..aa48306b0e 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1261,26 +1261,35 @@ static int local_name_to_path(FsContext *ctx, V9fsPath *dir_path,
         } else if (!strcmp(name, "..")) {
             if (!strcmp(dir_path->data, ".")) {
                 /* ".." relative to the root is "." */
-                v9fs_path_sprintf(target, ".");
+                if (v9fs_path_sprintf(target, ".") < 0) {
+                    return -1;
+                }
             } else {
-                char *tmp = g_path_get_dirname(dir_path->data);
+                g_autofree char *tmp = g_path_get_dirname(dir_path->data);
                 /* Symbolic links are resolved by the client. We can assume
                  * that ".." relative to "foo/bar" is equivalent to "foo"
                  */
-                v9fs_path_sprintf(target, "%s", tmp);
-                g_free(tmp);
+                if (v9fs_path_sprintf(target, "%s", tmp) < 0) {
+                    return -1;
+                }
             }
         } else {
             assert(!strchr(name, '/'));
-            v9fs_path_sprintf(target, "%s/%s", dir_path->data, name);
+            if (v9fs_path_sprintf(target, "%s/%s", dir_path->data, name) < 0) {
+                return -1;
+            }
         }
     } else if (!strcmp(name, "/") || !strcmp(name, ".") ||
                !strcmp(name, "..")) {
             /* This is the root fid */
-        v9fs_path_sprintf(target, ".");
+        if (v9fs_path_sprintf(target, ".") < 0) {
+            return -1;
+        }
     } else {
         assert(!strchr(name, '/'));
-        v9fs_path_sprintf(target, "./%s", name);
+        if (v9fs_path_sprintf(target, "./%s", name) < 0) {
+            return -1;
+        }
     }
     return 0;
 }
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index d704de644f..b4314d2549 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3325,12 +3325,14 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp,
             goto out;
         }
     } else {
-        char *dir_name = g_path_get_dirname(fidp->path.data);
+        g_autofree char *dir_name = g_path_get_dirname(fidp->path.data);
         V9fsPath dir_path;
 
         v9fs_path_init(&dir_path);
-        v9fs_path_sprintf(&dir_path, "%s", dir_name);
-        g_free(dir_name);
+        err = v9fs_path_sprintf(&dir_path, "%s", dir_name);
+        if (err < 0) {
+            goto out;
+        }
 
         err = v9fs_co_name_to_path(pdu, &dir_path, name->data, &new_path);
         v9fs_path_free(&dir_path);
@@ -3351,7 +3353,10 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp,
     while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &tfidp)) {
         if (v9fs_path_is_ancestor(&fidp->path, &tfidp->path)) {
             /* replace the name */
-            v9fs_fix_path(&tfidp->path, &new_path, strlen(fidp->path.data));
+            if (v9fs_fix_path(&tfidp->path, &new_path,
+                              strlen(fidp->path.data)) < 0) {
+                clunk_fid(s, tfidp->fid);
+            }
         }
     }
 out:
@@ -3448,7 +3453,10 @@ static int coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir,
     while (g_hash_table_iter_next(&iter, &fid, (gpointer *) &tfidp)) {
         if (v9fs_path_is_ancestor(&oldpath, &tfidp->path)) {
             /* replace the name */
-            v9fs_fix_path(&tfidp->path, &newpath, strlen(oldpath.data));
+            if (v9fs_fix_path(&tfidp->path, &newpath,
+                              strlen(oldpath.data)) < 0) {
+                clunk_fid(s, tfidp->fid);
+            }
         }
     }
 out:
-- 
2.47.3



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

* [PATCH 5/6] tests/qtest/libqos: add qvirtqueue_reset_pool() for descriptor pool reset
  2026-05-18 17:40 [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
                   ` (3 preceding siblings ...)
  2026-05-18 17:35 ` [PATCH 4/6] hw/9pfs: let callers of v9fs_path_sprintf() and v9fs_fix_path() handle errors Christian Schoenebeck
@ 2026-05-18 17:36 ` Christian Schoenebeck
  2026-05-19  7:51   ` Fabiano Rosas
  2026-05-18 17:36 ` [PATCH 6/6] tests/9pfs: add deep absolute path test Christian Schoenebeck
  2026-05-27 14:35 ` [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
  6 siblings, 1 reply; 9+ messages in thread
From: Christian Schoenebeck @ 2026-05-18 17:36 UTC (permalink / raw)
  To: qemu-devel, qemu-stable
  Cc: Greg Kurz, Jia Jia, Fabiano Rosas, Laurent Vivier, Paolo Bonzini

Add a function to reset the virtqueue descriptor pool state without
reinitializing the device. This is useful for tests that issue a high
number of requests and are limited by the simplified virtio test
driver's descriptor tracking, which decrements num_free but never
increments it back.

The function is safe for synchronous test code where requests are
sent and completed before the next request is issued.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/libqos/virtio.c | 23 +++++++++++++++++++++++
 tests/qtest/libqos/virtio.h |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/tests/qtest/libqos/virtio.c b/tests/qtest/libqos/virtio.c
index 010ff40834..ccbb325222 100644
--- a/tests/qtest/libqos/virtio.c
+++ b/tests/qtest/libqos/virtio.c
@@ -464,6 +464,29 @@ bool qvirtqueue_get_buf(QTestState *qts, QVirtQueue *vq, uint32_t *desc_idx,
     return true;
 }
 
+/*
+ * qvirtqueue_reset_pool:
+ * @vq: The virtqueue to reset
+ *
+ * Reset the descriptor pool state without reinitializing the device.
+ * This is useful for tests that issue a high number of requests and
+ * are limited by the simplified virtio test driver's descriptor tracking,
+ * which decrements num_free but never increments it back.
+ *
+ * This is only safe for synchronous test code where requests are
+ * sent and completed before the next request is issued. Do not use
+ * with asynchronous code where multiple requests may be in-flight.
+ *
+ * Note: This only resets the available descriptor pool (free_head,
+ * num_free). The used ring position (last_used_idx) is NOT reset
+ * and should continue to track consumed responses across iterations.
+ */
+void qvirtqueue_reset_pool(QVirtQueue *vq)
+{
+    vq->free_head = 0;
+    vq->num_free = vq->size;
+}
+
 void qvirtqueue_set_used_event(QTestState *qts, QVirtQueue *vq, uint16_t idx)
 {
     g_assert(vq->event);
diff --git a/tests/qtest/libqos/virtio.h b/tests/qtest/libqos/virtio.h
index e238f1726f..f17be0b9b6 100644
--- a/tests/qtest/libqos/virtio.h
+++ b/tests/qtest/libqos/virtio.h
@@ -150,6 +150,8 @@ void qvirtqueue_kick(QTestState *qts, QVirtioDevice *d, QVirtQueue *vq,
 bool qvirtqueue_get_buf(QTestState *qts, QVirtQueue *vq, uint32_t *desc_idx,
                         uint32_t *len);
 
+void qvirtqueue_reset_pool(QVirtQueue *vq);
+
 void qvirtqueue_set_used_event(QTestState *qts, QVirtQueue *vq, uint16_t idx);
 
 void qvirtio_start_device(QVirtioDevice *vdev);
-- 
2.47.3



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

* [PATCH 6/6] tests/9pfs: add deep absolute path test
  2026-05-18 17:40 [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
                   ` (4 preceding siblings ...)
  2026-05-18 17:36 ` [PATCH 5/6] tests/qtest/libqos: add qvirtqueue_reset_pool() for descriptor pool reset Christian Schoenebeck
@ 2026-05-18 17:36 ` Christian Schoenebeck
  2026-05-27 14:35 ` [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
  6 siblings, 0 replies; 9+ messages in thread
From: Christian Schoenebeck @ 2026-05-18 17:36 UTC (permalink / raw)
  To: qemu-devel, qemu-stable; +Cc: Greg Kurz, Jia Jia

Add fs_deep_absolute_path test that creates a deep directory
structure with an absolute path length exceeding 16-bit range
(i.e. >65536) to verify the previous buffer overflow fix.

This is a slow test (may take several seconds) and therefore
registered as "slow" test and not running by default.

Use -m slow to run this test.

Link: https://gitlab.com/qemu-project/qemu/-/issues/3358
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 tests/qtest/virtio-9p-test.c | 69 ++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index ac38ccf595..1c69d41e33 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -14,6 +14,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/module.h"
+#include "libqos/virtio.h"
 #include "libqos/virtio-9p-client.h"
 
 #define twalk(...) v9fs_twalk((TWalkOpt) __VA_ARGS__)
@@ -752,6 +753,72 @@ static void fs_use_after_unlink(void *obj, void *data,
     g_assert_cmpint(attr.size, ==, 2001);
 }
 
+/* https://gitlab.com/qemu-project/qemu/-/issues/3358 */
+static void fs_deep_absolute_path(void *obj, void *data,
+                                  QGuestAllocator *t_alloc)
+{
+    QVirtio9P *v9p = obj;
+    v9fs_set_allocator(t_alloc);
+
+    if (!g_test_slow()) {
+        g_test_skip("This is a slow test, run with -m slow");
+        return;
+    }
+
+    GString *path = g_string_new("/");
+    char name[256];
+    uint32_t current_fid = 0;
+
+    tattach({ .client = v9p });
+
+    /* Create deep directory structure until absolute path length
+     * exceeds 16-bit range.
+     */
+    while (path->len <= 65536) {
+        /* use 255-byte name (NAME_MAX) to reduce iterations to ~257 */
+        memset(name, 'A', 255);
+        name[255] = '\0';
+
+        /* create the directory relative to current FID */
+        tmkdir({
+            .client = v9p,
+            .dfid = current_fid,
+            .name = name
+        });
+
+        /* just for locally tracking the current path length */
+        g_string_append(path, name);
+        g_string_append(path, "/");
+
+        /* acquire new FID for the newly created directory */
+        char *wnames[] = { name };
+        current_fid = twalk({
+            .client = v9p,
+            .fid = current_fid,
+            .nwname = 1,
+            .wnames = wnames
+        }).newfid;
+
+        /* Reset descriptor pool to avoid exhaustion. The simplified
+         * virtio test driver does never free descriptors back to the pool
+         * after use, so we must manually reset it for the required high
+         * amount of 9p requests here.
+         */
+        qvirtqueue_reset_pool(v9p->vq);
+    }
+
+    /* check if the deepest directory is accessible */
+    v9fs_attr attr = {};
+    tgetattr({
+        .client = v9p,
+        .fid = current_fid,
+        .request_mask = P9_GETATTR_BASIC,
+        .rgetattr.attr = &attr
+    });
+
+    g_string_free(path, TRUE);
+}
+
 static void cleanup_9p_local_driver(void *data)
 {
     /* remove previously created test dir when test is completed */
@@ -819,6 +886,8 @@ static void register_virtio_9p_test(void)
                  &opts);
     qos_add_test("local/use_after_unlink", "virtio-9p", fs_use_after_unlink,
                  &opts);
+    qos_add_test("local/deep_absolute_path", "virtio-9p",
+                 fs_deep_absolute_path, &opts);
 }
 
 libqos_init(register_virtio_9p_test);
-- 
2.47.3



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

* [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow
@ 2026-05-18 17:40 Christian Schoenebeck
  2026-05-18 17:35 ` [PATCH 1/6] hw/9pfs: add NULL check in v9fs_path_is_ancestor() Christian Schoenebeck
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Christian Schoenebeck @ 2026-05-18 17:40 UTC (permalink / raw)
  To: qemu-devel, qemu-stable
  Cc: Greg Kurz, Jia Jia, Fabiano Rosas, Laurent Vivier, Paolo Bonzini

This series fixes a heap buffer overflow vulnerability in the 9pfs local
backend. The vulnerability occurs when handling paths exceeding 65536 bytes,
due to the V9fsPath.size field being limited to 16 bits.

The fix consists of:

- Changing V9fsPath.size from uint16_t to size_t.
- Converting v9fs_path_sprintf() to return int for error handling.
- Adding error propagation through all path manipulation functions.

Invididual Patches:

- Patch 1 is just an additional defensive patch.

- Actual fixes are patches 2..4, where patch 2..3 are prepatory, and
  patch 4 is the actual behaviour fix.

- Patch 5 adds a reset function to the virtio test client for the new
  test to work.

- Patch 6 adds a new test to guard this buffer overflow issue.
  It must be enabled explicitly by -m slow for it to run.
  
More details about this issue:
https://gitlab.com/qemu-project/qemu/-/issues/3358

Christian Schoenebeck (6):
  hw/9pfs: add NULL check in v9fs_path_is_ancestor()
  hw/9pfs: change V9fsPath.size to size_t and v9fs_path_sprintf() return
    type
  hw/9pfs: add error handling to v9fs_fix_path()
  hw/9pfs: let callers of v9fs_path_sprintf() and v9fs_fix_path() handle
    errors
  tests/qtest/libqos: add qvirtqueue_reset_pool() for descriptor pool
    reset
  tests/9pfs: add deep absolute path test

 fsdev/file-op-9p.h           |  2 +-
 hw/9pfs/9p-local.c           | 23 ++++++++----
 hw/9pfs/9p.c                 | 41 +++++++++++++++------
 hw/9pfs/9p.h                 |  4 +--
 tests/qtest/libqos/virtio.c  | 23 ++++++++++++
 tests/qtest/libqos/virtio.h  |  2 ++
 tests/qtest/virtio-9p-test.c | 69 ++++++++++++++++++++++++++++++++++++
 7 files changed, 144 insertions(+), 20 deletions(-)

-- 
2.47.3



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

* Re: [PATCH 5/6] tests/qtest/libqos: add qvirtqueue_reset_pool() for descriptor pool reset
  2026-05-18 17:36 ` [PATCH 5/6] tests/qtest/libqos: add qvirtqueue_reset_pool() for descriptor pool reset Christian Schoenebeck
@ 2026-05-19  7:51   ` Fabiano Rosas
  0 siblings, 0 replies; 9+ messages in thread
From: Fabiano Rosas @ 2026-05-19  7:51 UTC (permalink / raw)
  To: Christian Schoenebeck, qemu-devel, qemu-stable
  Cc: Greg Kurz, Jia Jia, Laurent Vivier, Paolo Bonzini

Christian Schoenebeck <qemu_oss@crudebyte.com> writes:

> Add a function to reset the virtqueue descriptor pool state without
> reinitializing the device. This is useful for tests that issue a high
> number of requests and are limited by the simplified virtio test
> driver's descriptor tracking, which decrements num_free but never
> increments it back.
>
> The function is safe for synchronous test code where requests are
> sent and completed before the next request is issued.
>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

Acked-by: Fabiano Rosas <farosas@suse.de>

> ---
>  tests/qtest/libqos/virtio.c | 23 +++++++++++++++++++++++
>  tests/qtest/libqos/virtio.h |  2 ++
>  2 files changed, 25 insertions(+)
>
> diff --git a/tests/qtest/libqos/virtio.c b/tests/qtest/libqos/virtio.c
> index 010ff40834..ccbb325222 100644
> --- a/tests/qtest/libqos/virtio.c
> +++ b/tests/qtest/libqos/virtio.c
> @@ -464,6 +464,29 @@ bool qvirtqueue_get_buf(QTestState *qts, QVirtQueue *vq, uint32_t *desc_idx,
>      return true;
>  }
>  
> +/*
> + * qvirtqueue_reset_pool:
> + * @vq: The virtqueue to reset
> + *
> + * Reset the descriptor pool state without reinitializing the device.
> + * This is useful for tests that issue a high number of requests and
> + * are limited by the simplified virtio test driver's descriptor tracking,
> + * which decrements num_free but never increments it back.
> + *
> + * This is only safe for synchronous test code where requests are
> + * sent and completed before the next request is issued. Do not use
> + * with asynchronous code where multiple requests may be in-flight.
> + *
> + * Note: This only resets the available descriptor pool (free_head,
> + * num_free). The used ring position (last_used_idx) is NOT reset
> + * and should continue to track consumed responses across iterations.
> + */
> +void qvirtqueue_reset_pool(QVirtQueue *vq)
> +{
> +    vq->free_head = 0;
> +    vq->num_free = vq->size;
> +}
> +
>  void qvirtqueue_set_used_event(QTestState *qts, QVirtQueue *vq, uint16_t idx)
>  {
>      g_assert(vq->event);
> diff --git a/tests/qtest/libqos/virtio.h b/tests/qtest/libqos/virtio.h
> index e238f1726f..f17be0b9b6 100644
> --- a/tests/qtest/libqos/virtio.h
> +++ b/tests/qtest/libqos/virtio.h
> @@ -150,6 +150,8 @@ void qvirtqueue_kick(QTestState *qts, QVirtioDevice *d, QVirtQueue *vq,
>  bool qvirtqueue_get_buf(QTestState *qts, QVirtQueue *vq, uint32_t *desc_idx,
>                          uint32_t *len);
>  
> +void qvirtqueue_reset_pool(QVirtQueue *vq);
> +
>  void qvirtqueue_set_used_event(QTestState *qts, QVirtQueue *vq, uint16_t idx);
>  
>  void qvirtio_start_device(QVirtioDevice *vdev);


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

* Re: [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow
  2026-05-18 17:40 [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
                   ` (5 preceding siblings ...)
  2026-05-18 17:36 ` [PATCH 6/6] tests/9pfs: add deep absolute path test Christian Schoenebeck
@ 2026-05-27 14:35 ` Christian Schoenebeck
  6 siblings, 0 replies; 9+ messages in thread
From: Christian Schoenebeck @ 2026-05-27 14:35 UTC (permalink / raw)
  To: qemu-devel, qemu-stable, Wang Jihe
  Cc: Greg Kurz, Jia Jia, Fabiano Rosas, Laurent Vivier, Paolo Bonzini

On Monday, 18 May 2026 19:40:34 CEST Christian Schoenebeck wrote:
> This series fixes a heap buffer overflow vulnerability in the 9pfs local
> backend. The vulnerability occurs when handling paths exceeding 65536 bytes,
> due to the V9fsPath.size field being limited to 16 bits.
> 
> The fix consists of:
> 
> - Changing V9fsPath.size from uint16_t to size_t.
> - Converting v9fs_path_sprintf() to return int for error handling.
> - Adding error propagation through all path manipulation functions.
> 
> Invididual Patches:
> 
> - Patch 1 is just an additional defensive patch.
> 
> - Actual fixes are patches 2..4, where patch 2..3 are prepatory, and
>   patch 4 is the actual behaviour fix.
> 
> - Patch 5 adds a reset function to the virtio test client for the new
>   test to work.
> 
> - Patch 6 adds a new test to guard this buffer overflow issue.
>   It must be enabled explicitly by -m slow for it to run.
> 
> More details about this issue:
> https://gitlab.com/qemu-project/qemu/-/issues/3358

Queued on 9p.next:
https://github.com/cschoenebeck/qemu/commits/9p.next

Thanks!

/Christian
 
> Christian Schoenebeck (6):
>   hw/9pfs: add NULL check in v9fs_path_is_ancestor()
>   hw/9pfs: change V9fsPath.size to size_t and v9fs_path_sprintf() return
>     type
>   hw/9pfs: add error handling to v9fs_fix_path()
>   hw/9pfs: let callers of v9fs_path_sprintf() and v9fs_fix_path() handle
>     errors
>   tests/qtest/libqos: add qvirtqueue_reset_pool() for descriptor pool
>     reset
>   tests/9pfs: add deep absolute path test
> 
>  fsdev/file-op-9p.h           |  2 +-
>  hw/9pfs/9p-local.c           | 23 ++++++++----
>  hw/9pfs/9p.c                 | 41 +++++++++++++++------
>  hw/9pfs/9p.h                 |  4 +--
>  tests/qtest/libqos/virtio.c  | 23 ++++++++++++
>  tests/qtest/libqos/virtio.h  |  2 ++
>  tests/qtest/virtio-9p-test.c | 69 ++++++++++++++++++++++++++++++++++++
>  7 files changed, 144 insertions(+), 20 deletions(-)




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

end of thread, other threads:[~2026-05-27 14:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-18 17:40 [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck
2026-05-18 17:35 ` [PATCH 1/6] hw/9pfs: add NULL check in v9fs_path_is_ancestor() Christian Schoenebeck
2026-05-18 17:35 ` [PATCH 2/6] hw/9pfs: change V9fsPath.size to size_t and v9fs_path_sprintf() return type Christian Schoenebeck
2026-05-18 17:35 ` [PATCH 3/6] hw/9pfs: add error handling to v9fs_fix_path() Christian Schoenebeck
2026-05-18 17:35 ` [PATCH 4/6] hw/9pfs: let callers of v9fs_path_sprintf() and v9fs_fix_path() handle errors Christian Schoenebeck
2026-05-18 17:36 ` [PATCH 5/6] tests/qtest/libqos: add qvirtqueue_reset_pool() for descriptor pool reset Christian Schoenebeck
2026-05-19  7:51   ` Fabiano Rosas
2026-05-18 17:36 ` [PATCH 6/6] tests/9pfs: add deep absolute path test Christian Schoenebeck
2026-05-27 14:35 ` [PATCH 0/6] 9pfs: fix V9fsPath heap buffer overflow Christian Schoenebeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.