[v5,13/18] dump/dump: Add arch section support

Commit Message

Janosch Frank Aug. 11, 2022, 12:11 p.m. UTC

Add hooks which architectures can use to add arbitrary data to custom
sections.

Signed-off-by: Janosch Frank <[e-mail]>
---
 dump/dump.c                | 120 ++++++++++++++++++++++++++++++-------
 include/sysemu/dump-arch.h |   3 +
 2 files changed, 100 insertions(+), 23 deletions(-)

Comments

Janis Schoetterl-Glausch Sept. 1, 2022, 9:26 a.m. UTC | #1

On Thu, 2022-08-11 at 12:11 +0000, Janosch Frank wrote:
> Add hooks which architectures can use to add arbitrary data to custom
> sections.
> 
> Signed-off-by: Janosch Frank <[e-mail]>
> ---
>  dump/dump.c                | 120 ++++++++++++++++++++++++++++++-------
>  include/sysemu/dump-arch.h |   3 +
>  2 files changed, 100 insertions(+), 23 deletions(-)
> 
> diff --git a/dump/dump.c b/dump/dump.c
> index 0d6dbf453a..65b18fc602 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -430,7 +430,7 @@ static void prepare_elf_section_hdr_string(DumpState *s, void *buff)
>      memcpy(buff, shdr, shdr_size);
>  }
>  
> -static void prepare_elf_section_hdrs(DumpState *s)
> +static void prepare_elf_section_hdrs(DumpState *s, Error **errp)
>  {
>      size_t len, sizeof_shdr;
>      void *buff_hdr;
> @@ -438,6 +438,7 @@ static void prepare_elf_section_hdrs(DumpState *s)
>      /*
>       * Section ordering:
>       * - HDR zero
> +     * - Arch section hdrs
>       * - String table hdr
>       */
>      sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
> @@ -465,6 +466,16 @@ static void prepare_elf_section_hdrs(DumpState *s)
>          return;
>      }
>  
> +    /* Add architecture defined section headers */
> +    if (s->dump_info.arch_sections_write_hdr_fn) {
> +        buff_hdr += s->dump_info.arch_sections_write_hdr_fn(s, buff_hdr);
> +
> +        if (s->shdr_num >= SHN_LORESERVE) {

               /* TODO: raise limit by encoding via sh_link */

> +            error_setg_errno(errp, EINVAL, "dump: too many architecture defined sections");
> +            return;
> +        }
> +    }
> +
>      /*
>       * String table is the last section since strings are added via
>       * arch_sections_write_hdr().
> @@ -477,7 +488,10 @@ static void write_elf_section_headers(DumpState *s, Error **errp)
>      size_t sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
>      int ret;
>  
> -    prepare_elf_section_hdrs(s);
> +    prepare_elf_section_hdrs(s, errp);
> +    if (*errp) {

You're depending on errp not being NULL here, which it isn't, but it
doesn't seem like good style to me.
error.h recommends to also indicate success/failure via the return
value if possible. prepare_elf_section_hdrs returns void right now, so
it's easy so side step it this way.
(ERRP_GUARD would work too of course)

> +        return;
> +    }
>  
>      ret = fd_write_vmcore(s->elf_section_hdrs, s->shdr_num * sizeof_shdr, s);
>      if (ret < 0) {
> @@ -485,6 +499,30 @@ static void write_elf_section_headers(DumpState *s, Error **errp)
>      }
>  }
>  
> +static void write_elf_sections(DumpState *s, Error **errp)
> +{
> +    int ret;
> +
> +    if (!s->elf_section_data_size) {
> +        return;
> +    }
> +
> +    /* Write architecture section data */
> +    ret = fd_write_vmcore(s->elf_section_data,
> +                          s->elf_section_data_size, s);
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "dump: failed to write architecture section  data");

Looks like two spaces between section and data.

