diff mbox series

[v3,08/12] linux-user: factor out reading of /proc/self/maps

Message ID 20200403191150.863-9-alex.bennee@linaro.org
State Superseded
Headers show
Series a selection of random fixes | expand

Commit Message

Alex Bennée April 3, 2020, 7:11 p.m. UTC 
Unfortunately reading /proc/self/maps is still considered the gold
standard for a process finding out about it's own memory layout. As we
will want this data in other contexts soon factor out the code to read
and parse the data. Rather than just blindly copying the existing
sscanf based code we use a more modern glib version of the parsing
code to make a more general purpose map structure.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>


---
v2
  - s/uint64_t/unsigned long/ for MapInfo structure
  - limit to 6 fields and strip leading spaces
---
 include/qemu/selfmap.h | 44 ++++++++++++++++++++++++
 linux-user/syscall.c   | 58 +++++++++++++++----------------
 util/selfmap.c         | 77 ++++++++++++++++++++++++++++++++++++++++++
 util/Makefile.objs     |  1 +
 4 files changed, 150 insertions(+), 30 deletions(-)
 create mode 100644 include/qemu/selfmap.h
 create mode 100644 util/selfmap.c

-- 
2.20.1

Comments

Richard Henderson April 4, 2020, 12:33 a.m. UTC | #1 
On 4/3/20 12:11 PM, Alex Bennée wrote:
> +                e->is_read  = fields[1][0] == 'r' ? true : false;

> +                e->is_write = fields[1][1] == 'w' ? true : false;

> +                e->is_exec  = fields[1][2] == 'x' ? true : false;

> +                e->is_priv  = fields[1][3] == 'p' ? true : false;


Drop the redundant ? true : false.  That is of course the result of the boolean
operation.

> +                errors += qemu_strtoi(fields[4], NULL, 10, &e->inode);


The root of the typedef chain for ino_t is

/usr/include/asm-generic/posix_types.h:typedef __kernel_ulong_t __kernel_ino_t;

so I think you should just go ahead and use unsigned long here too.  Or maybe
even uint64_t, because 32-bit has ino64_t, and could in fact have a Large
Number here.


r~
Alex Bennée April 6, 2020, 9:09 a.m. UTC | #2 
Richard Henderson <richard.henderson@linaro.org> writes:

> On 4/3/20 12:11 PM, Alex Bennée wrote:

>> +                e->is_read  = fields[1][0] == 'r' ? true : false;

>> +                e->is_write = fields[1][1] == 'w' ? true : false;

>> +                e->is_exec  = fields[1][2] == 'x' ? true : false;

>> +                e->is_priv  = fields[1][3] == 'p' ? true : false;

>

> Drop the redundant ? true : false.  That is of course the result of the boolean

> operation.


doh! Fortunately the compiler was smart enough to see through my idiocy...

>

>> +                errors += qemu_strtoi(fields[4], NULL, 10, &e->inode);

>

> The root of the typedef chain for ino_t is

>

> /usr/include/asm-generic/posix_types.h:typedef __kernel_ulong_t __kernel_ino_t;

>

> so I think you should just go ahead and use unsigned long here too.  Or maybe

> even uint64_t, because 32-bit has ino64_t, and could in fact have a Large

> Number here.


Will fix.

>

>

> r~



-- 
Alex Bennée
diff mbox series

Patch

diff --git a/include/qemu/selfmap.h b/include/qemu/selfmap.h
new file mode 100644
index 00000000000..302c459d695
--- /dev/null
+++ b/include/qemu/selfmap.h
@@ -0,0 +1,44 @@ 
+/*
+ * Utility functions to read our own memory map
+ *
+ * Copyright (c) 2020 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef _SELFMAP_H_
+#define _SELFMAP_H_
+
+typedef struct {
+    unsigned long start;
+    unsigned long end;
+
+    /* flags */
+    bool is_read;
+    bool is_write;
+    bool is_exec;
+    bool is_priv;
+
+    unsigned long offset;
+    gchar *dev;
+    int   inode;
+    gchar *path;
+} MapInfo;
+
+
+/**
+ * read_self_maps:
+ *
+ * Read /proc/self/maps and return a list of MapInfo structures.
+ */
+GSList *read_self_maps(void);
+
+/**
+ * free_self_maps:
+ * @info: a GSlist
+ *
+ * Free a list of MapInfo structures.
+ */
+void free_self_maps(GSList *info);
+
+#endif /* _SELFMAP_H_ */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b679bc6b136..666be20f3ff 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -117,6 +117,7 @@ 
 
 #include "qemu.h"
 #include "qemu/guest-random.h"
+#include "qemu/selfmap.h"
 #include "user/syscall-trace.h"
 #include "qapi/error.h"
 #include "fd-trans.h"
