[PATCH] Livepatch: Support local/global symbols.

[PATCH] Livepatch: Support local/global symbols.

[Konrad Rzeszutek Wilk]

Heya,

Please see the following two patches:

 [PATCH v1 1/3] livepatch: Add local and global symbol resolution.
 [PATCH v1 2/3] livepatch: Add xen_local_symbols test-case

which depend on "xen/test/Makefile: Fix clean target, broken by pattern rule"
to install properly.

There is also an OSSTEST test-case:
 [PATCH v1 OSSTEST 3/3] ts-livepatch: Expand testcase to include global/local symbols

which expands the regression testing (and it is smart enough to not do the
test-case is the test-case file does not exist).

The Xen diff stat is as follow:

 xen/common/livepatch.c                 | 21 +++++++++-----
 xen/include/xen/livepatch.h            |  3 +-
 xen/include/xen/livepatch_elf.h        |  7 +++++
 xen/test/livepatch/Makefile            | 10 ++++++-
 xen/test/livepatch/xen_local_symbols.c | 52 ++++++++++++++++++++++++++++++++++
 5 files changed, 84 insertions(+), 9 deletions(-)


Konrad Rzeszutek Wilk (2):
      livepatch: Add local and global symbol resolution.
      livepatch: Add xen_local_symbols test-case


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

[PATCH v1 1/3] livepatch: Add local and global symbol resolution.

[Konrad Rzeszutek Wilk]

This way we can load livepatches with symbol names that
are the same as long as they are local ('static').

The use case here is to replace an existing livepatch
with a newer one - and one which has the same local symbols.

Without this patch we get:
livepatch.c:819: livepatch: xen_local_symbols: duplicate new symbol: revert_hook

when loading the new livepatch (before doing the replace).

This due to livepatch assuming that all symbols are all
global. With this patch:

readelf --symbols xen_hello_world.livepatch
File: xen_hello_world.livepatch

Symbol table '.symtab' contains 55 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
..snip..
    34: 0000000000000000     4 OBJECT  LOCAL  DEFAULT   25 cnt
    35: 000000000000000a     8 OBJECT  LOCAL  DEFAULT    7 __func__.4654
    36: 0000000000000065    23 FUNC    LOCAL  DEFAULT    2 revert_hook
    37: 000000000000007c    23 FUNC    LOCAL  DEFAULT    2 apply_hook
    38: 0000000000000093    54 FUNC    LOCAL  DEFAULT    2 check_fnc
..snip..

    47: 0000000000000000    54 FUNC    GLOBAL HIDDEN     2 xen_hello_world
    48: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN   UND xen_extra_version
..snip..
    52: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN   UND printk
    53: 0000000000000000    64 OBJECT  GLOBAL HIDDEN    23 livepatch_xen_hello_world

All the 'GLOBAL' have to be unique per livepatch. But the
'LOCAL' can all be the same which means the semantic of 'static'
on functions and data variables is the right one.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/common/livepatch.c          | 21 ++++++++++++++-------
 xen/include/xen/livepatch.h     |  3 ++-
 xen/include/xen/livepatch_elf.h |  7 +++++++
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index df67a1a..2a86218 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -181,7 +181,10 @@ unsigned long livepatch_symbols_lookup_by_name(const char *symname)
             if ( !data->symtab[i].new_symbol )
                 continue;
 
-            if ( !strcmp(data->symtab[i].name, symname) )
+            if ( strcmp(data->symtab[i].name, symname) )
+                continue;
+
+            if ( data->symtab[i].global_symbol )
                 return data->symtab[i].value;
         }
     }
