mirror of
https://github.com/qemu/qemu.git
synced 2026-07-08 17:46:10 +00:00
Merge tag 'pull-9p-20260601' of https://github.com/cschoenebeck/qemu into staging
9pfs changes: - fix V9fsPath heap buffer overflow (gitlab #3358) - fix missing rename lock in v9fs_co_readdir_many (CVE-2026-48004) # -----BEGIN PGP SIGNATURE----- # # iQJLBAABCgA1FiEEltjREM96+AhPiFkBNMK1h2Wkc5UFAmodVmQXHHFlbXVfb3Nz # QGNydWRlYnl0ZS5jb20ACgkQNMK1h2Wkc5WQEA//VgSO/pQrK+6N0zKgPGCsNmY+ # gPqZMjZDnMSCHmmvEkQzdObbkBSJR8yrXnJm4MBkwx0CiVWL0AuGEpdlXmFkIrXR # 7w2aW12a6G9KStFmQzMShx5VtbQHECkxWSoGwEvNYKysgOC1rubokxQiW/FZMexr # SFkBuXlCdH5HEQHisidbeQOLPEzpZUqsF+6ex3cyBtTBBzE3Bm3e0EKEFsNw7Pod # 3tjGmZpc9vU0EA/tFpK21nOk4k6sVLws7QugsG75YbFdsMW3XYb2curBDOn8zJIp # Vc2685U8i1HKE349t8zBrrwXxZcI0vcV1S4tDKsexHxhBkLhNxWurERsX3XCV9pp # hygASyPULI25Ckvv4lvXG1tmGWcuvyJ0IKSH4VsOLVGAuckB+k9pUqVHpe/tzl4T # tL4jMISi63ud0VxZYdtmvvxgevdxa7dkM/0dbSl3r2De8KErPPTPxoOJR5IwbBca # kuyYHImv/sgV6O3z0bE3RgpYSDNKmzdagmZyXbe4JKchw/sHAsi5+2X23ow3YkQI # m6mJefb39HrQe6uMo5NKhGnv7x3kByvTi9eiIU/xdxaHRx+Q3o801u78jDcHPn4h # 8amzgjWtHxVngNdQ7NR8qExu+2iepw3LtVpz5sfqfGwwn4/CjMegV+/Vf4iZ5eTH # 22+c2sZfepyd2MqOL/I= # =vJVW # -----END PGP SIGNATURE----- # gpg: Signature made Mon 01 Jun 2026 05:52:36 EDT # gpg: using RSA key 96D8D110CF7AF8084F88590134C2B58765A47395 # gpg: issuer "qemu_oss@crudebyte.com" # gpg: Good signature from "Christian Schoenebeck <qemu_oss@crudebyte.com>" [unknown] # gpg: Note: This key has expired! # Primary key fingerprint: ECAB 1A45 4014 1413 BA38 4926 30DB 47C3 A012 D5F4 # Subkey fingerprint: 96D8 D110 CF7A F808 4F88 5901 34C2 B587 65A4 7395 * tag 'pull-9p-20260601' of https://github.com/cschoenebeck/qemu: 9pfs: fix missing rename lock in v9fs_co_readdir_many (CVE-2026-48004) tests/9pfs: add deep absolute path test tests/qtest/libqos: add qvirtqueue_reset_pool() for descriptor pool reset hw/9pfs: let callers of v9fs_path_sprintf() and v9fs_fix_path() handle errors hw/9pfs: add error handling to v9fs_fix_path() hw/9pfs: change V9fsPath.size to size_t and v9fs_path_sprintf() return type hw/9pfs: add NULL check in v9fs_path_is_ancestor() Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This commit is contained in:
@@ -112,7 +112,7 @@ struct FsContext {
|
||||
};
|
||||
|
||||
struct V9fsPath {
|
||||
uint16_t size;
|
||||
size_t size;
|
||||
char *data;
|
||||
};
|
||||
P9ARRAY_DECLARE_TYPE(V9fsPath);
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
41
hw/9pfs/9p.c
41
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:
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user