diff mbox series

[v10,7/8] linux-user/elfload: Parse NT_GNU_PROPERTY_TYPE_0 notes

Message ID 20201002215955.254866-8-richard.henderson@linaro.org
State New
Headers show
Series linux-user: User support for AArch64 BTI | expand

Commit Message

Richard Henderson Oct. 2, 2020, 9:59 p.m. UTC
For aarch64, this includes the GNU_PROPERTY_AARCH64_FEATURE_1_BTI bit,
which indicates that the image should be mapped with guarded pages.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
v9: Only map the startup executable with BTI; anything else must be
    handled by the interpreter.
v10: Split out preparatory patches (pmm).
---
 linux-user/qemu.h    |  4 +++
 linux-user/elfload.c | 68 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 70 insertions(+), 2 deletions(-)

Comments

Peter Maydell Oct. 8, 2020, 2:02 p.m. UTC | #1
On Fri, 2 Oct 2020 at 23:00, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> For aarch64, this includes the GNU_PROPERTY_AARCH64_FEATURE_1_BTI bit,

> which indicates that the image should be mapped with guarded pages.

>

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

> ---

> v9: Only map the startup executable with BTI; anything else must be

>     handled by the interpreter.

> v10: Split out preparatory patches (pmm).


> @@ -2467,6 +2467,50 @@ static void load_elf_image(const char *image_name, int image_fd,

>                  goto exit_errmsg;

>              }

>              *pinterp_name = interp_name;

