qemu-stable
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] 9pfs: Fix segfault in do_readdir_many caused by struct di


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v3] 9pfs: Fix segfault in do_readdir_many caused by struct dirent overread
Date: Fri, 4 Feb 2022 14:55:45 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.5.1

On 4/2/22 06:06, Vitaly Chikunov wrote:
`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, 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>
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Tested on x86-64 Linux again.

Changes from v2:
- Make it work with a simulated dirent where d_reclen is 0, which was
   caused abort in readdir qos-test, by using fallback at runtime.

  hw/9pfs/codir.c      |  3 +--
  include/qemu/osdep.h | 13 +++++++++++++
  util/osdep.c         | 18 ++++++++++++++++++
  3 files changed, 32 insertions(+), 2 deletions(-)

+struct dirent *
+qemu_dirent_dup(struct dirent *dent)
+{
+    size_t sz = 0;
+#if defined _DIRENT_HAVE_D_RECLEN
+    /* Avoid use of strlen() if there's d_reclen. */
+    sz = dent->d_reclen;
+#endif
+    if (sz == 0) {

If _DIRENT_HAVE_D_RECLEN is defined, this case is unlikely...

+        /* Fallback to the most portable way. */
+        sz = offsetof(struct dirent, d_name) +
+                      strlen(dent->d_name) + 1;
+    }
+    struct dirent *dst = g_malloc(sz);
+    return memcpy(dst, dent, sz);
+}

What about this?

struct dirent *
qemu_dirent_dup(struct dirent *dent)
{
    size_t sz;

#if defined _DIRENT_HAVE_D_RECLEN
    /* Avoid use of strlen() if there's d_reclen. */
    sz = dent->d_reclen;
#else
    /* Fallback to the most portable way. */
    sz = offsetof(struct dirent, d_name) +
                  strlen(dent->d_name) + 1;
#endif

    return g_memdup(dent, sz);
}



reply via email to

[Prev in Thread] Current Thread [Next in Thread]