aboutsummaryrefslogtreecommitdiff
path: root/hw
diff options
context:
space:
mode:
authorVitaly Chikunov <vt@altlinux.org>2022-02-16 21:18:21 +0300
committerChristian Schoenebeck <qemu_oss@crudebyte.com>2022-02-17 16:57:58 +0100
commite64e27d5cb103b7764f1a05b6eda7e7fedd517c5 (patch)
tree6f349f7536e1b2e9ee569f7363b9802e82fb8e20 /hw
parent494fbbd3ed46a14ef0924651c058b9b0dcb4a7b4 (diff)
9pfs: Fix segfault in do_readdir_many caused by struct dirent overread
`struct dirent' returned from readdir(3) could be shorter (or longer) than `sizeof(struct dirent)', thus memcpy of sizeof length will overread into unallocated page causing SIGSEGV. Example stack trace: #0 0x00005555559ebeed v9fs_co_readdir_many (/usr/bin/qemu-system-x86_64 + 0x497eed) #1 0x00005555559ec2e9 v9fs_readdir (/usr/bin/qemu-system-x86_64 + 0x4982e9) #2 0x0000555555eb7983 coroutine_trampoline (/usr/bin/qemu-system-x86_64 + 0x963983) #3 0x00007ffff73e0be0 n/a (n/a + 0x0) While fixing this, provide a helper for any future `struct dirent' cloning. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/841 Cc: qemu-stable@nongnu.org Co-authored-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Dmitry V. Levin <ldv@altlinux.org> Signed-off-by: Vitaly Chikunov <vt@altlinux.org> Tested-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Acked-by: Greg Kurz <groug@kaod.org> Tested-by: Vitaly Chikunov <vt@altlinux.org> Message-Id: <20220216181821.3481527-1-vt@altlinux.org> [C.S. - Fix typo in source comment. ] Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Diffstat (limited to 'hw')
-rw-r--r--hw/9pfs/9p-synth.c18
-rw-r--r--hw/9pfs/9p-synth.h5
-rw-r--r--hw/9pfs/codir.c3
3 files changed, 21 insertions, 5 deletions
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index b38088e066..7a7cd5c5ba 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -182,7 +182,12 @@ static int synth_opendir(FsContext *ctx,
V9fsSynthOpenState *synth_open;
V9fsSynthNode *node = *(V9fsSynthNode **)fs_path->data;
- synth_open = g_malloc(sizeof(*synth_open));
+ /*
+ * V9fsSynthOpenState contains 'struct dirent' which have OS-specific
+ * properties, thus it's zero cleared on allocation here and below
+ * in synth_open.
+ */
+ synth_open = g_new0(V9fsSynthOpenState, 1);
synth_open->node = node;
node->open_count++;
fs->private = synth_open;
@@ -220,7 +225,14 @@ static void synth_rewinddir(FsContext *ctx, V9fsFidOpenState *fs)
static void synth_direntry(V9fsSynthNode *node,
struct dirent *entry, off_t off)
{
- strcpy(entry->d_name, node->name);
+ size_t sz = strlen(node->name) + 1;
+ /*
+ * 'entry' is always inside of V9fsSynthOpenState which have NAME_MAX
+ * back padding. Ensure we do not overflow it.
+ */
+ g_assert(sizeof(struct dirent) + NAME_MAX >=
+ offsetof(struct dirent, d_name) + sz);
+ memcpy(entry->d_name, node->name, sz);
entry->d_ino = node->attr->inode;
entry->d_off = off + 1;
}
@@ -266,7 +278,7 @@ static int synth_open(FsContext *ctx, V9fsPath *fs_path,
V9fsSynthOpenState *synth_open;
V9fsSynthNode *node = *(V9fsSynthNode **)fs_path->data;
- synth_open = g_malloc(sizeof(*synth_open));
+ synth_open = g_new0(V9fsSynthOpenState, 1);
synth_open->node = node;
node->open_count++;
fs->private = synth_open;
diff --git a/hw/9pfs/9p-synth.h b/hw/9pfs/9p-synth.h
index 036d7e4a5b..eeb246f377 100644
--- a/hw/9pfs/9p-synth.h
+++ b/hw/9pfs/9p-synth.h
@@ -41,6 +41,11 @@ typedef struct V9fsSynthOpenState {
off_t offset;
V9fsSynthNode *node;
struct dirent dent;
+ /*
+ * Ensure there is enough space for 'dent' above, some systems have a
+ * d_name size of just 1, which would cause a buffer overrun.
+ */
+ char dent_trailing_space[NAME_MAX];
} V9fsSynthOpenState;
int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index 032cce04c4..c0873bde16 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -143,8 +143,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp,
} else {
e = e->next = g_malloc0(sizeof(V9fsDirEnt));
}
- e->dent = g_malloc0(sizeof(struct dirent));
- memcpy(e->dent, dent, sizeof(struct dirent));
+ e->dent = qemu_dirent_dup(dent);
/* perform a full stat() for directory entry if requested by caller */
if (dostat) {