@@ -7232,45 +7233,45 @@  static int open_self_maps(void *cpu_env, int fd)
 {
     CPUState *cpu = env_cpu((CPUArchState *)cpu_env);
     TaskState *ts = cpu->opaque;
-    FILE *fp;
-    char *line = NULL;
-    size_t len = 0;
-    ssize_t read;
+    GSList *map_info = read_self_maps();
+    GSList *s;
 
-    fp = fopen("/proc/self/maps", "r");
-    if (fp == NULL) {
-        return -1;
-    }
+    for (s = map_info; s; s = g_slist_next(s)) {
+        MapInfo *e = (MapInfo *) s->data;
 
-    while ((read = getline(&line, &len, fp)) != -1) {
-        int fields, dev_maj, dev_min, inode;
-        uint64_t min, max, offset;
-        char flag_r, flag_w, flag_x, flag_p;
-        char path[512] = "";
-        fields = sscanf(line, "%"PRIx64"-%"PRIx64" %c%c%c%c %"PRIx64" %x:%x %d"
-                        " %512s", &min, &max, &flag_r, &flag_w, &flag_x,
-                        &flag_p, &offset, &dev_maj, &dev_min, &inode, path);
-
-        if ((fields < 10) || (fields > 11)) {
-            continue;
-        }
-        if (h2g_valid(min)) {
+        if (h2g_valid(e->start)) {
+            unsigned long min = e->start;
+            unsigned long max = e->end;
             int flags = page_get_flags(h2g(min));
-            max = h2g_valid(max - 1) ? max : (uintptr_t)g2h(GUEST_ADDR_MAX) + 1;
+            const char *path;
+
+            max = h2g_valid(max - 1) ?
+                max : (uintptr_t) g2h(GUEST_ADDR_MAX) + 1;
+
             if (page_check_range(h2g(min), max - min, flags) == -1) {
                 continue;
             }
+
             if (h2g(min) == ts->info->stack_limit) {
-                pstrcpy(path, sizeof(path), "      [stack]");
+                path = "      [stack]";
+            } else {
+                path = e->path;
             }
+
             dprintf(fd, TARGET_ABI_FMT_ptr "-" TARGET_ABI_FMT_ptr
-                    " %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n",
-                    h2g(min), h2g(max - 1) + 1, flag_r, flag_w,
-                    flag_x, flag_p, offset, dev_maj, dev_min, inode,
-                    path[0] ? "         " : "", path);
+                    " %c%c%c%c %08" PRIx64 " %s %d %s%s\n",
+                    h2g(min), h2g(max - 1) + 1,
+                    e->is_read ? 'r' : '-',
+                    e->is_write ? 'w' : '-',
+                    e->is_exec ? 'x' : '-',
+                    e->is_priv ? 'p' : '-',
+                    (uint64_t) e->offset, e->dev, e->inode,
+                    path ? "         " : "", path ? path : "");
         }
     }
 
+    free_self_maps(map_info);
+
 #ifdef TARGET_VSYSCALL_PAGE
     /*
      * We only support execution from the vsyscall page.
@@ -7281,9 +7282,6 @@  static int open_self_maps(void *cpu_env, int fd)
             TARGET_VSYSCALL_PAGE, TARGET_VSYSCALL_PAGE + TARGET_PAGE_SIZE);
 #endif
 
-    free(line);
-    fclose(fp);
-
     return 0;
 }
 
diff --git a/util/selfmap.c b/util/selfmap.c
new file mode 100644
index 00000000000..15d0acdb437
--- /dev/null
+++ b/util/selfmap.c
@@ -0,0 +1,77 @@ 
+/*
+ * Utility function to get QEMU's own process map
+ *
+ * Copyright (c) 2020 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "qemu/selfmap.h"
+
+GSList *read_self_maps(void)
+{
+    gchar *maps;
+    GSList *map_info = NULL;
+
+    if (g_file_get_contents("/proc/self/maps", &maps, NULL, NULL)) {
+        gchar **lines = g_strsplit(maps, "\n", 0);
+        int i, entries = g_strv_length(lines);
+
+        for (i = 0; i < entries; i++) {
+            gchar **fields = g_strsplit(lines[i], " ", 6);
+            if (g_strv_length(fields) > 4) {
+                MapInfo *e = g_new0(MapInfo, 1);
+                int errors;
+                const char *end;
+
+                errors  = qemu_strtoul(fields[0], &end, 16, &e->start);
+                errors += qemu_strtoul(end + 1, NULL, 16, &e->end);
+
+                e->is_read  = fields[1][0] == 'r' ? true : false;
+                e->is_write = fields[1][1] == 'w' ? true : false;
+                e->is_exec  = fields[1][2] == 'x' ? true : false;
+                e->is_priv  = fields[1][3] == 'p' ? true : false;
+
+                errors += qemu_strtoul(fields[2], NULL, 16, &e->offset);
+                e->dev = g_strdup(fields[3]);
+                errors += qemu_strtoi(fields[4], NULL, 10, &e->inode);
+
+                /*
+                 * The last field may have leading spaces which we
+                 * need to strip.
+                 */
+                if (g_strv_length(fields) == 6) {
+                    e->path = g_strdup(g_strchug(fields[5]));
+                }
+                map_info = g_slist_prepend(map_info, e);
+            }
+
+            g_strfreev(fields);
+        }
+        g_strfreev(lines);
+        g_free(maps);
+    }
+
+    /* ensure the map data is in the same order we collected it */
+    return g_slist_reverse(map_info);
+}
+
+/**
+ * free_self_maps:
+ * @info: a GSlist
+ *
+ * Free a list of MapInfo structures.
+ */
+static void free_info(gpointer data)
+{
+    MapInfo *e = (MapInfo *) data;
+    g_free(e->dev);
+    g_free(e->path);
+}
+
+void free_self_maps(GSList *info)
+{
+    g_slist_free_full(info, &free_info);
+}
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 6718a38b616..fe339c2636b 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -63,3 +63,4 @@  util-obj-y += guest-random.o
 util-obj-$(CONFIG_GIO) += dbus.o
 dbus.o-cflags = $(GIO_CFLAGS)
 dbus.o-libs = $(GIO_LIBS)
+util-obj-$(CONFIG_USER_ONLY) += selfmap.o