Re: [Xen-devel] [PATCH v2 07/20] arm/x86: Add ALTERNATIVE and HAS_EX_TABLE

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Thu, Aug 25, 2016 at 08:56:07AM -0600, Jan Beulich wrote:
> >>> On 25.08.16 at 15:58, <andrew.cooper3@xxxxxxxxxx> wrote:
> > On 25/08/16 14:37, Konrad Rzeszutek Wilk wrote:
> > 
> >> x86 implements all of them by default - and we just
> >> add two extra CONFIG_ variables to be declared in autoconf.h.
> >>
> >> ARM 64 only has alternative while ARM 32 has none of them.
> >>
> >> And while at it change the livepatch common code that
> >> would benefit from this.
> >>
> >> Suggested-by: Julien Grall <julien.grall@xxxxxxx>
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > 
> > Surely livepatch should select alternatives ?
> 
> DYM depend on? It clearly shouldn't select them, as whether
> they're present is determined by arch code.

And I am not sure if there is a need for Kconfig LIVEPATCH
entry to depend on it?

Especially as the common/livepatch.c has #ifef CONFIG_ALTERNATIVE
(and also CONFIG_HAS_EX_TABLE) - added by this patch - to deal with
architectures that do not have support them.

Ssuch as ARM 32 or ARM64 without errata support built-in.

I spun out an seperate patch that would thrown an -EOPNOTSUPP
if these sections are part of the payload and the hypervisor
was built without the support, like this:

(This is to be on top of the patch discussed - or if folks
prefer I can squash it in).

From 3a4953dd0f7d2411e5d638a82bfe57c0e16a22b3 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Tue, 6 Sep 2016 16:28:23 -0400
Subject: [PATCH] livepatch: Reject payloads with .alternative or .ex_table if
 support is not built-in.

ARM 64 can be built without alternative support (without the
errata support) - and it would sad if the payload loaded
had .alternative section but we did not support parsing it.

As this could lead to sad results instead of ignoring this
lets error out during loading.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

---
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

v3: New submission.
---
 xen/common/livepatch.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 73d4edb..342a5ec 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -685,10 +685,10 @@ static int prepare_payload(struct payload *payload,
                                   sizeof(*region->frame[i].bugs);
     }
 
-#ifdef CONFIG_HAS_ALTERNATIVE
     sec = livepatch_elf_sec_by_name(elf, ".altinstructions");
     if ( sec )
     {
+#ifdef CONFIG_HAS_ALTERNATIVE
         struct alt_instr *a, *start, *end;
 
         if ( sec->sec->sh_size % sizeof(*a) )
@@ -715,13 +715,17 @@ static int prepare_payload(struct payload *payload,
             }
         }
         apply_alternatives(start, end);
-    }
+#else
+        dprintk(XENLOG_ERR, LIVEPATCH "%s: We don't support alternative 
patching!\n",
+                elf->name);
+        return -EOPNOTSUPP;
 #endif
+    }
 
-#ifdef CONFIG_HAS_EX_TABLE
     sec = livepatch_elf_sec_by_name(elf, ".ex_table");
     if ( sec )
     {
+#ifdef CONFIG_HAS_EX_TABLE
         struct exception_table_entry *s, *e;
 
         if ( !sec->sec->sh_size ||
@@ -740,8 +744,12 @@ static int prepare_payload(struct payload *payload,
 
         region->ex = s;
         region->ex_end = e;
-    }
+#else
+        dprintk(XENLOG_ERR, LIVEPATCH "%s: We don't support .ex_table!\n",
+                elf->name);
+        return -EOPNOTSUPP;
 #endif
+    }
 
     return 0;
 }
-- 
2.4.11


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel