Memory allegedly uninitialized after ioctl(PROCMAP_QUERY)

Fix ioctl(fd, PROCMAP_QUERY, ...) so that valgrind correctly considers
memory referenced by vma_name_size and vma_name_addr members of struct
procmap_query as initialized by ioctl().

Extend ioctl syscall wrappers with needed PRE_MEM_WRITE() and
mainly POST_MEM_WRITE().  Add a testcase.

https://bugs.kde.org/show_bug.cgi?id=508328

diff --git a/.gitignore b/.gitignore
index a9cb60c965d37b0e1d9e0e6c3b10282aea614209..5542f4e4b6605dd1b6dcdef825e16f85a1ad833d 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -1193,6 +1193,7 @@
 /memcheck/tests/linux/dlclose_leak_so.so
 /memcheck/tests/linux/getregset
 /memcheck/tests/linux/ioctl-tiocsig
+/memcheck/tests/linux/ioctl_procmap_query
 /memcheck/tests/linux/lsframe1
 /memcheck/tests/linux/lsframe2
 /memcheck/tests/linux/Makefile

diff --git a/NEWS b/NEWS
index 89493bbf6a408a9eeeb48300afecf7527844058f..252c8e8ec0bac1fece3ab479a546816b7e11e6d0 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -233,6 +233,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 508030  Add several missing syscall hooks to ppc64-linux
 508093  VALGRIND_CLO_CHANGE does not update vex_control
 508145  ppc64le needs ld.so hardwire for strcmp
+508328  Memory allegedly uninitialized after ioctl(PROCMAP_QUERY)
 508154  PRE(sys_fchownat) not handling VKI_AT_FDCWD
 508638  Self-hosting not working on FreeBSD
 508777  amd64-linux: add minimal scalar test

diff --git a/configure.ac b/configure.ac
index 869487db4d01f2d7326c0c2180468878493251f5..09a352a707e8e97d3a475a6645ecd762d82a733e 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -2173,6 +2173,21 @@ AC_MSG_RESULT([no])
 
 AM_CONDITIONAL(HAVE_NR_IO_PGETEVENTS, [test x$ac_have_nr_io_pgetevents = xyes])
 
+AC_MSG_CHECKING([for PROCMAP_QUERY])
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+#include <linux/fs.h>
+]], [[
+return PROCMAP_QUERY
+]])], [
+ac_have_procmap_query=yes
+AC_MSG_RESULT([yes])
+], [
+ac_have_procmap_query=no
+AC_MSG_RESULT([no])
+])
+
+AM_CONDITIONAL(HAVE_PROCMAP_QUERY, [test x$ac_have_procmap_query = xyes])
+
 #----------------------------------------------------------------------------
 # Checking for supported compiler flags.
 #----------------------------------------------------------------------------

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 4db8743dd01999d0693bbf94620e207c6a2cb0ca..8bf0bab56d7249968565a3815afb065a9558de3a 100644 (file)
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -10826,6 +10826,26 @@ PRE(sys_ioctl)
    case VKI_EVIOCGRAB:
        /* This just takes an int argument. */
        break;
+   case VKI_PROCMAP_QUERY: {
+      /* https://www.kernel.org/doc/html/latest/filesystems/proc.html */
+      /* linux source: include/uapi/linux/fs.h:561 */
+      struct vki_procmap_query *pq =
+         (struct vki_procmap_query *)(Addr)ARG3;
+      if (!ML_(safe_to_deref) (pq, sizeof(struct vki_procmap_query)))
+         break;
+      PRE_FIELD_READ("ioctl(PROCMAP_QUERY).size", pq->size);
+      PRE_FIELD_READ("ioctl(PROCMAP_QUERY).query_flags", pq->query_flags);
+      PRE_FIELD_READ("ioctl(PROCMAP_QUERY).query_addr", pq->query_addr);
+      PRE_FIELD_READ("ioctl(PROCMAP_QUERY).vma_name_size", pq->vma_name_size);
+      PRE_FIELD_READ("ioctl(PROCMAP_QUERY).vma_name_addr", pq->vma_name_addr);
+      PRE_FIELD_READ("ioctl(PROCMAP_QUERY).build_id_size", pq->build_id_size);
+      PRE_FIELD_READ("ioctl(PROCMAP_QUERY).build_id_addr", pq->build_id_addr);
+      if (pq->vma_name_size > 0)
+         PRE_MEM_WRITE("ioctl(PROCMAP_QUERY)", (Addr)pq->vma_name_addr, pq->vma_name_size);
+      if (pq->build_id_size > 0)
+         PRE_MEM_WRITE("ioctl(PROCMAP_QUERY)", (Addr)pq->build_id_addr, pq->build_id_size);
+      break;
+   }
 
    default:
       /* EVIOC* are variable length and return size written on success */