> +        return;
> +    }
> +
> +    /* Write string table */
> +    ret = fd_write_vmcore(s->string_table_buf->data,
> +                          s->string_table_buf->len, s);
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "dump: failed to write string table data");
> +    }
> +}
> +
>  static void write_data(DumpState *s, void *buf, int length, Error **errp)
>  {
>      int ret;
> @@ -744,6 +782,24 @@ static void dump_iterate(DumpState *s, Error **errp)
>      }
>  }
>  
> +static void dump_end(DumpState *s, Error **errp)
> +{
> +    ERRP_GUARD();
> +
> +    if (!s->elf_section_data_size) {
> +        return;
> +    }
> +    s->elf_section_data = g_malloc0(s->elf_section_data_size);

Why zero initialize the memory, do you depend on that?

> +
> +    /* Adds the architecture defined section data to s->elf_section_data  */
> +    if (s->dump_info.arch_sections_write_fn) {
> +        s->dump_info.arch_sections_write_fn(s, s->elf_section_data);
> +    }
> +
> +    /* write sections to vmcore */
> +    write_elf_sections(s, errp);
> +}
> +
>  static void create_vmcore(DumpState *s, Error **errp)
>  {
>      ERRP_GUARD();
> @@ -758,6 +814,9 @@ static void create_vmcore(DumpState *s, Error **errp)
>      if (*errp) {
>          return;
>      }
> +
> +    /* Write the section data */
> +    dump_end(s, errp);
>  }
>  
>  static int write_start_flat_header(int fd)
> @@ -1883,38 +1942,53 @@ static void dump_init(DumpState *s, int fd, bool has_format,
>      }
>  
>      /*
> -     * calculate phdr_num
> -     *
> -     * the type of ehdr->e_phnum is uint16_t, so we should avoid overflow
> +     * Adds the number of architecture sections to shdr_num, sets
> +     * string_table_usage and sets elf_section_data_size so we know
> +     * the offsets and sizes of all parts.
>       */
> -    s->phdr_num = 1; /* PT_NOTE */
> -    if (s->list.num < UINT16_MAX - 2) {
> -        s->shdr_num = 0;
> -        s->phdr_num += s->list.num;
> -    } else {
> -        /* sh_info of section 0 holds the real number of phdrs */
> -        s->shdr_num = 1;
> +    if (s->dump_info.arch_sections_add_fn) {
> +        s->dump_info.arch_sections_add_fn(s);
>  
> -        /* the type of shdr->sh_info is uint32_t, so we should avoid overflow */
> -        if (s->list.num <= UINT32_MAX - 1) {
> -            s->phdr_num += s->list.num;
> -        } else {
> -            s->phdr_num = UINT32_MAX;
> +        if (s->shdr_num) {
> +            s->string_table_usage = true;
>          }
>      }

I'm not convinced that this is the right patch for the stuff below.

>      /*
> -     * calculate shdr_num and elf_section_data_size so we know the offsets and
> -     * sizes of all parts.
> +     * Calculate phdr_num
>       *
> -     * If phdr_num overflowed we have at least one section header
> -     * More sections/hdrs can be added by the architectures
> +     * The absolute maximum amount of phdrs is UINT32_MAX - 1 as
> +     * sh_info is 32 bit. There's special handling once we go over
> +     * UINT16_MAX - 1 but that is handled in the ehdr and section
> +     * code.
>       */
> -    if (s->shdr_num > 1) {
> -        /* Reserve the string table */
> +    s->phdr_num = 1; /* Reserve PT_NOTE */
> +    if (s->list.num <= UINT32_MAX - 1) {
> +        s->phdr_num += s->list.num;
> +    } else {
> +        s->phdr_num = UINT32_MAX;
> +    }
> +
> +    /*
> +     * The first section header is always a special one in which most
> +     * fields are 0.
> +     *
> +     * We need it if we have custom sections and if phdr_num >=
> +     * PN_XNUM so we can write the real phdr_num into sh_info.
> +     */
> +    if (s->shdr_num || s->phdr_num >= PN_XNUM) {
>          s->shdr_num += 1;
>      }
>  
> +    /* Reserve the string table for the arch sections if needed */
> +    if (s->string_table_usage) {
> +        s->shdr_num += 1;
> +    }
> +
> +    /*
> +     * Now that the number of section and program headers is known we
> +     * can calculate the offsets of the headers and data.
> +     */
>      if (dump_is_64bit(s)) {
>          s->shdr_offset = sizeof(Elf64_Ehdr);
>          s->phdr_offset = s->shdr_offset + sizeof(Elf64_Shdr) * s->shdr_num;
> diff --git a/include/sysemu/dump-arch.h b/include/sysemu/dump-arch.h
> index e25b02e990..824c0c1152 100644
> --- a/include/sysemu/dump-arch.h
> +++ b/include/sysemu/dump-arch.h
> @@ -21,6 +21,9 @@ typedef struct ArchDumpInfo {
>      uint32_t page_size;      /* The target's page size. If it's variable and
>                                * unknown, then this should be the maximum. */
>      uint64_t phys_base;      /* The target's physmem base. */
> +    void (*arch_sections_add_fn)(DumpState *s);
> +    uint64_t (*arch_sections_write_hdr_fn)(DumpState *s, uint8_t *buff);
> +    void (*arch_sections_write_fn)(DumpState *s, uint8_t *buff);
>  } ArchDumpInfo;
>  
>  struct GuestPhysBlockList; /* memory_mapping.h */

Patch

diff --git a/dump/dump.c b/dump/dump.c
index 0d6dbf453a..65b18fc602 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -430,7 +430,7 @@  static void prepare_elf_section_hdr_string(DumpState *s, void *buff)
     memcpy(buff, shdr, shdr_size);
 }
 
-static void prepare_elf_section_hdrs(DumpState *s)
+static void prepare_elf_section_hdrs(DumpState *s, Error **errp)
 {
     size_t len, sizeof_shdr;
     void *buff_hdr;
@@ -438,6 +438,7 @@  static void prepare_elf_section_hdrs(DumpState *s)
     /*
      * Section ordering:
      * - HDR zero
+     * - Arch section hdrs
      * - String table hdr
      */
     sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
@@ -465,6 +466,16 @@  static void prepare_elf_section_hdrs(DumpState *s)
         return;
     }
 
+    /* Add architecture defined section headers */
+    if (s->dump_info.arch_sections_write_hdr_fn) {
+        buff_hdr += s->dump_info.arch_sections_write_hdr_fn(s, buff_hdr);
+
+        if (s->shdr_num >= SHN_LORESERVE) {
+            error_setg_errno(errp, EINVAL, "dump: too many architecture defined sections");
+            return;
+        }
+    }
+
     /*
      * String table is the last section since strings are added via
      * arch_sections_write_hdr().
@@ -477,7 +488,10 @@  static void write_elf_section_headers(DumpState *s, Error **errp)
     size_t sizeof_shdr = dump_is_64bit(s) ? sizeof(Elf64_Shdr) : sizeof(Elf32_Shdr);
     int ret;
 
-    prepare_elf_section_hdrs(s);
+    prepare_elf_section_hdrs(s, errp);
+    if (*errp) {
+        return;
+    }
 
     ret = fd_write_vmcore(s->elf_section_hdrs, s->shdr_num * sizeof_shdr, s);
     if (ret < 0) {
@@ -485,6 +499,30 @@  static void write_elf_section_headers(DumpState *s, Error **errp)
     }
 }
 
+static void write_elf_sections(DumpState *s, Error **errp)
+{
+    int ret;
+
+    if (!s->elf_section_data_size) {
+        return;
+    }
+
+    /* Write architecture section data */
+    ret = fd_write_vmcore(s->elf_section_data,
+                          s->elf_section_data_size, s);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "dump: failed to write architecture section  data");
+        return;
+    }
+
+    /* Write string table */
+    ret = fd_write_vmcore(s->string_table_buf->data,
+                          s->string_table_buf->len, s);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "dump: failed to write string table data");
+    }
+}
+
 static void write_data(DumpState *s, void *buf, int length, Error **errp)
 {
     int ret;
@@ -744,6 +782,24 @@  static void dump_iterate(DumpState *s, Error **errp)
     }
 }
 