@@ -791,6 +794,7 @@ static int build_symbol_table(struct payload *payload,
             symtab[nsyms].size = elf->sym[i].sym->st_size;
             symtab[nsyms].value = elf->sym[i].sym->st_value;
             symtab[nsyms].new_symbol = 0; /* May be overwritten below. */
+            symtab[nsyms].global_symbol = livepatch_sym_is_global(elf->sym[i].sym);
             strtab_len += strlcpy(strtab + strtab_len, elf->sym[i].name,
                                   KSYM_NAME_LEN) + 1;
             nsyms++;
@@ -815,21 +819,24 @@ static int build_symbol_table(struct payload *payload,
             if ( symbols_lookup_by_name(symtab[i].name) ||
                  livepatch_symbols_lookup_by_name(symtab[i].name) )
             {
-                dprintk(XENLOG_ERR, LIVEPATCH "%s: duplicate new symbol: %s\n",
-                        elf->name, symtab[i].name);
+                dprintk(XENLOG_ERR, LIVEPATCH "%s: duplicate new %s symbol: %s\n",
+                        elf->name, symtab[i].global_symbol ? "global" : "local",
+                        symtab[i].name);
                 xfree(symtab);
                 xfree(strtab);
                 return -EEXIST;
             }
             symtab[i].new_symbol = 1;
-            dprintk(XENLOG_DEBUG, LIVEPATCH "%s: new symbol %s\n",
-                     elf->name, symtab[i].name);
+            dprintk(XENLOG_DEBUG, LIVEPATCH "%s: new %s symbol %s\n",
+                     elf->name, symtab[i].global_symbol ? "global" : "local",
+                     symtab[i].name);
         }
         else
         {
             /* new_symbol is not set. */
-            dprintk(XENLOG_DEBUG, LIVEPATCH "%s: overriding symbol %s\n",
-                    elf->name, symtab[i].name);
+            dprintk(XENLOG_DEBUG, LIVEPATCH "%s: overriding %s symbol %s\n",
+                    elf->name, symtab[i].global_symbol ? "global" : "local",
+                    symtab[i].name);
         }
     }
 
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 98ec012..fccff94 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -38,7 +38,8 @@ struct livepatch_symbol {
     const char *name;
     unsigned long value;
     unsigned int size;
-    bool_t new_symbol;
+    unsigned int new_symbol:1;
+    unsigned int global_symbol:1;
 };
 
 int livepatch_op(struct xen_sysctl_livepatch_op *);
diff --git a/xen/include/xen/livepatch_elf.h b/xen/include/xen/livepatch_elf.h
index 9ad499e..870c4bc 100644
--- a/xen/include/xen/livepatch_elf.h
+++ b/xen/include/xen/livepatch_elf.h
@@ -50,6 +50,13 @@ static inline bool livepatch_elf_ignore_section(const Elf_Shdr *sec)
 {
     return !(sec->sh_flags & SHF_ALLOC) || sec->sh_size == 0;
 }