@@ -12984,6 +13004,20 @@ POST(sys_ioctl)
    case VKI_PTP_ENABLE_PPS:
    case VKI_PTP_PIN_SETFUNC:
       break;
+   case VKI_PROCMAP_QUERY: {
+      /* https://www.kernel.org/doc/html/latest/filesystems/proc.html */
+      /* linux source: include/uapi/linux/fs.h:561 */
+      struct vki_procmap_query *pq =
+         (struct vki_procmap_query *)(Addr)ARG3;
+      if (pq->vma_name_size > 0)
+         POST_MEM_WRITE(pq->vma_name_addr, pq->vma_name_size);
+      if (pq->build_id_size > 0)
+         POST_MEM_WRITE(pq->build_id_addr, pq->build_id_size);
+      /* assume everything is written/defined with POST_MEM_WRITE */
+      /* instead of doing individual POST_FIELD_WRITEs */
+      POST_MEM_WRITE((Addr)pq, pq->size);
+      break;
+   }
 
    default:
       /* EVIOC* are variable length and return size written on success */

diff --git a/include/vki/vki-linux.h b/include/vki/vki-linux.h
index 3f9272f4d18ed0a897ee8841150c42de88c2a41f..59c4d57c8af6c68262fca7c9a3af15d48794d6d8 100644 (file)
--- a/include/vki/vki-linux.h
+++ b/include/vki/vki-linux.h
@@ -3900,6 +3900,30 @@ struct vki_ion_custom_data {
 #define VKI_ION_IOC_CUSTOM \
    _VKI_IOWR(VKI_ION_IOC_MAGIC, 6, struct vki_ion_custom_data)
 
+struct vki_procmap_query {
+    __vki_u64 size;
+    __vki_u64 query_flags;              /* in */
+    __vki_u64 query_addr;               /* in */
+    __vki_u64 vma_start;                /* out */
+    __vki_u64 vma_end;                  /* out */
+    __vki_u64 vma_flags;                /* out */
+    __vki_u64 vma_page_size;            /* out */
+    __vki_u64 vma_offset;               /* out */
+    __vki_u64 inode;                    /* out */
+    __vki_u32 dev_major;                /* out */
+    __vki_u32 dev_minor;                /* out */
+    __vki_u32 vma_name_size;            /* in/out */
+    __vki_u32 build_id_size;            /* in/out */
+    __vki_u64 vma_name_addr;            /* in */
+    __vki_u64 build_id_addr;            /* in */
+};
+
+// linux/fs.h
+#define VKI_PROCFS_IOCTL_MAGIC 'f'
+
+#define VKI_PROCMAP_QUERY \
+   _VKI_IOWR(VKI_PROCFS_IOCTL_MAGIC, 17, struct vki_procmap_query)
+
 //----------------------------------------------------------------------
 // From include/uapi/linux/sync_file.h 6.10.3
 //----------------------------------------------------------------------

diff --git a/memcheck/tests/linux/Makefile.am b/memcheck/tests/linux/Makefile.am
index e28866fc18d82a9badb1b4f05f55103efd352624..b9f273be8eb74b15756d463f9b81541128c2cac5 100644 (file)
--- a/memcheck/tests/linux/Makefile.am
+++ b/memcheck/tests/linux/Makefile.am
@@ -1,7 +1,8 @@
 
 include $(top_srcdir)/Makefile.tool-tests.am
 
-dist_noinst_SCRIPTS = filter_stderr
+dist_noinst_SCRIPTS = filter_stderr \
+       filter_ioctl_procmap_query
 
 EXTRA_DIST = \
        aligned_alloc.vgtest aligned_alloc.stderr.exp \
@@ -19,6 +20,9 @@ EXTRA_DIST = \
            dlclose_leak-no-keep.vgtest \
        dlclose_leak.stderr.exp dlclose_leak.stdout.exp \
            dlclose_leak.vgtest \
+       ioctl_procmap_query.stderr.exp \
+               ioctl_procmap_query.stdout.exp \
+               ioctl_procmap_query.vgtest \
        ioctl-tiocsig.vgtest ioctl-tiocsig.stderr.exp \
        lsframe1.vgtest lsframe1.stdout.exp lsframe1.stderr.exp \
        lsframe2.vgtest lsframe2.stdout.exp lsframe2.stderr.exp \
@@ -70,6 +74,10 @@ check_PROGRAMS = \
        enomem \
        memalign
 
+if HAVE_PROCMAP_QUERY
+check_PROGRAMS += ioctl_procmap_query
+endif
+
 if HAVE_OPENSSL
 check_PROGRAMS += bug480706
 endif

diff --git a/memcheck/tests/linux/filter_ioctl_procmap_query b/memcheck/tests/linux/filter_ioctl_procmap_query
new file mode 100755 (executable)
index 0000000..bdd7641
--- /dev/null
+++ b/memcheck/tests/linux/filter_ioctl_procmap_query
@@ -0,0 +1,6 @@
+#! /bin/sh
+
+# drop the 3rd (last) line of the output, since it's a
+# binary blob (build_id) which needs to be printed but
+# not checked
+/usr/bin/head -2

diff --git a/memcheck/tests/linux/ioctl_procmap_query.c b/memcheck/tests/linux/ioctl_procmap_query.c
new file mode 100644 (file)
index 0000000..396cf3d
--- /dev/null
+++ b/memcheck/tests/linux/ioctl_procmap_query.c
@@ -0,0 +1,34 @@
+#include <stdio.h>
+#include <stdint.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <linux/fs.h>
+#include <unistd.h>
+#include <string.h>
+
+int main(int argc, char** argv)
+{
+       char name[256];
+        char buildid[256];
+        char cwd[256];
+        getcwd(cwd, sizeof(cwd));
+
+       struct procmap_query pq = {
+               .size = sizeof(pq), .query_addr = (uintptr_t)main,
+               .vma_name_size = 256, .vma_name_addr = (uintptr_t)name,
+                .build_id_size = 256, .build_id_addr = (uintptr_t)buildid
+       };
+       int fd = open("/proc/self/maps", O_RDONLY);
+       ioctl(fd, PROCMAP_QUERY, &pq);
+        // print name but strip off the PWD prefix so that
+        // we always get a known output we can easily check
+        puts(name + strlen(cwd) + 1);
+        // buildid is a binary blob, not NUL terminated C string
+        buildid[pq.build_id_size-1] = '\0';
+        // make sure that kernel returned some reasonable build_id_size
+        if (pq.build_id_size > 0 && pq.build_id_size < 256)
+            puts("OK");
+        // print the buildid so that the bug can trigger
+        puts(buildid);
+}
+

diff --git a/memcheck/tests/linux/ioctl_procmap_query.stderr.exp b/memcheck/tests/linux/ioctl_procmap_query.stderr.exp
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/memcheck/tests/linux/ioctl_procmap_query.stdout.exp b/memcheck/tests/linux/ioctl_procmap_query.stdout.exp
new file mode 100644 (file)
index 0000000..280e91a
--- /dev/null
+++ b/memcheck/tests/linux/ioctl_procmap_query.stdout.exp
@@ -0,0 +1,2 @@
+ioctl_procmap_query
+OK

diff --git a/memcheck/tests/linux/ioctl_procmap_query.vgtest b/memcheck/tests/linux/ioctl_procmap_query.vgtest
new file mode 100644 (file)
index 0000000..8d12e07
--- /dev/null
+++ b/memcheck/tests/linux/ioctl_procmap_query.vgtest
@@ -0,0 +1,4 @@
+prog: ioctl_procmap_query
+vgopts: -q
+stdout_filter: filter_ioctl_procmap_query
+prereq: test -x ioctl_procmap_query