+static void dump_end(DumpState *s, Error **errp)
+{
+    ERRP_GUARD();
+
+    if (!s->elf_section_data_size) {
+        return;
+    }
+    s->elf_section_data = g_malloc0(s->elf_section_data_size);
+
+    /* Adds the architecture defined section data to s->elf_section_data  */
+    if (s->dump_info.arch_sections_write_fn) {
+        s->dump_info.arch_sections_write_fn(s, s->elf_section_data);
+    }
+
+    /* write sections to vmcore */
+    write_elf_sections(s, errp);
+}
+
 static void create_vmcore(DumpState *s, Error **errp)
 {
     ERRP_GUARD();
@@ -758,6 +814,9 @@  static void create_vmcore(DumpState *s, Error **errp)
     if (*errp) {
         return;
     }
+
+    /* Write the section data */
+    dump_end(s, errp);
 }
 
 static int write_start_flat_header(int fd)
@@ -1883,38 +1942,53 @@  static void dump_init(DumpState *s, int fd, bool has_format,
     }
 
     /*
-     * calculate phdr_num
-     *
-     * the type of ehdr->e_phnum is uint16_t, so we should avoid overflow
+     * Adds the number of architecture sections to shdr_num, sets
+     * string_table_usage and sets elf_section_data_size so we know
+     * the offsets and sizes of all parts.
      */
