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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.06.17 at 04:47, <konrad.wilk@xxxxxxxxxx> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel