[PATCH 00/14] linux-user: Rewrite core dump

[PATCH 00/14] linux-user: Rewrite core dump

[Richard Henderson]

This started simply to remove the two page[TARGET_PAGE_SIZE] instances.
These turn into variable length arrays, when I start to allow the page
size to vary for linux-user.

However, the first thing I noticed is that it is silly to write out
target memory to the corefile page by page.  As I started to clean
that up, I noticed some actual errors in the writing of notes.

Finally, we can stop creating local data structures to represent vmas
and rely on the ones over in user-exec.c.


r~


Richard Henderson (14):
  linux-user/elfload: Disable core dump if getrlimit fails
  linux-user/elfload: Merge init_note_info and fill_note_info
  linux-user/elfload: Tidy fill_note_info and struct elf_note_info
  linux-user/elfload: Stack allocate struct mm_struct
  linux-user/elfload: Latch errno before cleanup in elf_core_dump
  linux-user/elfload: Open core file after vma_init
  linux-user/elfload: Truncate core file on open
  linux-user/elfload: Lock cpu list and mmap during elf_core_dump
  linux-user/elfload: Size corefile before opening
  linux-user/elfload: Write corefile elf header in one block
  linux-user/elfload: Write process memory to core file in larger chunks
  linux-user/elfload: Simplify vma_dump_size
  linux-user/elfload: Rely on walk_memory_regions for vmas
  linux-user/elfload: Unprotect regions before core dump

 linux-user/elfload.c | 721 +++++++++++++------------------------------
 1 file changed, 221 insertions(+), 500 deletions(-)

-- 
2.34.1

[PATCH 01/14] linux-user/elfload: Disable core dump if getrlimit fails

[Richard Henderson]

Do not dump core at all if getrlimit fails; this ensures
that dumpsize is valid throughout the function, not just
for the initial test vs rlim_cur.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index b8eef893d0..fb47fe39c9 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4673,7 +4673,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
         return 0;
     }
 
-    if (getrlimit(RLIMIT_CORE, &dumpsize) == 0 && dumpsize.rlim_cur == 0) {
+    if (getrlimit(RLIMIT_CORE, &dumpsize) < 0 || dumpsize.rlim_cur == 0) {
         return 0;
     }
 
-- 
2.34.1

[PATCH 02/14] linux-user/elfload: Merge init_note_info and fill_note_info

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index fb47fe39c9..7b3a2c20f2 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4514,16 +4514,6 @@ static void fill_thread_info(struct elf_note_info *info, const CPUArchState *env
     info->notes_size += note_size(&ets->notes[0]);
 }
 
-static void init_note_info(struct elf_note_info *info)
-{
-    /* Initialize the elf_note_info structure so that it is at
-     * least safe to call free_note_info() on it. Must be
-     * called before calling fill_note_info().
-     */
-    memset(info, 0, sizeof (*info));
-    QTAILQ_INIT(&info->thread_list);
-}
-
 static int fill_note_info(struct elf_note_info *info,
                           long signr, const CPUArchState *env)
 {
@@ -4532,6 +4522,9 @@ static int fill_note_info(struct elf_note_info *info,
     TaskState *ts = (TaskState *)cpu->opaque;
     int i;
 
+    memset(info, 0, sizeof (*info));
+    QTAILQ_INIT(&info->thread_list);
+
     info->notes = g_new0(struct memelfnote, NUMNOTES);
     if (info->notes == NULL)
         return (-ENOMEM);
@@ -4665,8 +4658,6 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     int segs = 0;
     int fd = -1;
 
-    init_note_info(&info);
-
     errno = 0;
 
     if (prctl(PR_GET_DUMPABLE) == 0) {
-- 
2.34.1

[PATCH 03/14] linux-user/elfload: Tidy fill_note_info and struct elf_note_info

[Richard Henderson]

In fill_note_info, there were unnecessary checks for
success of g_new/g_malloc.  But these structures do not
need to be dyamically allocated at all, and can in fact
be statically allocated within the parent structure.

This removes all error paths from fill_note_info, so
change the return type to void.

Change type of signr to match both caller (elf_core_dump)
and callee (fill_prstatus), which both use int for signr.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 48 +++++++++++++++-----------------------------
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 7b3a2c20f2..cc43487a37 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4066,10 +4066,12 @@ struct elf_thread_status {
     int num_notes;
 };
 
+#define NUMNOTES 3
+
 struct elf_note_info {
-    struct memelfnote   *notes;
-    struct target_elf_prstatus *prstatus;  /* NT_PRSTATUS */
-    struct target_elf_prpsinfo *psinfo;    /* NT_PRPSINFO */
+    struct memelfnote notes[NUMNOTES];
+    struct target_elf_prstatus prstatus;  /* NT_PRSTATUS */
+    struct target_elf_prpsinfo psinfo;    /* NT_PRPSINFO */
 
     QTAILQ_HEAD(, elf_thread_status) thread_list;
 #if 0
@@ -4117,7 +4119,7 @@ static void fill_auxv_note(struct memelfnote *, const TaskState *);
 static void fill_elf_note_phdr(struct elf_phdr *, int, off_t);
 static size_t note_size(const struct memelfnote *);
 static void free_note_info(struct elf_note_info *);
-static int fill_note_info(struct elf_note_info *, long, const CPUArchState *);
+static void fill_note_info(struct elf_note_info *, int, const CPUArchState *);
 static void fill_thread_info(struct elf_note_info *, const CPUArchState *);
 
 static int dump_write(int, const void *, size_t);
@@ -4514,44 +4516,33 @@ static void fill_thread_info(struct elf_note_info *info, const CPUArchState *env
     info->notes_size += note_size(&ets->notes[0]);
 }
 
-static int fill_note_info(struct elf_note_info *info,
-                          long signr, const CPUArchState *env)
+static void fill_note_info(struct elf_note_info *info,
+                           int signr, const CPUArchState *env)
 {
-#define NUMNOTES 3
     CPUState *cpu = env_cpu((CPUArchState *)env);
     TaskState *ts = (TaskState *)cpu->opaque;
-    int i;
 
     memset(info, 0, sizeof (*info));
     QTAILQ_INIT(&info->thread_list);
 
-    info->notes = g_new0(struct memelfnote, NUMNOTES);
-    if (info->notes == NULL)
-        return (-ENOMEM);
-    info->prstatus = g_malloc0(sizeof (*info->prstatus));
-    if (info->prstatus == NULL)
-        return (-ENOMEM);
-    info->psinfo = g_malloc0(sizeof (*info->psinfo));
-    if (info->prstatus == NULL)
-        return (-ENOMEM);
-
     /*
      * First fill in status (and registers) of current thread
      * including process info & aux vector.
      */
-    fill_prstatus(info->prstatus, ts, signr);
-    elf_core_copy_regs(&info->prstatus->pr_reg, env);
+    fill_prstatus(&info->prstatus, ts, signr);
+    elf_core_copy_regs(&info->prstatus.pr_reg, env);
     fill_note(&info->notes[0], "CORE", NT_PRSTATUS,
-              sizeof (*info->prstatus), info->prstatus);
-    fill_psinfo(info->psinfo, ts);
+              sizeof(info->prstatus), &info->prstatus);
+    fill_psinfo(&info->psinfo, ts);
     fill_note(&info->notes[1], "CORE", NT_PRPSINFO,
-              sizeof (*info->psinfo), info->psinfo);
+              sizeof(info->psinfo), &info->psinfo);
     fill_auxv_note(&info->notes[2], ts);
     info->numnote = 3;
 
     info->notes_size = 0;
-    for (i = 0; i < info->numnote; i++)
+    for (int i = 0; i < info->numnote; i++) {
         info->notes_size += note_size(&info->notes[i]);
+    }
 
     /* read and fill status of all threads */
     WITH_QEMU_LOCK_GUARD(&qemu_cpu_list_lock) {
@@ -4562,8 +4553,6 @@ static int fill_note_info(struct elf_note_info *info,
             fill_thread_info(info, cpu_env(cpu));
         }
     }
-
-    return (0);
 }
 
 static void free_note_info(struct elf_note_info *info)
@@ -4575,10 +4564,6 @@ static void free_note_info(struct elf_note_info *info)
         QTAILQ_REMOVE(&info->thread_list, ets, ets_link);
         g_free(ets);
     }
-
-    g_free(info->prstatus);
-    g_free(info->psinfo);
-    g_free(info->notes);
 }
 
 static int write_note_info(struct elf_note_info *info, int fd)
@@ -4694,8 +4679,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
         goto out;
 
     /* fill in the in-memory version of notes */
-    if (fill_note_info(&info, signr, env) < 0)
-        goto out;
+    fill_note_info(&info, signr, env);
 
     offset += sizeof (elf);                             /* elf header */
     offset += (segs + 1) * sizeof (struct elf_phdr);    /* program headers */
-- 
2.34.1

[PATCH 04/14] linux-user/elfload: Stack allocate struct mm_struct

[Richard Henderson]

Ignoring the fact that g_malloc cannot fail, the structure
is quite small and might as well be allocated locally.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index cc43487a37..98b82b1a49 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4099,7 +4099,7 @@ struct mm_struct {
     int mm_count;           /* number of mappings */
 };
 
-static struct mm_struct *vma_init(void);
+static void vma_init(struct mm_struct *);
 static void vma_delete(struct mm_struct *);
 static int vma_add_mapping(struct mm_struct *, target_ulong,
                            target_ulong, abi_ulong);
@@ -4174,17 +4174,10 @@ static inline void bswap_note(struct elf_note *en) { }
  * thread that received the signal is stopped.
  */
 
-static struct mm_struct *vma_init(void)
+static void vma_init(struct mm_struct *mm)
 {
-    struct mm_struct *mm;
-
-    if ((mm = g_malloc(sizeof (*mm))) == NULL)
-        return (NULL);
-
     mm->mm_count = 0;
     QTAILQ_INIT(&mm->mm_mmap);
-
-    return (mm);
 }
 
 static void vma_delete(struct mm_struct *mm)
@@ -4195,7 +4188,6 @@ static void vma_delete(struct mm_struct *mm)
         QTAILQ_REMOVE(&mm->mm_mmap, vma, vma_link);
         g_free(vma);
     }
