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-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 e2713b9eee..b4314d2549 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) @@ -241,6 +249,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; @@ -1406,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) @@ -3312,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); @@ -3338,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: @@ -3435,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: 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, diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c index bce7dd96e9..5568399343 100644 --- a/hw/9pfs/codir.c +++ b/hw/9pfs/codir.c @@ -220,13 +220,16 @@ int coroutine_fn v9fs_co_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp, bool dostat) { int err = 0; + V9fsState *s = pdu->s; if (v9fs_request_cancelled(pdu)) { return -EINTR; } + v9fs_path_read_lock(s); v9fs_co_run_in_worker({ err = do_readdir_many(pdu, fidp, entries, offset, maxsize, dostat); }); + v9fs_path_unlock(s); return err; } 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); 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);