Re: [PATCH v3 07/17] livepatch/arm/x86: Strip note_depends symbol from test-cases.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Sep 13, 2017 at 02:51:41AM -0600, Jan Beulich wrote:
> >>> On 13.09.17 at 01:46, <konrad@kernel.org> wrote:
> > On Tue, Sep 12, 2017 at 08:48:33AM -0600, Jan Beulich wrote:
> >> >>> On 12.09.17 at 02:37, <konrad@kernel.org> wrote:
> >> > This surfaced due to "xen/livepatch/x86/arm32: Force
> >> > .livepatch.depends section to be uint32_t aligned." which switched
> >> > to a different way of including the build-id.
> >> > 
> >> > Each livepatch ends with a global:
> >> > 
> >> >     30: 00000000     1 OBJECT  GLOBAL HIDDEN     7 note_depends
> >> > 
> >> > which will cause collision when loading.
> >> > 
> >> > One attempted solution was to add in the Makefile stanza:
> >> >  @sed -i '/unsigned/static unsinged/' $@
> >> > 
> >> > But that resulted in the note_depends being omitted from the livepatch
> >> > (as it was static and not used) which meant we would not have an
> >> > .livepatch_depends section which we require.
> >> 
> >> Did you consider using objcopy's --localize-symbol instead?
> > 
> > Yes, so that note_depends is not globally visible. But that won't help
> > as hypervisor treats both local and global symbols as global when resolving
> > them.
> > 
> > That is each of the livepatch has the node_depends in it, and we can't
> > load xen_hello_world, followed by xen_replace_world test-case (so
> > stacking them on top of each other) - as both have the same local
> > symbol.
> 
> Oh, right. Then perhaps stripping the symbol is as good or as bad as
> deriving the symbol name from e.g. the patch name, or putting some
> randomized tag on it.

Yes, and I had in mind changing the name of it (to prefix it with the
livepatch name) using --redefine-sym.

But then figured it may be just easier to ditch the symbol altogether.

Let me try it out - I do wonder if that would remove the need
for stripping the debug symbols or if that would still trip the issue
I keep on having - which is that the debug section would reference the original
symbol.


> 
> > (This is fixed in "livepatch: Add local and global symbol resolution."
> > on which you said:
> > 
> > 	> 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.
> > 
> > (https://www.mail-archive.com/xen-devel@lists.xen.org/msg111710.html)
> 
> Right, this is a basically unresolvable ambiguity, I'm afraid. We'd
> need a 3rd class of symbols. It may be worth considering to (ab)use
> e.g. STV_INTERNAL for this purpose.

Oooooh. Let me look at that. Thank you!
> 
> Jan
> 

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