[v3,08/18] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x]

Commit Message

Konrad Rzeszutek Wilk Sept. 11, 2016, 8:35 p.m. UTC

Those symbols are used to help final linkers to replace insn.
The ARM ELF specification mandates that they are present
to denote the start of certain CPU features. There are two
variants of it - short and long format.

Either way - we can ignore these symbols.

Reviewed-by: Andrew Cooper <[e-mail]> [x86 bits]
Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>

---
Cc: Konrad Rzeszutek Wilk <[e-mail]>
Cc: Ross Lagerwall <[e-mail]>
Cc: Stefano Stabellini <[e-mail]>
Cc: Julien Grall <[e-mail]
Cc: Jan Beulich <[e-mail]>
Cc: Andrew Cooper <[e-mail]>

v1: First submission
v2: Update the order of symbols, fix title
    Add {} in after the first if - per Jan's recommendation.
v3: Add Andrew's Review tag
    Make the function return an bool_t.
    Skip check for '$t'
    Fix spelling of comments.
    s/arch_is_payload_symbol/arch_livepatch_symbol_ok/
---
 xen/arch/arm/livepatch.c    | 33 +++++++++++++++++++++++++++++++++
 xen/arch/x86/livepatch.c    |  7 +++++++
 xen/common/livepatch.c      |  2 +-
 xen/include/xen/livepatch.h |  2 ++
 4 files changed, 43 insertions(+), 1 deletion(-)

Comments

Jan Beulich Sept. 12, 2016, 3:52 p.m. UTC | #1

>>> On 11.09.16 at 22:35, <[e-mail]> wrote:
> Those symbols are used to help final linkers to replace insn.
> The ARM ELF specification mandates that they are present
> to denote the start of certain CPU features. There are two
> variants of it - short and long format.
> 
> Either way - we can ignore these symbols.
> 
> Reviewed-by: Andrew Cooper <[e-mail]> [x86 bits]
> Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>
> 
> ---
> Cc: Konrad Rzeszutek Wilk <[e-mail]>
> Cc: Ross Lagerwall <[e-mail]>
> Cc: Stefano Stabellini <[e-mail]>
> Cc: Julien Grall <[e-mail] 
> Cc: Jan Beulich <[e-mail]>
> Cc: Andrew Cooper <[e-mail]>
> 
> v1: First submission
> v2: Update the order of symbols, fix title
>     Add {} in after the first if - per Jan's recommendation.
> v3: Add Andrew's Review tag
>     Make the function return an bool_t.

Yet it should have been bool.

Jan

Patch

diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index 7a3e2f7..d2ae128 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -84,6 +84,39 @@  void arch_livepatch_unmask(void)
     local_abort_enable();
 }
 
+bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
+                                const struct livepatch_elf_sym *sym)
+{
+    /*
+     * - Mapping symbols - denote the "start of a sequence of bytes of the
+     *   appropriate type" to mark certain features - such as start of region
+     *   containing data ($d); ARM ($a), or A64 ($x) instructions.
+     *   We ignore Thumb instructions ($t) as we shouldn't have them.
+     *
+     * The format is either short: '$x' or long: '$x.<any>'. We do not
+     * need this and more importantly - each payload will contain this
+     * resulting in symbol collisions.
+     */
+    if ( *sym->name == '$' && sym->name[1] != '\0' )
+    {
+        char p = sym->name[1];
+        size_t len = strlen(sym->name);
+
+        if ( (len >= 3 && ( sym->name[2] == '.' )) || (len == 2) )
+        {
+            if ( p == 'd' ||
+#ifdef CONFIG_ARM_32
+                 p == 'a'
+#else
+                 p == 'x'
+#endif
+               )
+                return false;
+        }
+    }
+    return true;
+}
+
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
                                const struct livepatch_elf_sec *base,
                                const struct livepatch_elf_sec *rela)
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index cf3eb22..bbcb5fe 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -118,6 +118,13 @@  int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
     return 0;
 }
 
+bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
+                                const struct livepatch_elf_sym *sym)
+{
+    /* No special checks on x86. */
+    return true;
+}
+
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
                                const struct livepatch_elf_sec *base,
                                const struct livepatch_elf_sec *rela)
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 4f156c2..3c726a4 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -734,7 +734,7 @@  static bool_t is_payload_symbol(const struct livepatch_elf *elf,
          !strncmp(sym->name, ".L", 2) )
         return 0;
 
-    return 1;
+    return arch_livepatch_symbol_ok(elf, sym);
 }
 
 static int build_symbol_table(struct payload *payload,
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 904785a..c67b02c 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -46,6 +46,8 @@  bool_t is_patch(const void *addr);
 
 /* Arch hooks. */
 int arch_livepatch_verify_elf(const struct livepatch_elf *elf);
+bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
+                                const struct livepatch_elf_sym *sym);
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
                                const struct livepatch_elf_sec *base,
                                const struct livepatch_elf_sec *rela);