diff mbox

[v4,03/11] livepatch: Include sizes when an mismatch occurs

Message ID 20170920223148.13137-4-konrad.wilk@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Konrad Rzeszutek Wilk Sept. 20, 2017, 10:31 p.m. UTC
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.

Also fix one case where we would fail if the size of the .ex_table
was being zero - but that is OK.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>

v1: First posting.
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.
v3: - Drop bool zero_ok
    - Return bool instead of int (and invert the return condition)
    - Change name of the function to be more clear
---
 xen/common/livepatch.c       | 46 +++++++++++++++++++++-----------------------
 xen/include/xen/elfstructs.h |  2 ++
 2 files changed, 24 insertions(+), 24 deletions(-)

Comments

Jan Beulich Sept. 21, 2017, 11:58 a.m. UTC | #1
>>> On 21.09.17 at 00:31, <konrad@kernel.org> wrote:
> 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.
> 
> Also fix one case where we would fail if the size of the .ex_table
> was being zero - but that is OK.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
Ross Lagerwall Oct. 5, 2017, 2:06 p.m. UTC | #2
On 09/20/2017 11:31 PM, Konrad Rzeszutek Wilk wrote:
> 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.
> 
> Also fix one case where we would fail if the size of the .ex_table
> was being zero - but that is OK.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> 

Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
diff mbox

Patch

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index b0dcd415ba..b9376c94e9 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -460,6 +460,22 @@  static int secure_payload(struct payload *payload, struct livepatch_elf *elf)
     return rc;
 }
 
+static bool section_ok(const struct livepatch_elf *elf,
+                       const struct livepatch_elf_sec *sec, size_t sz)
+{
+    if ( !elf || !sec )
+        return false;
+
+    if ( 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 false;
+    }
+
+    return true;
+}
+
 static int check_special_sections(const struct livepatch_elf *elf)
 {
     unsigned int i;
@@ -509,12 +525,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 ( !section_ok(elf, sec, sizeof(*payload->funcs)) )
         return -EINVAL;
-    }
 
     payload->funcs = sec->load_addr;
     payload->nfuncs = sec->sec->sh_size / sizeof(*payload->funcs);
@@ -556,7 +568,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 ( !section_ok(elf, sec, sizeof(*payload->load_funcs)) )
             return -EINVAL;
 
         payload->load_funcs = sec->load_addr;
@@ -566,7 +578,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 ( !section_ok(elf, sec, sizeof(*payload->unload_funcs)) )
             return -EINVAL;
 
         payload->unload_funcs = sec->load_addr;
@@ -637,12 +649,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 ( !section_ok(elf, sec, sizeof(*region->frame[i].bugs)) )
             return -EINVAL;
-        }
 
         region->frame[i].bugs = sec->load_addr;
         region->frame[i].n_bugs = sec->sec->sh_size /
@@ -655,12 +663,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 ( !section_ok(elf, sec, sizeof(*a)) )
             return -EINVAL;
-        }
 
         start = sec->load_addr;
         end = sec->load_addr + sec->sec->sh_size;
@@ -692,14 +696,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 ( !section_ok(elf, sec, sizeof(*region->ex)) )
             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