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

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

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@xxxxxxxxxx>
---
 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel