[v2,2/5] livepatch: Include sizes when an mismatch occurs

Commit Message

Konrad Rzeszutek Wilk July 26, 2017, 7:47 p.m. UTC

From: Konrad Rzeszutek Wilk <[e-mail]>

If the .bug.frames.X or .livepatch.funcs sizes are different
than what the hypervisor expects - we fail the payload. To help
in diagnosing this include the expected and the payload
sizes.

Also make it more natural by having "Multiples" in the warning.

Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>
---
v1: Initial version
v2: - Changed to 'Multiple' per Jan's recommendation.
    - Folded the checks in 'check_size' function and removed all the other
      parts of code that checked for this.
---
 xen/common/livepatch.c       | 48 ++++++++++++++++++++++----------------------
 xen/include/xen/elfstructs.h |  2 ++
 2 files changed, 26 insertions(+), 24 deletions(-)

Comments

Jan Beulich July 31, 2017, 1:51 p.m. UTC | #1

>>> Konrad Rzeszutek Wilk <[e-mail]> 07/26/17 9:48 PM >>>
>--- a/xen/common/livepatch.c
>+++ b/xen/common/livepatch.c
>@@ -457,6 +457,24 @@ static int secure_payload(struct payload *payload, struct livepatch_elf *elf)
>return rc;
>}
 >
>+static int check_section(const struct livepatch_elf *elf,
>+                         const struct livepatch_elf_sec *sec,
>+                         const size_t sz, bool zero_ok)

I guess you want to drop the const here (or else for consistency add one to the
last parameter). As to the last parameter - I doubt its usefulness: There's one
place where you pass false, and that place looks bogus. I don't see anything
wrong with an empty .ex_table section.

>+{
>+    if ( !elf || !sec )
>+        return -EINVAL;

None of the callers actually uses the return value. Perhaps the function should
return bool?

Jan

Patch

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 66d532db14..40ff6b3a27 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -457,6 +457,24 @@  static int secure_payload(struct payload *payload, struct livepatch_elf *elf)
     return rc;
 }
 
+static int check_section(const struct livepatch_elf *elf,
+                         const struct livepatch_elf_sec *sec,
+                         const size_t sz, bool zero_ok)
+{
+    if ( !elf || !sec )
+        return -EINVAL;
+
+    if ( (!sec->sec->sh_size && !zero_ok) ||
+        (sec->sec->sh_size % sz) )
+    {
+        dprintk(XENLOG_ERR, LIVEPATCH "%s: Wrong size %"PRIuElfWord" of %s (must be multiple of %zu)\n",
+                elf->name, sec->sec->sh_size, sec->name, sz);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static int check_special_sections(const struct livepatch_elf *elf)
 {
     unsigned int i;
@@ -506,12 +524,8 @@  static int prepare_payload(struct payload *payload,
 
     sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_FUNC);
     ASSERT(sec);
-    if ( sec->sec->sh_size % sizeof(*payload->funcs) )
-    {
-        dprintk(XENLOG_ERR, LIVEPATCH "%s: Wrong size of "ELF_LIVEPATCH_FUNC"!\n",
-                elf->name);
+    if ( check_section(elf, sec, sizeof(*payload->funcs), true) )
         return -EINVAL;
-    }
 
     payload->funcs = sec->load_addr;
     payload->nfuncs = sec->sec->sh_size / sizeof(*payload->funcs);
@@ -553,7 +567,7 @@  static int prepare_payload(struct payload *payload,
     sec = livepatch_elf_sec_by_name(elf, ".livepatch.hooks.load");
     if ( sec )
     {
-        if ( sec->sec->sh_size % sizeof(*payload->load_funcs) )
+        if ( check_section(elf, sec, sizeof(*payload->load_funcs), true) )
             return -EINVAL;
 
         payload->load_funcs = sec->load_addr;
@@ -563,7 +577,7 @@  static int prepare_payload(struct payload *payload,
     sec = livepatch_elf_sec_by_name(elf, ".livepatch.hooks.unload");
     if ( sec )
     {
-        if ( sec->sec->sh_size % sizeof(*payload->unload_funcs) )
+        if ( check_section(elf, sec, sizeof(*payload->unload_funcs), true) )
             return -EINVAL;
 
         payload->unload_funcs = sec->load_addr;
@@ -634,12 +648,8 @@  static int prepare_payload(struct payload *payload,
         if ( !sec )
             continue;
 
-        if ( sec->sec->sh_size % sizeof(*region->frame[i].bugs) )
-        {
-            dprintk(XENLOG_ERR, LIVEPATCH "%s: Wrong size of .bug_frames.%u!\n",
-                    elf->name, i);
+        if ( check_section(elf, sec, sizeof(*region->frame[i].bugs), true) )
             return -EINVAL;
-        }
 
         region->frame[i].bugs = sec->load_addr;
         region->frame[i].n_bugs = sec->sec->sh_size /
@@ -652,12 +662,8 @@  static int prepare_payload(struct payload *payload,
 #ifdef CONFIG_HAS_ALTERNATIVE
         struct alt_instr *a, *start, *end;
 
-        if ( sec->sec->sh_size % sizeof(*a) )
-        {
-            dprintk(XENLOG_ERR, LIVEPATCH "%s: Size of .alt_instr is not multiple of %zu!\n",
-                    elf->name, sizeof(*a));
+        if ( check_section(elf, sec, sizeof(*a), true) )
             return -EINVAL;
-        }
 
         start = sec->load_addr;
         end = sec->load_addr + sec->sec->sh_size;
@@ -689,14 +695,8 @@  static int prepare_payload(struct payload *payload,
 #ifdef CONFIG_HAS_EX_TABLE
         struct exception_table_entry *s, *e;
 
-        if ( !sec->sec->sh_size ||
-             (sec->sec->sh_size % sizeof(*region->ex)) )
-        {
-            dprintk(XENLOG_ERR, LIVEPATCH "%s: Wrong size of .ex_table (exp:%lu vs %lu)!\n",
-                    elf->name, sizeof(*region->ex),
-                    sec->sec->sh_size);
+        if ( check_section(elf, sec, sizeof(*region->ex), false) )
             return -EINVAL;
-        }
 
         s = sec->load_addr;
         e = sec->load_addr + sec->sec->sh_size;
diff --git a/xen/include/xen/elfstructs.h b/xen/include/xen/elfstructs.h
index 950e1492e5..726ca8f60d 100644
--- a/xen/include/xen/elfstructs.h
+++ b/xen/include/xen/elfstructs.h
@@ -555,6 +555,7 @@  typedef struct {
 
 #if defined(ELFSIZE) && (ELFSIZE == 32)
 #define PRIxElfAddr	"08x"
+#define PRIuElfWord	"8u"
 
 #define Elf_Ehdr	Elf32_Ehdr
 #define Elf_Phdr	Elf32_Phdr
@@ -582,6 +583,7 @@  typedef struct {
 #define AuxInfo		Aux32Info
 #elif defined(ELFSIZE) && (ELFSIZE == 64)
 #define PRIxElfAddr	PRIx64
+#define PRIuElfWord	PRIu64
 
 #define Elf_Ehdr	Elf64_Ehdr
 #define Elf_Phdr	Elf64_Phdr