-    s->phdr_num = 1; /* PT_NOTE */
-    if (s->list.num < UINT16_MAX - 2) {
-        s->shdr_num = 0;
-        s->phdr_num += s->list.num;
-    } else {
-        /* sh_info of section 0 holds the real number of phdrs */
-        s->shdr_num = 1;
+    if (s->dump_info.arch_sections_add_fn) {
+        s->dump_info.arch_sections_add_fn(s);
 
-        /* the type of shdr->sh_info is uint32_t, so we should avoid overflow */
-        if (s->list.num <= UINT32_MAX - 1) {
-            s->phdr_num += s->list.num;
-        } else {
-            s->phdr_num = UINT32_MAX;
+        if (s->shdr_num) {
+            s->string_table_usage = true;
         }
     }
 
     /*
-     * calculate shdr_num and elf_section_data_size so we know the offsets and
-     * sizes of all parts.
+     * Calculate phdr_num
      *
-     * If phdr_num overflowed we have at least one section header
-     * More sections/hdrs can be added by the architectures
+     * The absolute maximum amount of phdrs is UINT32_MAX - 1 as
+     * sh_info is 32 bit. There's special handling once we go over
+     * UINT16_MAX - 1 but that is handled in the ehdr and section
+     * code.
      */
-    if (s->shdr_num > 1) {
-        /* Reserve the string table */
+    s->phdr_num = 1; /* Reserve PT_NOTE */
+    if (s->list.num <= UINT32_MAX - 1) {
+        s->phdr_num += s->list.num;
+    } else {
+        s->phdr_num = UINT32_MAX;
+    }
+
+    /*
+     * The first section header is always a special one in which most
+     * fields are 0.
+     *
+     * We need it if we have custom sections and if phdr_num >=
+     * PN_XNUM so we can write the real phdr_num into sh_info.
+     */
+    if (s->shdr_num || s->phdr_num >= PN_XNUM) {
         s->shdr_num += 1;
     }
 
+    /* Reserve the string table for the arch sections if needed */
+    if (s->string_table_usage) {
+        s->shdr_num += 1;
+    }
+
+    /*
+     * Now that the number of section and program headers is known we
+     * can calculate the offsets of the headers and data.
+     */
     if (dump_is_64bit(s)) {
         s->shdr_offset = sizeof(Elf64_Ehdr);
         s->phdr_offset = s->shdr_offset + sizeof(Elf64_Shdr) * s->shdr_num;
diff --git a/include/sysemu/dump-arch.h b/include/sysemu/dump-arch.h
index e25b02e990..824c0c1152 100644
--- a/include/sysemu/dump-arch.h
+++ b/include/sysemu/dump-arch.h
@@ -21,6 +21,9 @@  typedef struct ArchDumpInfo {
     uint32_t page_size;      /* The target's page size. If it's variable and
                               * unknown, then this should be the maximum. */
     uint64_t phys_base;      /* The target's physmem base. */
+    void (*arch_sections_add_fn)(DumpState *s);
+    uint64_t (*arch_sections_write_hdr_fn)(DumpState *s, uint8_t *buff);
+    void (*arch_sections_write_fn)(DumpState *s, uint8_t *buff);
 } ArchDumpInfo;
 
 struct GuestPhysBlockList; /* memory_mapping.h */