+
+static inline bool livepatch_sym_is_global(const Elf_Sym *sym)
+{
+    return ((ELF_ST_BIND(sym->st_info) & STB_GLOBAL) &&
+            (sym->st_shndx != SHN_UNDEF));
+}
+
 #endif /* __XEN_LIVEPATCH_ELF_H__ */
 
 /*
-- 
2.9.4


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

[PATCH v1 2/3] livepatch: Add xen_local_symbols test-case

[Konrad Rzeszutek Wilk]

To exercise the local/global visibility.

With "livepatch: Add local and global symbol resolution."
we can load both xen_hello_world and xen_local_symbols
without having to worry about:

-bash-4.1# xen-livepatch load xen_hello_world.livepatch
Uploading xen_hello_world.livepatch... completed
Applying xen_hello_world... completed
-bash-4.1# xen-livepatch list
 ID                                     | status
----------------------------------------+------------
xen_hello_world                         | APPLIED
-bash-4.1# xen-livepatch upload xen_local_symbols xen_local_symbols.livepatch
Uploading xen_local_symbols.livepatch... failed
(XEN) livepatch.c:819: livepatch: xen_local_symbols: duplicate new symbol: revert_hook

In fact you will see:

livepatch: xen_hello_world: new local symbol revert_hook
livepatch: xen_hello_world: new local symbol apply_hook
livepatch: xen_hello_world: new local symbol check_fnc
livepatch: xen_hello_world: new local symbol hello_world_patch_this_fnc

...
livepatch: xen_local_symbols: new local symbol revert_hook
livepatch: xen_local_symbols: new local symbol apply_hook
livepatch: xen_local_symbols: new local symbol hello_world_patch_this_fnc
..

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/test/livepatch/Makefile            | 10 ++++++-
 xen/test/livepatch/xen_local_symbols.c | 52 ++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 xen/test/livepatch/xen_local_symbols.c

diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index 6831383d..b9ad291 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -19,11 +19,13 @@ LIVEPATCH := xen_hello_world.livepatch
 LIVEPATCH_BYE := xen_bye_world.livepatch
 LIVEPATCH_REPLACE := xen_replace_world.livepatch
 LIVEPATCH_NOP := xen_nop.livepatch
+LIVEPATCH_LOCAL := xen_local_symbols.livepatch
 
 LIVEPATCHES += $(LIVEPATCH)
 LIVEPATCHES += $(LIVEPATCH_BYE)
 LIVEPATCHES += $(LIVEPATCH_REPLACE)
 LIVEPATCHES += $(LIVEPATCH_NOP)
+LIVEPATCHES += $(LIVEPATCH_LOCAL)
 
 LIVEPATCH_DEBUG_DIR ?= $(DEBUG_DIR)/xen-livepatch
 
@@ -107,5 +109,11 @@ xen_nop.o: config.h
 $(LIVEPATCH_NOP): xen_nop.o note.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NOP) $^
 
+xen_local_symbols.o: config.h
+
+.PHONY: $(LIVEPATCH_LOCAL)
+$(LIVEPATCH_LOCAL): xen_hello_world_func.o xen_local_symbols.o note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_LOCAL) $^
+
 .PHONY: livepatch
-livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP)
+livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP) $(LIVEPATCH_LOCAL)
diff --git a/xen/test/livepatch/xen_local_symbols.c b/xen/test/livepatch/xen_local_symbols.c
new file mode 100644
index 0000000..eb01b69
--- /dev/null
+++ b/xen/test/livepatch/xen_local_symbols.c
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2017 Oracle and/or its affiliates. All rights reserved.
+ */
+
+#include "config.h"
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <xen/version.h>
+#include <xen/livepatch.h>
+#include <xen/livepatch_payload.h>
+
+#include <public/sysctl.h>
+
+/* Same name as in xen_hello_world */
+static const char hello_world_patch_this_fnc[] = "xen_extra_version";
+extern const char *xen_hello_world(void);
+
+/*
+ * The hooks are static here (LOCAL) and also in xen_hello_world.c
+ * and their name is exactly the same.
+ */
+static void apply_hook(void)
+{
+    printk(KERN_DEBUG "local_symbols: Hook executing.\n");
+}
+
+static void revert_hook(void)
+{
+    printk(KERN_DEBUG "local_symbols: Hook unloaded.\n");
+}
+
+LIVEPATCH_LOAD_HOOK(apply_hook);
+LIVEPATCH_UNLOAD_HOOK(revert_hook);
+
+struct livepatch_func __section(".livepatch.funcs") livepatch_xen_local_symbols = {
+    .version = LIVEPATCH_PAYLOAD_VERSION,
+    .name = hello_world_patch_this_fnc,
+    .new_addr = xen_hello_world,
+    .old_addr = xen_extra_version,
+    .new_size = NEW_CODE_SZ,
+    .old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.9.4


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

[PATCH v1 OSSTEST 3/3] ts-livepatch: Expand testcase to include global/local symbols

[Konrad Rzeszutek Wilk]

testing. The test is to verify that the local symbols
of payloads are ignored during loading.

See Xen's patch "livepatch: Add local and global symbol resolution."

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 ts-livepatch-run | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/ts-livepatch-run b/ts-livepatch-run
index f119458..1eaceff 100755
--- a/ts-livepatch-run
+++ b/ts-livepatch-run
@@ -27,6 +27,7 @@ my @livepatch_files = qw(xen_hello_world.livepatch
                          xen_bye_world.livepatch
 			 xen_nop.livepatch);
 
+my $livepatch_dir="/usr/lib/debug/xen-livepatch";
 my $xen_extra_info;
 my $xen_minor_ver;
 
@@ -128,6 +129,24 @@ my @livepatch_tests = (
     { C => "xen-livepatch unload xen_nop" },
     );
 
+my @livepatch_tests_symbols = (
+    { C => "xen-livepatch load xen_hello_world.livepatch" },
+    { C => "xen-livepatch list",
+      OutputCheck => sub { m/xen_hello_world/ } },
+    { C => "xl info",
+      OutputCheck => \&check_for_hello_world },
+    # This would normally fail as xen_local_symbols has the same local
+    # symbols as xen_hello_world.livepatch but with
+    # "livepatch: Add local and global symbol resolution." it works.
+    { C => "xen-livepatch upload xen_local_symbols xen_local_symbols.livepatch" },
+    { C => "xen-livepatch replace xen_local_symbols" },
+    { C => "xl info",
+      OutputCheck => \&check_for_hello_world },
+    { C => "xen-livepatch revert xen_local_symbols" },
+    { C => "xen-livepatch unload xen_hello_world" },
+    { C => "xen-livepatch unload xen_local_symbols" },
+    );
+
 our $ho = selecthost('host');
 
 sub livepatch_test () {
@@ -138,7 +157,7 @@ sub livepatch_test () {
     foreach my $test (@livepatch_tests) {
         # Default rc is zero.
         my $expected_rc = defined($test->{rc}) ? $test->{rc} : 0;
-        my $cmd = "(set -e;cd /usr/lib/debug/xen-livepatch;$test->{C})";
+        my $cmd = "(set -e;cd $livepatch_dir;$test->{C})";
         my ($rc, $output) = target_cmd_output_root_status($ho, $cmd);
 
         if ($rc != $expected_rc) {
@@ -158,10 +177,13 @@ sub livepatch_test () {
 
 sub livepatch_check () {
     foreach my $file (@livepatch_files) {
-        if (!target_file_exists($ho, "/usr/lib/debug/xen-livepatch/$$file")) {
+        if (!target_file_exists($ho, "$$livepatch_dir/$$file")) {
             die "$file is missing!\n";
         }
     }
+    if (target_file_exists($ho, "$livepatch_dir/xen_local_symbols.livepatch")) {
+        @livepatch_tests = (@livepatch_tests, @livepatch_tests_symbols);
+    }
 }
 
 
-- 
2.1.4


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

Re: [PATCH v1 1/3] livepatch: Add local and global symbol resolution.

[Jan Beulich]

>>> On 20.06.17 at 04:47, <konrad.wilk@oracle.com> wrote:
> This way we can load livepatches with symbol names that
> are the same as long as they are local ('static').
> 
> The use case here is to replace an existing livepatch
> with a newer one - and one which has the same local symbols.
> 
> Without this patch we get:
> livepatch.c:819: livepatch: xen_local_symbols: duplicate new symbol: 
> revert_hook
> 
> when loading the new livepatch (before doing the replace).
> 
> This due to livepatch assuming that all symbols are all
> global. With this patch:
> 
> readelf --symbols xen_hello_world.livepatch
> File: xen_hello_world.livepatch
> 
> Symbol table '.symtab' contains 55 entries:
>    Num:    Value          Size Type    Bind   Vis      Ndx Name
> ..snip..
>     34: 0000000000000000     4 OBJECT  LOCAL  DEFAULT   25 cnt
>     35: 000000000000000a     8 OBJECT  LOCAL  DEFAULT    7 __func__.4654
>     36: 0000000000000065    23 FUNC    LOCAL  DEFAULT    2 revert_hook
>     37: 000000000000007c    23 FUNC    LOCAL  DEFAULT    2 apply_hook
>     38: 0000000000000093    54 FUNC    LOCAL  DEFAULT    2 check_fnc
> ..snip..
> 
>     47: 0000000000000000    54 FUNC    GLOBAL HIDDEN     2 xen_hello_world
>     48: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN   UND xen_extra_version
> ..snip..
>     52: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN   UND printk
>     53: 0000000000000000    64 OBJECT  GLOBAL HIDDEN    23 livepatch_xen_hello_world
> 
> All the 'GLOBAL' have to be unique per livepatch. But the
> 'LOCAL' can all be the same which means the semantic of 'static'
> on functions and data variables is the right one.

I think this is wrong: Afaict your change results in main image and
patch local symbols to now be treated differently. While this may
indeed help patches which are meant to replace others, it is going
to get in the way if a patch wants to reference a local symbol
already altered (or newly introduced) by a prior one.

Question then is at what point in time name collisions should be
detected: In order for patch replacement to work, this obviously
shouldn't happen at the time the patch is being loaded, but in the
course of being applied. Otoh iirc "replace" doesn't use separate
old and new patch names, so the system must already be aware
of the correlation between them, and hence collision detection at
patch load time may still be possible (special casing the patch
being replaced).

Jan


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

Re: [PATCH v1 OSSTEST 3/3] ts-livepatch: Expand testcase to include global/local symbols

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("[PATCH v1 OSSTEST 3/3] ts-livepatch: Expand testcase to include global/local symbols"):
> testing. The test is to verify that the local symbols
> of payloads are ignored during loading.

Can we do this with substeps rather than a conditional test
execution ?

For example, if xen.git should suddenly stop producing this file, it
ought to be a test failure.

Maybe we should make each test into a separate testid ?

Ian.

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

Re: [PATCH v1 1/3] livepatch: Add local and global symbol resolution.

[Konrad Rzeszutek Wilk]

On Tue, Jun 20, 2017 at 01:51:41AM -0600, Jan Beulich wrote:
> >>> On 20.06.17 at 04:47, <konrad.wilk@oracle.com> wrote:
> > This way we can load livepatches with symbol names that
> > are the same as long as they are local ('static').
> > 
> > The use case here is to replace an existing livepatch
> > with a newer one - and one which has the same local symbols.
> > 
> > Without this patch we get:
> > livepatch.c:819: livepatch: xen_local_symbols: duplicate new symbol: 
> > revert_hook
> > 
> > when loading the new livepatch (before doing the replace).
> > 
> > This due to livepatch assuming that all symbols are all
> > global. With this patch:
> > 
> > readelf --symbols xen_hello_world.livepatch
> > File: xen_hello_world.livepatch
> > 
> > Symbol table '.symtab' contains 55 entries:
> >    Num:    Value          Size Type    Bind   Vis      Ndx Name
> > ..snip..
> >     34: 0000000000000000     4 OBJECT  LOCAL  DEFAULT   25 cnt
> >     35: 000000000000000a     8 OBJECT  LOCAL  DEFAULT    7 __func__.4654
> >     36: 0000000000000065    23 FUNC    LOCAL  DEFAULT    2 revert_hook
> >     37: 000000000000007c    23 FUNC    LOCAL  DEFAULT    2 apply_hook
> >     38: 0000000000000093    54 FUNC    LOCAL  DEFAULT    2 check_fnc
> > ..snip..
> > 
> >     47: 0000000000000000    54 FUNC    GLOBAL HIDDEN     2 xen_hello_world
> >     48: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN   UND xen_extra_version
> > ..snip..
> >     52: 0000000000000000     0 NOTYPE  GLOBAL HIDDEN   UND printk
> >     53: 0000000000000000    64 OBJECT  GLOBAL HIDDEN    23 livepatch_xen_hello_world
> > 
> > All the 'GLOBAL' have to be unique per livepatch. But the
> > 'LOCAL' can all be the same which means the semantic of 'static'
> > on functions and data variables is the right one.
> 
> I think this is wrong: Afaict your change results in main image and
> patch local symbols to now be treated differently. While this may
> indeed help patches which are meant to replace others, it is going
> to get in the way if a patch wants to reference a local symbol
> already altered (or newly introduced) by a prior one.
> 
> Question then is at what point in time name collisions should be
> detected: In order for patch replacement to work, this obviously
> shouldn't happen at the time the patch is being loaded, but in the
> course of being applied. Otoh iirc "replace" doesn't use separate
> old and new patch names, so the system must already be aware
> of the correlation between them, and hence collision detection at
> patch load time may still be possible (special casing the patch
> being replaced).

This gets complicated, let me break it down to make sure I got it right.

I think what you are saying is that after a livepatch has been
applied the distinction of global/local should be lifted for
that livepatch.

But before we even get to that stage we have to deal with
the situation in which two (or more) livepatches have identical
local symbols ('revert_hook' for example). And there the
local symbols collision should not happen.

Now you mention an interesting case "if a patch wants to reference
a local symbol already altered (or newly introduced) by a prior one"

Let me break that apart, the "(or newly introduced)" meaning
we would have say two livepatches already applied.
The first one adds this new symbol and the second one
depends on the first livepatch (and uses the symbol).
We are trying to replace the second one with a newer version. 

[I remember talking to Ross about this: 
https://lists.xen.org/archives/html/xen-devel/2016-08/msg01756.html]

If the newly introduced symbol is local, the second livepatch
should _not_ be able to resolve it [But it does today, this patch
fixes this].

If the newly introduced symbol is global, the second livepatch
should be able to resolve it [and it does today]

If this symbol we want to reference is not within the live
patches, but in the main code - then:

If the symbol is local, we will still find it (symbols_lookup_by_name)
If the symbol is global, we will still find it (symbols_lookup_by_name)

Ah, so your point is that since the main code does not provide
_any_ semantics about local/global symbols, then by extension
the livepatches should neither.

While my patch does introduce this distinction?

Hmm.

Well, what I can say is that this issue (local symbol collision
in the livepatches) has been bitting us since November.

Our mechanism when deploying livepatches is to replace the loaded
livepatch with another one. Which means we only have on livepatch
applied and during the upgrade process have to load another one.

In November (XSA-204) we started shipping in the livepatches
a new local symbol (x86_emulate.c#_get_fpu).

livepatch: 617712b: new symbol x86_emulate.c#_get_fpu

Then when the next XSA came about and we made a new livepatch that
included everything prior and the new one, and we got:

livepatch.c:751: livepatch: fbab204: duplicate new symbol: x86_emulate.c#_get_fpu

which aborted the replacement of the livepatch.

The solution at hand was to rename the offending local symbol to
something more unique (the name of the livepatch):

livepatch: fbab204: new symbol x86_emulate.c#_get_fpu_fbab204

Which we are doing now for every livepatch. However I would
like a better mechanism, which is why I am proposing this patch.

P.S.
[Note to myself: Include this long explanation in the commit description]


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

Re: [PATCH v1 1/3] livepatch: Add local and global symbol resolution.

[Jan Beulich]

>>> On 20.06.17 at 17:59, <konrad.wilk@oracle.com> wrote:
> Our mechanism when deploying livepatches is to replace the loaded
> livepatch with another one. Which means we only have on livepatch
> applied and during the upgrade process have to load another one.

I think this is the main problematic part here: You're trying to fix one
use case (single patch being replaced every time) while - afaict - you
break the other one (multiple stacked patches). For your case,
wouldn't it be sufficient to load the patch with some flag indicating
that all symbol handling (resolution as well as insertion) should only
consider the main image? Of course with that flag, a plain "apply"
would need to fail as long as there's any other patch loaded.

The downside of this is that it would special case things a little too
much for my taste. For example I'd expect to also be able to have
a couple of stacked patches and replace just the topmost one(s).
That would require more than a boolean flag to tell which symbols
to consider and which to ignore.

Jan


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