> +        } else if (eppnt->p_type == PT_GNU_PROPERTY) {

> +            /* Process NT_GNU_PROPERTY_TYPE_0. */

> +            const uint32_t gnu0_magic = const_le32('G' | 'N' << 8 | 'U' << 16);

> +            uint32_t note[7];

> +

> +            /*

> +             * The note contents are 7 words, but depending on LP64 vs ILP32

> +             * there may be an 8th padding word at the end.  Check for and

> +             * read the minimum size.  Further checks below will validate

> +             * that the sizes of everything involved are as we expect.

> +             */

> +            if (eppnt->p_filesz < sizeof(note)) {

> +                continue;

> +            }

> +            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {

> +                memcpy(note, bprm_buf + eppnt->p_offset, sizeof(note));

> +            } else {

> +                retval = pread(image_fd, note, sizeof(note), eppnt->p_offset);

> +                if (retval != sizeof(note)) {

> +                    goto exit_perror;

> +                }

> +            }

> +#ifdef BSWAP_NEEDED

> +            for (i = 0; i < ARRAY_SIZE(note); ++i) {

> +                bswap32s(note + i);

> +            }

> +#endif

> +            /*

> +             * Check that this is a NT_GNU_PROPERTY_TYPE_0 note.

> +             * Again, descsz includes padding.  Full size validation

> +             * awaits checking the final payload.

> +             */

> +            if (note[0] != 4 ||                       /* namesz */

> +                note[1] < 12 ||                       /* descsz */

> +                note[2] != NT_GNU_PROPERTY_TYPE_0 ||  /* type */

> +                note[3] != gnu0_magic) {              /* name */


note[2] and note[3] are both basically magic numbers, AIUI.
Why do we have a #define for one but we assemble the other
with a const_le32() expression ?

> +                continue;

> +            }

> +#ifdef TARGET_AARCH64

> +            if (note[4] == GNU_PROPERTY_AARCH64_FEATURE_1_AND &&

> +                note[5] == 4) {

> +                info->note_flags = note[6];

> +            }


The spec for the .note.gnu.property section (which AIUI is
https://raw.githubusercontent.com/wiki/hjl-tools/linux-abi/linux-abi-draft.pdf
) says that the n_desc (words 4 and up) is an array of program
properties. There doesn't seem to be any guarantee that there
is only one entry or that the FEATURE_1_AND entry is the first
in the list. Don't we need to iterate through the array to find
matches? This seems to be how the kernel does it:
 https://elixir.bootlin.com/linux/latest/source/fs/binfmt_elf.c#L786

(Is it worth adding the infrastructure to parse notes generically
the way the kernel has? I dunno if we think it's likely we'll
want to do this for more note types and/or other architectures
in future, so it might just be pointless complexity.)

thanks
-- PMM
Richard Henderson Oct. 8, 2020, 5:13 p.m. UTC | #2
On 10/8/20 9:02 AM, Peter Maydell wrote:
>> +            if (note[0] != 4 ||                       /* namesz */

>> +                note[1] < 12 ||                       /* descsz */

>> +                note[2] != NT_GNU_PROPERTY_TYPE_0 ||  /* type */

>> +                note[3] != gnu0_magic) {              /* name */

> 

> note[2] and note[3] are both basically magic numbers, AIUI.

> Why do we have a #define for one but we assemble the other

> with a const_le32() expression ?


Because one is defined as a number, and the other is defined as a string.  And
why *that* is, I don't know.  Silliness, perhaps.

> The spec for the .note.gnu.property section (which AIUI is

> https://raw.githubusercontent.com/wiki/hjl-tools/linux-abi/linux-abi-draft.pdf

> ) says that the n_desc (words 4 and up) is an array of program

> properties. There doesn't seem to be any guarantee that there

> is only one entry or that the FEATURE_1_AND entry is the first

> in the list. Don't we need to iterate through the array to find

> matches? This seems to be how the kernel does it:

>  https://elixir.bootlin.com/linux/latest/source/fs/binfmt_elf.c#L786


Hmm.  I missed that change since the first time I looked at the in-flight patch
set.

> (Is it worth adding the infrastructure to parse notes generically

> the way the kernel has? I dunno if we think it's likely we'll

> want to do this for more note types and/or other architectures

> in future, so it might just be pointless complexity.)


I dunno about that either.  I'm not really sure what "generically" would look
like without another exemplar.  I'll look at what else
arch_parse_elf_property() is being used for.


r~
diff mbox series

Patch

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 941ca99722..534753ca12 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -61,6 +61,10 @@  struct image_info {
         abi_ulong       interpreter_loadmap_addr;
         abi_ulong       interpreter_pt_dynamic_addr;
         struct image_info *other_info;
+
+        /* For target-specific processing of NT_GNU_PROPERTY_TYPE_0. */
+        uint32_t        note_flags;
+
 #ifdef TARGET_MIPS
         int             fp_abi;
         int             interp_fp_abi;
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 6b422990ff..3c6cbd35c3 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2391,7 +2391,7 @@  static void load_elf_image(const char *image_name, int image_fd,
     struct elfhdr *ehdr = (struct elfhdr *)bprm_buf;
     struct elf_phdr *phdr;
     abi_ulong load_addr, load_bias, loaddr, hiaddr, error;
-    int i, retval;
+    int i, retval, prot_exec;
     const char *errmsg;
 
     /* First of all, some simple consistency checks */
@@ -2467,6 +2467,50 @@  static void load_elf_image(const char *image_name, int image_fd,
                 goto exit_errmsg;
             }
             *pinterp_name = interp_name;
+        } else if (eppnt->p_type == PT_GNU_PROPERTY) {
+            /* Process NT_GNU_PROPERTY_TYPE_0. */
+            const uint32_t gnu0_magic = const_le32('G' | 'N' << 8 | 'U' << 16);
+            uint32_t note[7];
+
+            /*
+             * The note contents are 7 words, but depending on LP64 vs ILP32
+             * there may be an 8th padding word at the end.  Check for and
+             * read the minimum size.  Further checks below will validate
+             * that the sizes of everything involved are as we expect.
+             */
+            if (eppnt->p_filesz < sizeof(note)) {
+                continue;
+            }
+            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
+                memcpy(note, bprm_buf + eppnt->p_offset, sizeof(note));
+            } else {
+                retval = pread(image_fd, note, sizeof(note), eppnt->p_offset);
+                if (retval != sizeof(note)) {
+                    goto exit_perror;
+                }
+            }
+#ifdef BSWAP_NEEDED
+            for (i = 0; i < ARRAY_SIZE(note); ++i) {
+                bswap32s(note + i);
+            }
+#endif
+            /*
+             * Check that this is a NT_GNU_PROPERTY_TYPE_0 note.
+             * Again, descsz includes padding.  Full size validation
+             * awaits checking the final payload.
+             */
+            if (note[0] != 4 ||                       /* namesz */
+                note[1] < 12 ||                       /* descsz */
+                note[2] != NT_GNU_PROPERTY_TYPE_0 ||  /* type */
+                note[3] != gnu0_magic) {              /* name */
+                continue;
+            }
+#ifdef TARGET_AARCH64
+            if (note[4] == GNU_PROPERTY_AARCH64_FEATURE_1_AND &&
+                note[5] == 4) {
+                info->note_flags = note[6];
+            }
+#endif /* TARGET_AARCH64 */
         }
     }
 
@@ -2555,6 +2599,26 @@  static void load_elf_image(const char *image_name, int image_fd,
     info->brk = 0;
     info->elf_flags = ehdr->e_flags;
 
+    prot_exec = PROT_EXEC;
+#ifdef TARGET_AARCH64
+    /*
+     * If the BTI feature is present, this indicates that the executable
+     * pages of the startup binary should be mapped with PROT_BTI, so that
+     * branch targets are enforced.
+     *
+     * The startup binary is either the interpreter or the static executable.
+     * The interpreter is responsible for all pages of a dynamic executable.
+     *
+     * Elf notes are backward compatible to older cpus.
+     * Do not enable BTI unless it is supported.
+     */
+    if ((info->note_flags & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)
+        && (pinterp_name == NULL || *pinterp_name == 0)
+        && cpu_isar_feature(aa64_bti, ARM_CPU(thread_cpu))) {
+        prot_exec |= TARGET_PROT_BTI;
+    }
+#endif
+
     for (i = 0; i < ehdr->e_phnum; i++) {
         struct elf_phdr *eppnt = phdr + i;
         if (eppnt->p_type == PT_LOAD) {
@@ -2568,7 +2632,7 @@  static void load_elf_image(const char *image_name, int image_fd,
                 elf_prot |= PROT_WRITE;
             }
             if (eppnt->p_flags & PF_X) {
-                elf_prot |= PROT_EXEC;
+                elf_prot |= prot_exec;
             }
 
             vaddr = load_bias + eppnt->p_vaddr;