-    g_free(mm);
 }
 
 static int vma_add_mapping(struct mm_struct *mm, target_ulong start,
@@ -4638,7 +4630,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     struct elfhdr elf;
     struct elf_phdr phdr;
     struct rlimit dumpsize;
-    struct mm_struct *mm = NULL;
+    struct mm_struct mm;
     off_t offset = 0, data_offset = 0;
     int segs = 0;
     int fd = -1;
@@ -4664,11 +4656,10 @@ static int elf_core_dump(int signr, const CPUArchState *env)
      * set up structure containing this information.  After
      * this point vma_xxx functions can be used.
      */
-    if ((mm = vma_init()) == NULL)
-        goto out;
+    vma_init(&mm);
 
-    walk_memory_regions(mm, vma_walker);
-    segs = vma_get_mapping_count(mm);
+    walk_memory_regions(&mm, vma_walker);
+    segs = vma_get_mapping_count(&mm);
 
     /*
      * Construct valid coredump ELF header.  We also
@@ -4701,7 +4692,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
      * Write program headers for memory regions mapped in
      * the target process.
      */
-    for (vma = vma_first(mm); vma != NULL; vma = vma_next(vma)) {
+    for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
         (void) memset(&phdr, 0, sizeof (phdr));
 
         phdr.p_type = PT_LOAD;
@@ -4738,7 +4729,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     /*
      * Finally we can dump process memory into corefile as well.
      */
-    for (vma = vma_first(mm); vma != NULL; vma = vma_next(vma)) {
+    for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
         abi_ulong addr;
         abi_ulong end;
 
@@ -4767,8 +4758,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
 
  out:
     free_note_info(&info);
-    if (mm != NULL)
-        vma_delete(mm);
+    vma_delete(&mm);
     (void) close(fd);
 
     if (errno != 0)
-- 
2.34.1

[PATCH 05/14] linux-user/elfload: Latch errno before cleanup in elf_core_dump

[Richard Henderson]

On the off-chance that one of the cleanup functions changes
errno, latch the errno that we want to return beforehand.

Flush errno to 0 upon success, rather than at the beginning.
No need to avoid negation of 0.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 98b82b1a49..39d9ef9acc 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4634,8 +4634,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     off_t offset = 0, data_offset = 0;
     int segs = 0;
     int fd = -1;
-
-    errno = 0;
+    int ret;
 
     if (prctl(PR_GET_DUMPABLE) == 0) {
         return 0;
@@ -4755,15 +4754,14 @@ static int elf_core_dump(int signr, const CPUArchState *env)
                 goto out;
         }
     }
+    errno = 0;
 
  out:
+    ret = -errno;
     free_note_info(&info);
     vma_delete(&mm);
-    (void) close(fd);
-
-    if (errno != 0)
-        return (-errno);
-    return (0);
+    close(fd);
+    return ret;
 }
 #endif /* USE_ELF_CORE_DUMP */
 
-- 
2.34.1

[PATCH 06/14] linux-user/elfload: Open core file after vma_init

[Richard Henderson]

Swap the ordering of vma_init and open.  This will be necessary
for further changes, and adjusts the error cleanup path.  Narrow
the scope of corefile, as the variable can be freed immediately
after use in open().

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 39d9ef9acc..877799e9c7 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4625,7 +4625,6 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     const CPUState *cpu = env_cpu((CPUArchState *)env);
     const TaskState *ts = (const TaskState *)cpu->opaque;
     struct vm_area_struct *vma = NULL;
-    g_autofree char *corefile = NULL;
     struct elf_note_info info;
     struct elfhdr elf;
     struct elf_phdr phdr;
@@ -4644,12 +4643,6 @@ static int elf_core_dump(int signr, const CPUArchState *env)
         return 0;
     }
 
-    corefile = core_dump_filename(ts);
-
-    if ((fd = open(corefile, O_WRONLY | O_CREAT,
-                   S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)) < 0)
-        return (-errno);
-
     /*
      * Walk through target process memory mappings and
      * set up structure containing this information.  After
@@ -4657,6 +4650,15 @@ static int elf_core_dump(int signr, const CPUArchState *env)
      */
     vma_init(&mm);
 
+    {
+        g_autofree char *corefile = core_dump_filename(ts);
+        fd = open(corefile, O_WRONLY | O_CREAT,
+                  S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+    }
+    if (fd < 0) {
+        goto out;
+    }
+
     walk_memory_regions(&mm, vma_walker);
     segs = vma_get_mapping_count(&mm);
 
-- 
2.34.1

[PATCH 07/14] linux-user/elfload: Truncate core file on open

[Richard Henderson]

While we usually create a new corefile, truncate otherwise.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 877799e9c7..16dd08a828 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4652,7 +4652,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
 
     {
         g_autofree char *corefile = core_dump_filename(ts);
-        fd = open(corefile, O_WRONLY | O_CREAT,
+        fd = open(corefile, O_WRONLY | O_CREAT | O_TRUNC,
                   S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
     }
     if (fd < 0) {
-- 
2.34.1

[PATCH 08/14] linux-user/elfload: Lock cpu list and mmap during elf_core_dump

[Richard Henderson]

Do not allow changes to the set of cpus and memory regions
while we are dumping core.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 16dd08a828..6f9da721d7 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4537,13 +4537,11 @@ static void fill_note_info(struct elf_note_info *info,
     }
 
     /* read and fill status of all threads */
-    WITH_QEMU_LOCK_GUARD(&qemu_cpu_list_lock) {
-        CPU_FOREACH(cpu) {
-            if (cpu == thread_cpu) {
-                continue;
-            }
-            fill_thread_info(info, cpu_env(cpu));
+    CPU_FOREACH(cpu) {
+        if (cpu == thread_cpu) {
+            continue;
         }
+        fill_thread_info(info, cpu_env(cpu));
     }
 }
 
@@ -4643,6 +4641,9 @@ static int elf_core_dump(int signr, const CPUArchState *env)
         return 0;
     }
 
+    cpu_list_lock();
+    mmap_lock();
+
     /*
      * Walk through target process memory mappings and
      * set up structure containing this information.  After
@@ -4760,6 +4761,8 @@ static int elf_core_dump(int signr, const CPUArchState *env)
 
  out:
     ret = -errno;
+    mmap_unlock();
+    cpu_list_unlock();
     free_note_info(&info);
     vma_delete(&mm);
     close(fd);
-- 
2.34.1

[PATCH 09/14] linux-user/elfload: Size corefile before opening

[Richard Henderson]

Verify the size of the corefile vs the rlimit before
opening and creating the core file at all.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 83 +++++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 39 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 6f9da721d7..bad01bd2ef 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4270,6 +4270,16 @@ static int vma_walker(void *priv, target_ulong start, target_ulong end,
     return (0);
 }
 
+static size_t size_note(const char *name, size_t datasz)
+{
+    size_t namesz = strlen(name) + 1;
+
+    namesz = ROUND_UP(namesz, 4);
+    datasz = ROUND_UP(datasz, 4);
+
+    return sizeof(struct elf_note) + namesz + datasz;
+}
+
 static void fill_note(struct memelfnote *note, const char *name, int type,
                       unsigned int sz, void *data)
 {
@@ -4428,27 +4438,9 @@ static int dump_write(int fd, const void *ptr, size_t size)
 {
     const char *bufp = (const char *)ptr;
     ssize_t bytes_written, bytes_left;
-    struct rlimit dumpsize;
-    off_t pos;
 
     bytes_written = 0;
-    getrlimit(RLIMIT_CORE, &dumpsize);
-    if ((pos = lseek(fd, 0, SEEK_CUR))==-1) {
-        if (errno == ESPIPE) { /* not a seekable stream */
-            bytes_left = size;
-        } else {
-            return pos;
-        }
-    } else {
-        if (dumpsize.rlim_cur <= pos) {
-            return -1;
-        } else if (dumpsize.rlim_cur == RLIM_INFINITY) {
-            bytes_left = size;
-        } else {
-            size_t limit_left=dumpsize.rlim_cur - pos;
-            bytes_left = limit_left >= size ? size : limit_left ;
-        }
-    }
+    bytes_left = size;
 
     /*
      * In normal conditions, single write(2) should do but
@@ -4622,16 +4614,15 @@ static int elf_core_dump(int signr, const CPUArchState *env)
 {
     const CPUState *cpu = env_cpu((CPUArchState *)env);
     const TaskState *ts = (const TaskState *)cpu->opaque;
-    struct vm_area_struct *vma = NULL;
+    struct vm_area_struct *vma;
     struct elf_note_info info;
     struct elfhdr elf;
     struct elf_phdr phdr;
     struct rlimit dumpsize;
     struct mm_struct mm;
-    off_t offset = 0, data_offset = 0;
-    int segs = 0;
+    off_t offset, note_offset, data_offset;
+    int segs, cpus, ret;
     int fd = -1;
-    int ret;
 
     if (prctl(PR_GET_DUMPABLE) == 0) {
         return 0;
@@ -4646,10 +4637,36 @@ static int elf_core_dump(int signr, const CPUArchState *env)
 
     /*
      * Walk through target process memory mappings and
-     * set up structure containing this information.  After
-     * this point vma_xxx functions can be used.
+     * set up structure containing this information.
      */
     vma_init(&mm);
+    walk_memory_regions(&mm, vma_walker);
+    segs = vma_get_mapping_count(&mm);
+
+    cpus = 0;
+    CPU_FOREACH(cpu) {
+        cpus++;
+    }
+
+    offset = sizeof(struct elfhdr);
+    offset += (segs + 1) * sizeof(struct elf_phdr);
+    note_offset = offset;
+
+    offset += size_note("CORE", ts->info->auxv_len);
+    offset += size_note("CORE", sizeof(struct target_elf_prpsinfo));
+    offset += size_note("CORE", sizeof(struct target_elf_prstatus)) * cpus;
+    offset = ROUND_UP(offset, ELF_EXEC_PAGESIZE);
+    data_offset = offset;
+
+    for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
+        offset += vma_dump_size(vma);
+    }
+
+    /* Do not dump if the corefile size exceeds the limit. */
+    if (dumpsize.rlim_cur != RLIM_INFINITY && dumpsize.rlim_cur < offset) {
+        errno = 0;
+        goto out;
+    }
 
     {
         g_autofree char *corefile = core_dump_filename(ts);
@@ -4660,9 +4677,6 @@ static int elf_core_dump(int signr, const CPUArchState *env)
         goto out;
     }
 
-    walk_memory_regions(&mm, vma_walker);
-    segs = vma_get_mapping_count(&mm);
-
     /*
      * Construct valid coredump ELF header.  We also
      * add one more segment for notes.
@@ -4674,26 +4688,17 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     /* fill in the in-memory version of notes */
     fill_note_info(&info, signr, env);
 
-    offset += sizeof (elf);                             /* elf header */
-    offset += (segs + 1) * sizeof (struct elf_phdr);    /* program headers */
-
     /* write out notes program header */
-    fill_elf_note_phdr(&phdr, info.notes_size, offset);
+    fill_elf_note_phdr(&phdr, info.notes_size, note_offset);
 
-    offset += info.notes_size;
     if (dump_write(fd, &phdr, sizeof (phdr)) != 0)
         goto out;
 
-    /*
-     * ELF specification wants data to start at page boundary so
-     * we align it here.
-     */
-    data_offset = offset = roundup(offset, ELF_EXEC_PAGESIZE);
-
     /*
      * Write program headers for memory regions mapped in
      * the target process.
      */
+    offset = data_offset;
     for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
         (void) memset(&phdr, 0, sizeof (phdr));
 
-- 
2.34.1

[PATCH 10/14] linux-user/elfload: Write corefile elf header in one block

[Richard Henderson]

Fixes a bug in which write_note() wrote namesz_rounded
and datasz_rounded bytes, even though name and data
pointers contain only the unrounded number of bytes.

Instead of many small writes, allocate a block to contain all
of the elf headers and all of the notes.  Copy the data into the
block piecemeal and the write it to the file as a chunk.
This also avoids the need to lseek forward for alignment.
---
 linux-user/elfload.c | 410 ++++++++++++-------------------------------
 1 file changed, 113 insertions(+), 297 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index bad01bd2ef..b8d07d8054 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4002,18 +4002,6 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
  * Example for ARM target is provided in this file.
  */
 
-/* An ELF note in memory */
-struct memelfnote {
-    const char *name;
-    size_t     namesz;
-    size_t     namesz_rounded;
-    int        type;
-    size_t     datasz;
-    size_t     datasz_rounded;
-    void       *data;
-    size_t     notesz;
-};
-
 struct target_elf_siginfo {
     abi_int    si_signo; /* signal number */
     abi_int    si_code;  /* extra code */
@@ -4053,40 +4041,6 @@ struct target_elf_prpsinfo {
     char    pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
 };
 
-/* Here is the structure in which status of each thread is captured. */
-struct elf_thread_status {
-    QTAILQ_ENTRY(elf_thread_status)  ets_link;
-    struct target_elf_prstatus prstatus;   /* NT_PRSTATUS */
-#if 0
-    elf_fpregset_t fpu;             /* NT_PRFPREG */
-    struct task_struct *thread;
-    elf_fpxregset_t xfpu;           /* ELF_CORE_XFPREG_TYPE */
-#endif
-    struct memelfnote notes[1];
-    int num_notes;
-};
-
-#define NUMNOTES 3
-
-struct elf_note_info {
-    struct memelfnote notes[NUMNOTES];
-    struct target_elf_prstatus prstatus;  /* NT_PRSTATUS */
-    struct target_elf_prpsinfo psinfo;    /* NT_PRPSINFO */
-
-    QTAILQ_HEAD(, elf_thread_status) thread_list;
-#if 0
-    /*
-     * Current version of ELF coredump doesn't support
-     * dumping fp regs etc.
-     */
-    elf_fpregset_t *fpu;
-    elf_fpxregset_t *xfpu;
-    int thread_status_size;
-#endif
-    int notes_size;
-    int numnote;
-};
-
 struct vm_area_struct {
     target_ulong   vma_start;  /* start vaddr of memory region */
     target_ulong   vma_end;    /* end vaddr of memory region */
@@ -4110,22 +4064,6 @@ static abi_ulong vma_dump_size(const struct vm_area_struct *);
 static int vma_walker(void *priv, target_ulong start, target_ulong end,
                       unsigned long flags);
 
-static void fill_elf_header(struct elfhdr *, int, uint16_t, uint32_t);
-static void fill_note(struct memelfnote *, const char *, int,
-                      unsigned int, void *);
-static void fill_prstatus(struct target_elf_prstatus *, const TaskState *, int);
-static int fill_psinfo(struct target_elf_prpsinfo *, const TaskState *);
-static void fill_auxv_note(struct memelfnote *, const TaskState *);
-static void fill_elf_note_phdr(struct elf_phdr *, int, off_t);
-static size_t note_size(const struct memelfnote *);
-static void free_note_info(struct elf_note_info *);
-static void fill_note_info(struct elf_note_info *, int, const CPUArchState *);
-static void fill_thread_info(struct elf_note_info *, const CPUArchState *);
-
-static int dump_write(int, const void *, size_t);
-static int write_note(struct memelfnote *, int);
-static int write_note_info(struct elf_note_info *, int);
-
 #ifdef BSWAP_NEEDED
 static void bswap_prstatus(struct target_elf_prstatus *prstatus)
 {
@@ -4280,35 +4218,32 @@ static size_t size_note(const char *name, size_t datasz)
     return sizeof(struct elf_note) + namesz + datasz;
 }
 
-static void fill_note(struct memelfnote *note, const char *name, int type,
-                      unsigned int sz, void *data)
+static void *fill_note(void **pptr, int type, const char *name, size_t datasz)
 {
-    unsigned int namesz;
+    void *ptr = *pptr;
+    struct elf_note *n = ptr;
+    size_t namesz = strlen(name) + 1;
 
-    namesz = strlen(name) + 1;
-    note->name = name;
-    note->namesz = namesz;
-    note->namesz_rounded = roundup(namesz, sizeof (int32_t));
-    note->type = type;
-    note->datasz = sz;
-    note->datasz_rounded = roundup(sz, sizeof (int32_t));
+    n->n_namesz = namesz;
+    n->n_descsz = datasz;
+    n->n_type = type;
+    bswap_note(n);
 
-    note->data = data;
+    ptr += sizeof(*n);
+    memcpy(ptr, name, namesz);
 
-    /*
-     * We calculate rounded up note size here as specified by
-     * ELF document.
-     */
-    note->notesz = sizeof (struct elf_note) +
-        note->namesz_rounded + note->datasz_rounded;
+    namesz = ROUND_UP(namesz, 4);
+    datasz = ROUND_UP(datasz, 4);
+
+    *pptr = ptr + namesz + datasz;
+    return ptr + namesz;
 }
 
 static void fill_elf_header(struct elfhdr *elf, int segs, uint16_t machine,
                             uint32_t flags)
 {
-    (void) memset(elf, 0, sizeof(*elf));
+    memcpy(elf->e_ident, ELFMAG, SELFMAG);
 
-    (void) memcpy(elf->e_ident, ELFMAG, SELFMAG);
     elf->e_ident[EI_CLASS] = ELF_CLASS;
     elf->e_ident[EI_DATA] = ELF_DATA;
     elf->e_ident[EI_VERSION] = EV_CURRENT;
@@ -4326,95 +4261,79 @@ static void fill_elf_header(struct elfhdr *elf, int segs, uint16_t machine,
     bswap_ehdr(elf);
 }
 
-static void fill_elf_note_phdr(struct elf_phdr *phdr, int sz, off_t offset)
+static void fill_elf_note_phdr(struct elf_phdr *phdr, size_t sz, off_t offset)
 {
     phdr->p_type = PT_NOTE;
     phdr->p_offset = offset;
-    phdr->p_vaddr = 0;
-    phdr->p_paddr = 0;
     phdr->p_filesz = sz;
-    phdr->p_memsz = 0;
-    phdr->p_flags = 0;
-    phdr->p_align = 0;
 
     bswap_phdr(phdr, 1);
 }
 
-static size_t note_size(const struct memelfnote *note)
+static void fill_prstatus_note(void *data, const TaskState *ts,
+                               CPUState *cpu, int signr)
 {
-    return (note->notesz);
+    /*
+     * Because note memory is only aligned to 4, and target_elf_prstatus
+     * may well have higher alignment requirements, fill locally and
+     * memcpy to the destination afterward.
+     */
+    struct target_elf_prstatus prstatus = {
+        .pr_info.si_signo = signr,
+        .pr_cursig = signr,
+        .pr_pid = ts->ts_tid,
+        .pr_ppid = getppid(),
+        .pr_pgrp = getpgrp(),
+        .pr_sid = getsid(0),
+    };
+
+    elf_core_copy_regs(&prstatus.pr_reg, cpu_env(cpu));
+    bswap_prstatus(&prstatus);
+    memcpy(data, &prstatus, sizeof(prstatus));
 }
 
-static void fill_prstatus(struct target_elf_prstatus *prstatus,
-                          const TaskState *ts, int signr)
-{
-    (void) memset(prstatus, 0, sizeof (*prstatus));
-    prstatus->pr_info.si_signo = prstatus->pr_cursig = signr;
-    prstatus->pr_pid = ts->ts_tid;
-    prstatus->pr_ppid = getppid();
-    prstatus->pr_pgrp = getpgrp();
-    prstatus->pr_sid = getsid(0);
-
-    bswap_prstatus(prstatus);
-}
-
-static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)
+static void fill_prpsinfo_note(void *data, const TaskState *ts)
 {
+    /*
+     * Because note memory is only aligned to 4, and target_elf_prpsinfo
+     * may well have higher alignment requirements, fill locally and
+     * memcpy to the destination afterward.
+     */
+    struct target_elf_prpsinfo psinfo;
     char *base_filename;
-    unsigned int i, len;
-
-    (void) memset(psinfo, 0, sizeof (*psinfo));
+    size_t len;
 
     len = ts->info->env_strings - ts->info->arg_strings;
-    if (len >= ELF_PRARGSZ)
-        len = ELF_PRARGSZ - 1;
-    if (copy_from_user(&psinfo->pr_psargs, ts->info->arg_strings, len)) {
-        return -EFAULT;
+    len = MIN(len, ELF_PRARGSZ);
+    memcpy(&psinfo.pr_psargs, g2h_untagged(ts->info->arg_strings), len);
+    for (size_t i = 0; i < len; i++) {
+        if (psinfo.pr_psargs[i] == 0) {
+            psinfo.pr_psargs[i] = ' ';
+        }
     }
-    for (i = 0; i < len; i++)
-        if (psinfo->pr_psargs[i] == 0)
-            psinfo->pr_psargs[i] = ' ';
-    psinfo->pr_psargs[len] = 0;
 
-    psinfo->pr_pid = getpid();
-    psinfo->pr_ppid = getppid();
-    psinfo->pr_pgrp = getpgrp();
-    psinfo->pr_sid = getsid(0);
-    psinfo->pr_uid = getuid();
-    psinfo->pr_gid = getgid();
+    psinfo.pr_pid = getpid();
+    psinfo.pr_ppid = getppid();
+    psinfo.pr_pgrp = getpgrp();
+    psinfo.pr_sid = getsid(0);
+    psinfo.pr_uid = getuid();
+    psinfo.pr_gid = getgid();
 
     base_filename = g_path_get_basename(ts->bprm->filename);
     /*
      * Using strncpy here is fine: at max-length,
      * this field is not NUL-terminated.
      */
-    (void) strncpy(psinfo->pr_fname, base_filename,
-                   sizeof(psinfo->pr_fname));
-
+    strncpy(psinfo.pr_fname, base_filename, sizeof(psinfo.pr_fname));
     g_free(base_filename);
-    bswap_psinfo(psinfo);
-    return (0);
+
+    bswap_psinfo(&psinfo);
+    memcpy(data, &psinfo, sizeof(psinfo));
 }
 
-static void fill_auxv_note(struct memelfnote *note, const TaskState *ts)
+static void fill_auxv_note(void *data, const TaskState *ts)
 {
-    elf_addr_t auxv = (elf_addr_t)ts->info->saved_auxv;
-    elf_addr_t orig_auxv = auxv;
-    void *ptr;
-    int len = ts->info->auxv_len;
-
-    /*
-     * Auxiliary vector is stored in target process stack.  It contains
-     * {type, value} pairs that we need to dump into note.  This is not
-     * strictly necessary but we do it here for sake of completeness.
-     */
-
-    /* read in whole auxv vector and copy it to memelfnote */
-    ptr = lock_user(VERIFY_READ, orig_auxv, len, 0);
-    if (ptr != NULL) {
-        fill_note(note, "CORE", NT_AUXV, len, ptr);
-        unlock_user(ptr, auxv, len);
-    }
+    memcpy(data, g2h_untagged(ts->info->saved_auxv), ts->info->auxv_len);
 }
 
 /*
@@ -4462,111 +4381,6 @@ static int dump_write(int fd, const void *ptr, size_t size)
     return (0);
 }
 
-static int write_note(struct memelfnote *men, int fd)
-{
-    struct elf_note en;
-
-    en.n_namesz = men->namesz;
-    en.n_type = men->type;
-    en.n_descsz = men->datasz;
-
-    bswap_note(&en);
-
-    if (dump_write(fd, &en, sizeof(en)) != 0)
-        return (-1);
-    if (dump_write(fd, men->name, men->namesz_rounded) != 0)
-        return (-1);
-    if (dump_write(fd, men->data, men->datasz_rounded) != 0)
-        return (-1);
-
-    return (0);
-}
-
-static void fill_thread_info(struct elf_note_info *info, const CPUArchState *env)
-{
-    CPUState *cpu = env_cpu((CPUArchState *)env);
-    TaskState *ts = (TaskState *)cpu->opaque;
-    struct elf_thread_status *ets;
-
-    ets = g_malloc0(sizeof (*ets));
-    ets->num_notes = 1; /* only prstatus is dumped */
-    fill_prstatus(&ets->prstatus, ts, 0);
-    elf_core_copy_regs(&ets->prstatus.pr_reg, env);
-    fill_note(&ets->notes[0], "CORE", NT_PRSTATUS, sizeof (ets->prstatus),
-              &ets->prstatus);
-
-    QTAILQ_INSERT_TAIL(&info->thread_list, ets, ets_link);
-
-    info->notes_size += note_size(&ets->notes[0]);
-}
-
-static void fill_note_info(struct elf_note_info *info,
-                           int signr, const CPUArchState *env)
-{
-    CPUState *cpu = env_cpu((CPUArchState *)env);
-    TaskState *ts = (TaskState *)cpu->opaque;
-
-    memset(info, 0, sizeof (*info));
-    QTAILQ_INIT(&info->thread_list);
-
-    /*
-     * First fill in status (and registers) of current thread
-     * including process info & aux vector.
-     */
-    fill_prstatus(&info->prstatus, ts, signr);
-    elf_core_copy_regs(&info->prstatus.pr_reg, env);
-    fill_note(&info->notes[0], "CORE", NT_PRSTATUS,
-              sizeof(info->prstatus), &info->prstatus);
-    fill_psinfo(&info->psinfo, ts);
-    fill_note(&info->notes[1], "CORE", NT_PRPSINFO,
-              sizeof(info->psinfo), &info->psinfo);
-    fill_auxv_note(&info->notes[2], ts);
-    info->numnote = 3;
-
-    info->notes_size = 0;
-    for (int i = 0; i < info->numnote; i++) {
-        info->notes_size += note_size(&info->notes[i]);
-    }
-
-    /* read and fill status of all threads */
-    CPU_FOREACH(cpu) {
-        if (cpu == thread_cpu) {
-            continue;
-        }
-        fill_thread_info(info, cpu_env(cpu));
-    }
-}
-
-static void free_note_info(struct elf_note_info *info)
-{
-    struct elf_thread_status *ets;
-
-    while (!QTAILQ_EMPTY(&info->thread_list)) {
-        ets = QTAILQ_FIRST(&info->thread_list);
-        QTAILQ_REMOVE(&info->thread_list, ets, ets_link);
-        g_free(ets);
-    }
-}
-
-static int write_note_info(struct elf_note_info *info, int fd)
-{
-    struct elf_thread_status *ets;
-    int i, error = 0;
-
-    /* write prstatus, psinfo and auxv for current thread */
-    for (i = 0; i < info->numnote; i++)
-        if ((error = write_note(&info->notes[i], fd)) != 0)
-            return (error);
-
-    /* write prstatus for each thread */
-    QTAILQ_FOREACH(ets, &info->thread_list, ets_link) {
-        if ((error = write_note(&ets->notes[0], fd)) != 0)
-            return (error);
-    }
-
-    return (0);
-}
-
 /*
  * Write out ELF coredump.
  *
@@ -4615,14 +4429,13 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     const CPUState *cpu = env_cpu((CPUArchState *)env);
     const TaskState *ts = (const TaskState *)cpu->opaque;
     struct vm_area_struct *vma;
-    struct elf_note_info info;
-    struct elfhdr elf;
-    struct elf_phdr phdr;
     struct rlimit dumpsize;
     struct mm_struct mm;
     off_t offset, note_offset, data_offset;
+    size_t note_size;
     int segs, cpus, ret;
     int fd = -1;
+    CPUState *cpu_iter;
 
     if (prctl(PR_GET_DUMPABLE) == 0) {
         return 0;
@@ -4644,7 +4457,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     segs = vma_get_mapping_count(&mm);
 
     cpus = 0;
-    CPU_FOREACH(cpu) {
+    CPU_FOREACH(cpu_iter) {
         cpus++;
     }
 
@@ -4655,6 +4468,7 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     offset += size_note("CORE", ts->info->auxv_len);
     offset += size_note("CORE", sizeof(struct target_elf_prpsinfo));
     offset += size_note("CORE", sizeof(struct target_elf_prstatus)) * cpus;
+    note_size = offset - note_offset;
     offset = ROUND_UP(offset, ELF_EXEC_PAGESIZE);
     data_offset = offset;
 
@@ -4678,61 +4492,64 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     }
 
     /*
-     * Construct valid coredump ELF header.  We also
-     * add one more segment for notes.
+     * There is a fair amount of alignment padding within the notes
+     * as well as preceeding the process memory.  Allocate a zeroed
+     * block to hold it all.  Write all of the headers directly into
+     * this buffer and then write it out as a block.
      */
-    fill_elf_header(&elf, segs + 1, ELF_MACHINE, 0);
-    if (dump_write(fd, &elf, sizeof (elf)) != 0)
-        goto out;
+    {
+        g_autofree void *header = g_malloc0(data_offset);
+        void *hptr, *dptr;
 
-    /* fill in the in-memory version of notes */
-    fill_note_info(&info, signr, env);
+        /* Create elf file header. */
+        hptr = header;
+        fill_elf_header(hptr, segs + 1, ELF_MACHINE, 0);
+        hptr += sizeof(struct elfhdr);
 
-    /* write out notes program header */
-    fill_elf_note_phdr(&phdr, info.notes_size, note_offset);
+        /* Create elf program headers. */
+        fill_elf_note_phdr(hptr, note_size, note_offset);
+        hptr += sizeof(struct elf_phdr);
 
-    if (dump_write(fd, &phdr, sizeof (phdr)) != 0)
-        goto out;
+        offset = data_offset;
+        for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
+            struct elf_phdr *phdr = hptr;
 
-    /*
-     * Write program headers for memory regions mapped in
-     * the target process.
-     */
-    offset = data_offset;
-    for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
-        (void) memset(&phdr, 0, sizeof (phdr));
+            phdr->p_type = PT_LOAD;
+            phdr->p_offset = offset;
+            phdr->p_vaddr = vma->vma_start;
+            phdr->p_paddr = 0;
+            phdr->p_filesz = vma_dump_size(vma);
+            offset += phdr->p_filesz;
+            phdr->p_memsz = vma->vma_end - vma->vma_start;
+            phdr->p_flags = (vma->vma_flags & PROT_READ ? PF_R : 0)
+                          | (vma->vma_flags & PROT_WRITE ? PF_W : 0)
+                          | (vma->vma_flags & PROT_EXEC ? PF_X : 0);
+            phdr->p_align = ELF_EXEC_PAGESIZE;
 
-        phdr.p_type = PT_LOAD;
-        phdr.p_offset = offset;
-        phdr.p_vaddr = vma->vma_start;
-        phdr.p_paddr = 0;
-        phdr.p_filesz = vma_dump_size(vma);
-        offset += phdr.p_filesz;
-        phdr.p_memsz = vma->vma_end - vma->vma_start;
-        phdr.p_flags = vma->vma_flags & PROT_READ ? PF_R : 0;
-        if (vma->vma_flags & PROT_WRITE)
-            phdr.p_flags |= PF_W;
-        if (vma->vma_flags & PROT_EXEC)
-            phdr.p_flags |= PF_X;
-        phdr.p_align = ELF_EXEC_PAGESIZE;
+            bswap_phdr(phdr, 1);
+            hptr += sizeof(struct elf_phdr);
+        }
 
-        bswap_phdr(&phdr, 1);
-        if (dump_write(fd, &phdr, sizeof(phdr)) != 0) {
+        /* Create the notes. */
+        dptr = fill_note(&hptr, NT_AUXV, "CORE", ts->info->auxv_len);
+        fill_auxv_note(dptr, ts);
+
+        dptr = fill_note(&hptr, NT_PRPSINFO, "CORE",
+                         sizeof(struct target_elf_prpsinfo));
+        fill_prpsinfo_note(dptr, ts);
+
+        CPU_FOREACH(cpu_iter) {
+            dptr = fill_note(&hptr, NT_PRSTATUS, "CORE",
+                             sizeof(struct target_elf_prstatus));
+            fill_prstatus_note(dptr, ts, cpu_iter,
+                               cpu_iter == cpu ? signr : 0);
+        }
+
+        if (dump_write(fd, header, data_offset) < 0) {
             goto out;
         }
     }
 
-    /*
-     * Next we write notes just after program headers.  No
-     * alignment needed here.
-     */
-    if (write_note_info(&info, fd) < 0)
-        goto out;
-
-    /* align data to page boundary */
-    if (lseek(fd, data_offset, SEEK_SET) != data_offset)
-        goto out;
-
     /*
      * Finally we can dump process memory into corefile as well.
      */
@@ -4768,7 +4585,6 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     ret = -errno;
     mmap_unlock();
     cpu_list_unlock();
-    free_note_info(&info);
     vma_delete(&mm);
     close(fd);
     return ret;
-- 
2.34.1

[PATCH 11/14] linux-user/elfload: Write process memory to core file in larger chunks

[Richard Henderson]

We do not need to copy pages from guest memory before writing
them out.  Because vmas are contiguous in host memory, we can
write them in one go.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index b8d07d8054..491e754f72 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4551,32 +4551,13 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     }
 
     /*
-     * Finally we can dump process memory into corefile as well.
+     * Finally write process memory into the corefile as well.
      */
     for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
-        abi_ulong addr;
-        abi_ulong end;
+        size_t size = vma_dump_size(vma);
 
-        end = vma->vma_start + vma_dump_size(vma);
-
-        for (addr = vma->vma_start; addr < end;
-             addr += TARGET_PAGE_SIZE) {
-            char page[TARGET_PAGE_SIZE];
-            int error;
-
-            /*
-             *  Read in page from target process memory and
-             *  write it to coredump file.
-             */
-            error = copy_from_user(page, addr, sizeof (page));
-            if (error != 0) {
-                (void) fprintf(stderr, "unable to dump " TARGET_ABI_FMT_lx "\n",
-                               addr);
-                errno = -error;
-                goto out;
-            }
-            if (dump_write(fd, page, TARGET_PAGE_SIZE) < 0)
-                goto out;
+        if (size && dump_write(fd, g2h_untagged(vma->vma_start), size) < 0) {
+            goto out;
         }
     }
     errno = 0;
-- 
2.34.1

[PATCH 12/14] linux-user/elfload: Simplify vma_dump_size

[Richard Henderson]

Use the flags that we've already saved in order to test
accessibility.  Use g2h_untagged and compare guest memory
directly instead of copy_from_user.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 491e754f72..47b5ce3005 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4166,37 +4166,23 @@ static int vma_get_mapping_count(const struct mm_struct *mm)
  */
 static abi_ulong vma_dump_size(const struct vm_area_struct *vma)
 {
-    /* if we cannot even read the first page, skip it */
-    if (!access_ok_untagged(VERIFY_READ, vma->vma_start, TARGET_PAGE_SIZE))
-        return (0);
+    /* The area must be readable. */
+    if (!(vma->vma_flags & PROT_READ)) {
+        return 0;
+    }
 
     /*
      * Usually we don't dump executable pages as they contain
      * non-writable code that debugger can read directly from
-     * target library etc.  However, thread stacks are marked
-     * also executable so we read in first page of given region
-     * and check whether it contains elf header.  If there is
-     * no elf header, we dump it.
+     * target library etc. If there is no elf header, we dump it.
      */
-    if (vma->vma_flags & PROT_EXEC) {
-        char page[TARGET_PAGE_SIZE];
-
-        if (copy_from_user(page, vma->vma_start, sizeof (page))) {
-            return 0;
-        }
-        if ((page[EI_MAG0] == ELFMAG0) &&
-            (page[EI_MAG1] == ELFMAG1) &&
-            (page[EI_MAG2] == ELFMAG2) &&
-            (page[EI_MAG3] == ELFMAG3)) {
-            /*
-             * Mappings are possibly from ELF binary.  Don't dump
-             * them.
-             */
-            return (0);
-        }
+    if (!(vma->vma_flags & PROT_WRITE) &&
+        (vma->vma_flags & PROT_EXEC) &&
+        memcmp(g2h_untagged(vma->vma_start), ELFMAG, SELFMAG) == 0) {
+        return 0;
     }
 
-    return (vma->vma_end - vma->vma_start);
+    return vma->vma_end - vma->vma_start;
 }
 
 static int vma_walker(void *priv, target_ulong start, target_ulong end,
-- 
2.34.1

[PATCH 13/14] linux-user/elfload: Rely on walk_memory_regions for vmas

[Richard Henderson]

Rather than creating new data structures for vma,
rely on the IntervalTree used by walk_memory_regions.

Use PAGE_* constants, per the page table api, rather
than PROT_* constants, per the mmap api.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 213 ++++++++++++++++---------------------------
 1 file changed, 78 insertions(+), 135 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 47b5ce3005..ae0abc4931 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4041,29 +4041,6 @@ struct target_elf_prpsinfo {
     char    pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
 };
 
-struct vm_area_struct {
-    target_ulong   vma_start;  /* start vaddr of memory region */
-    target_ulong   vma_end;    /* end vaddr of memory region */
-    abi_ulong      vma_flags;  /* protection etc. flags for the region */
-    QTAILQ_ENTRY(vm_area_struct) vma_link;
-};
-
-struct mm_struct {
-    QTAILQ_HEAD(, vm_area_struct) mm_mmap;
-    int mm_count;           /* number of mappings */
-};
-
-static void vma_init(struct mm_struct *);
-static void vma_delete(struct mm_struct *);
-static int vma_add_mapping(struct mm_struct *, target_ulong,
-                           target_ulong, abi_ulong);
-static int vma_get_mapping_count(const struct mm_struct *);
-static struct vm_area_struct *vma_first(const struct mm_struct *);
-static struct vm_area_struct *vma_next(struct vm_area_struct *);
-static abi_ulong vma_dump_size(const struct vm_area_struct *);
-static int vma_walker(void *priv, target_ulong start, target_ulong end,
-                      unsigned long flags);
-
 #ifdef BSWAP_NEEDED
 static void bswap_prstatus(struct target_elf_prstatus *prstatus)
 {
@@ -4105,69 +4082,14 @@ static inline void bswap_psinfo(struct target_elf_prpsinfo *p) {}
 static inline void bswap_note(struct elf_note *en) { }
 #endif /* BSWAP_NEEDED */
 
-/*
- * Minimal support for linux memory regions.  These are needed
- * when we are finding out what memory exactly belongs to
- * emulated process.  No locks needed here, as long as
- * thread that received the signal is stopped.
- */
-
-static void vma_init(struct mm_struct *mm)
-{
-    mm->mm_count = 0;
-    QTAILQ_INIT(&mm->mm_mmap);
-}
-
-static void vma_delete(struct mm_struct *mm)
-{
-    struct vm_area_struct *vma;
-
-    while ((vma = vma_first(mm)) != NULL) {
-        QTAILQ_REMOVE(&mm->mm_mmap, vma, vma_link);
-        g_free(vma);
-    }
-}
-
-static int vma_add_mapping(struct mm_struct *mm, target_ulong start,
-                           target_ulong end, abi_ulong flags)
-{
-    struct vm_area_struct *vma;
-
-    if ((vma = g_malloc0(sizeof (*vma))) == NULL)
-        return (-1);
-
-    vma->vma_start = start;
-    vma->vma_end = end;
-    vma->vma_flags = flags;
-
-    QTAILQ_INSERT_TAIL(&mm->mm_mmap, vma, vma_link);
-    mm->mm_count++;
-
-    return (0);
-}
-
-static struct vm_area_struct *vma_first(const struct mm_struct *mm)
-{
-    return (QTAILQ_FIRST(&mm->mm_mmap));
-}
-
-static struct vm_area_struct *vma_next(struct vm_area_struct *vma)
-{
-    return (QTAILQ_NEXT(vma, vma_link));
-}
-
-static int vma_get_mapping_count(const struct mm_struct *mm)
-{
-    return (mm->mm_count);
-}
-
 /*
  * Calculate file (dump) size of given memory region.
  */
-static abi_ulong vma_dump_size(const struct vm_area_struct *vma)
+static size_t vma_dump_size(target_ulong start, target_ulong end,
+                            unsigned long flags)
 {
     /* The area must be readable. */
-    if (!(vma->vma_flags & PROT_READ)) {
+    if (!(flags & PAGE_READ)) {
         return 0;
     }
 
@@ -4176,22 +4098,13 @@ static abi_ulong vma_dump_size(const struct vm_area_struct *vma)
      * non-writable code that debugger can read directly from
      * target library etc. If there is no elf header, we dump it.
      */
-    if (!(vma->vma_flags & PROT_WRITE) &&
-        (vma->vma_flags & PROT_EXEC) &&
-        memcmp(g2h_untagged(vma->vma_start), ELFMAG, SELFMAG) == 0) {
+    if (!(flags & PAGE_WRITE_ORG) &&
+        (flags & PAGE_EXEC) &&
+        memcmp(g2h_untagged(start), ELFMAG, SELFMAG) == 0) {
         return 0;
     }
 
-    return vma->vma_end - vma->vma_start;
-}
-
-static int vma_walker(void *priv, target_ulong start, target_ulong end,
-                      unsigned long flags)
-{
-    struct mm_struct *mm = (struct mm_struct *)priv;
-
-    vma_add_mapping(mm, start, end, flags);
-    return (0);
+    return end - start;
 }
 
 static size_t size_note(const char *name, size_t datasz)
@@ -4367,6 +4280,61 @@ static int dump_write(int fd, const void *ptr, size_t size)
     return (0);
 }
 
+typedef struct {
+    unsigned count;
+    size_t size;
+} CountAndSizeRegions;
+
+static int wmr_count_and_size_regions(void *opaque, target_ulong start,
+                                      target_ulong end, unsigned long flags)
+{
+    CountAndSizeRegions *css = opaque;
+
+    css->count++;
+    css->size += vma_dump_size(start, end, flags);
+    return 0;
+}
+
+typedef struct {
+    struct elf_phdr *phdr;
+    off_t offset;
+} FillRegionPhdr;
+
+static int wmr_fill_region_phdr(void *opaque, target_ulong start,
+                                target_ulong end, unsigned long flags)
+{
+    FillRegionPhdr *d = opaque;
+    struct elf_phdr *phdr = d->phdr;
+
+    phdr->p_type = PT_LOAD;
+    phdr->p_vaddr = start;
+    phdr->p_paddr = 0;
+    phdr->p_filesz = vma_dump_size(start, end, flags);
+    phdr->p_offset = d->offset;
+    d->offset += phdr->p_filesz;
+    phdr->p_memsz = end - start;
+    phdr->p_flags = (flags & PAGE_READ ? PF_R : 0)
+                  | (flags & PAGE_WRITE_ORG ? PF_W : 0)
+                  | (flags & PAGE_EXEC ? PF_X : 0);
+    phdr->p_align = ELF_EXEC_PAGESIZE;
+
+    bswap_phdr(phdr, 1);
+    d->phdr = phdr + 1;
+    return 0;
+}
+
+static int wmr_write_region(void *opaque, target_ulong start,
+                            target_ulong end, unsigned long flags)
+{
+    int fd = *(int *)opaque;
+    size_t size = vma_dump_size(start, end, flags);
+
+    if (!size) {
+        return 0;
+    }
+    return dump_write(fd, g2h_untagged(start), size);
+}
+
 /*
  * Write out ELF coredump.
  *
@@ -4414,12 +4382,11 @@ static int elf_core_dump(int signr, const CPUArchState *env)
 {
     const CPUState *cpu = env_cpu((CPUArchState *)env);
     const TaskState *ts = (const TaskState *)cpu->opaque;
-    struct vm_area_struct *vma;
     struct rlimit dumpsize;
-    struct mm_struct mm;
+    CountAndSizeRegions css;
     off_t offset, note_offset, data_offset;
     size_t note_size;
-    int segs, cpus, ret;
+    int cpus, ret;
     int fd = -1;
     CPUState *cpu_iter;
 
@@ -4438,9 +4405,8 @@ static int elf_core_dump(int signr, const CPUArchState *env)
      * Walk through target process memory mappings and
      * set up structure containing this information.
      */
-    vma_init(&mm);
-    walk_memory_regions(&mm, vma_walker);
-    segs = vma_get_mapping_count(&mm);
+    memset(&css, 0, sizeof(css));
+    walk_memory_regions(&css, wmr_count_and_size_regions);
 
     cpus = 0;
     CPU_FOREACH(cpu_iter) {
@@ -4448,22 +4414,18 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     }
 
     offset = sizeof(struct elfhdr);
-    offset += (segs + 1) * sizeof(struct elf_phdr);
+    offset += (css.count + 1) * sizeof(struct elf_phdr);
     note_offset = offset;
 
     offset += size_note("CORE", ts->info->auxv_len);
     offset += size_note("CORE", sizeof(struct target_elf_prpsinfo));
     offset += size_note("CORE", sizeof(struct target_elf_prstatus)) * cpus;
     note_size = offset - note_offset;
-    offset = ROUND_UP(offset, ELF_EXEC_PAGESIZE);
-    data_offset = offset;
-
-    for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
-        offset += vma_dump_size(vma);
-    }
+    data_offset = ROUND_UP(offset, ELF_EXEC_PAGESIZE);
 
     /* Do not dump if the corefile size exceeds the limit. */
-    if (dumpsize.rlim_cur != RLIM_INFINITY && dumpsize.rlim_cur < offset) {
+    if (dumpsize.rlim_cur != RLIM_INFINITY
+        && dumpsize.rlim_cur < data_offset + css.size) {
         errno = 0;
         goto out;
     }
@@ -4485,36 +4447,22 @@ static int elf_core_dump(int signr, const CPUArchState *env)
      */
     {
         g_autofree void *header = g_malloc0(data_offset);
+        FillRegionPhdr frp;
         void *hptr, *dptr;
 
         /* Create elf file header. */
         hptr = header;
-        fill_elf_header(hptr, segs + 1, ELF_MACHINE, 0);
+        fill_elf_header(hptr, css.count + 1, ELF_MACHINE, 0);
         hptr += sizeof(struct elfhdr);
 
         /* Create elf program headers. */
         fill_elf_note_phdr(hptr, note_size, note_offset);
         hptr += sizeof(struct elf_phdr);
 
-        offset = data_offset;
-        for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
-            struct elf_phdr *phdr = hptr;
-
-            phdr->p_type = PT_LOAD;
-            phdr->p_offset = offset;
-            phdr->p_vaddr = vma->vma_start;
-            phdr->p_paddr = 0;
-            phdr->p_filesz = vma_dump_size(vma);
-            offset += phdr->p_filesz;
-            phdr->p_memsz = vma->vma_end - vma->vma_start;
-            phdr->p_flags = (vma->vma_flags & PROT_READ ? PF_R : 0)
-                          | (vma->vma_flags & PROT_WRITE ? PF_W : 0)
-                          | (vma->vma_flags & PROT_EXEC ? PF_X : 0);
-            phdr->p_align = ELF_EXEC_PAGESIZE;
-
-            bswap_phdr(phdr, 1);
-            hptr += sizeof(struct elf_phdr);
-        }
+        frp.phdr = hptr;
+        frp.offset = data_offset;
+        walk_memory_regions(&frp, wmr_fill_region_phdr);
+        hptr = frp.phdr;
 
         /* Create the notes. */
         dptr = fill_note(&hptr, NT_AUXV, "CORE", ts->info->auxv_len);
@@ -4539,12 +4487,8 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     /*
      * Finally write process memory into the corefile as well.
      */
-    for (vma = vma_first(&mm); vma != NULL; vma = vma_next(vma)) {
-        size_t size = vma_dump_size(vma);
-
-        if (size && dump_write(fd, g2h_untagged(vma->vma_start), size) < 0) {
-            goto out;
-        }
+    if (walk_memory_regions(&fd, wmr_write_region) < 0) {
+        goto out;
     }
     errno = 0;
 
@@ -4552,7 +4496,6 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     ret = -errno;
     mmap_unlock();
     cpu_list_unlock();
-    vma_delete(&mm);
     close(fd);
     return ret;
 }
-- 
2.34.1

[PATCH 14/14] linux-user/elfload: Unprotect regions before core dump

[Richard Henderson]

By unprotecting regions, we re-instate writability and
unify regions that have been split, which may reduce
the total number of regions.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index ae0abc4931..38bfc9ac67 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3963,6 +3963,8 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
 }
 
 #ifdef USE_ELF_CORE_DUMP
+#include "exec/translate-all.h"
+
 /*
  * Definitions to generate Intel SVR4-like core files.
  * These mostly have the same names as the SVR4 types with "target_elf_"
@@ -4280,6 +4282,23 @@ static int dump_write(int fd, const void *ptr, size_t size)
     return (0);
 }
 
+static int wmr_page_unprotect_regions(void *opaque, target_ulong start,
+                                      target_ulong end, unsigned long flags)
+{
+    if ((flags & (PAGE_WRITE | PAGE_WRITE_ORG)) == PAGE_WRITE_ORG) {
+        size_t step = MAX(TARGET_PAGE_SIZE, qemu_host_page_size);
+
+        while (1) {
+            page_unprotect(start, 0);
+            if (end - start <= step) {
+                break;
+            }
+            start += step;
+        }
+    }
+    return 0;
+}
+
 typedef struct {
     unsigned count;
     size_t size;
@@ -4401,6 +4420,9 @@ static int elf_core_dump(int signr, const CPUArchState *env)
     cpu_list_lock();
     mmap_lock();
 
+    /* By unprotecting, we merge vmas that might be split. */
+    walk_memory_regions(NULL, wmr_page_unprotect_regions);
+
     /*
      * Walk through target process memory mappings and
      * set up structure containing this information.
-- 
2.34.1

Re: [PATCH 01/14] linux-user/elfload: Disable core dump if getrlimit fails

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Do not dump core at all if getrlimit fails; this ensures
> that dumpsize is valid throughout the function, not just
> for the initial test vs rlim_cur.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 02/14] linux-user/elfload: Merge init_note_info and fill_note_info

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 08/14] linux-user/elfload: Lock cpu list and mmap during elf_core_dump

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Do not allow changes to the set of cpus and memory regions
> while we are dumping core.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro