Re: [Xen-devel] [PATCH v1 5/5] xsplice: Use ld-embedded build-ids

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

On September 16, 2015 5:41:26 PM EDT, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
wrote:
>On 16/09/2015 22:01, Konrad Rzeszutek Wilk wrote:
>> From: Martin Pohlack <mpohlack@xxxxxxxxx>
>>
>> The mechanism to get this is via the XSPLICE_OP and
>> we add a new subsequent hypercall to retrieve the
>> binary build-id. The hypercall allows an arbirarty
>> size (the buffer is provided to the hypervisor) - however
>> by default the toolstack will allocate it up to 128
>> bytes.
>>
>> We also add two places for the build-id to be printed:
>>  - xsplice keyhandler. We cannot use 'hh' in the hypervisor
>>    snprintf handler (as it is not implemented) so instead
>>    we use an simpler way to print it.
>>  - In the 'xen-xsplice' tool add an extra parameter - build-id
>>    to print this as an human readable value.
>>
>> Note that one can also retrieve the value by 'readelf -h xen-syms'.
>>
>> Signed-off-by: Martin Pohlack <mpohlack@xxxxxxxxx>
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>> ---
>>  tools/libxc/include/xenctrl.h |  1 +
>>  tools/libxc/xc_misc.c         | 26 +++++++++++++
>>  tools/misc/xen-xsplice.c      | 39 ++++++++++++++++++++
>>  xen/arch/x86/Makefile         |  4 +-
>>  xen/arch/x86/xen.lds.S        |  5 +++
>>  xen/common/xsplice.c          | 86
>+++++++++++++++++++++++++++++++++++++++++++
>>  xen/include/public/sysctl.h   | 18 +++++++++
>>  xen/include/xen/version.h     |  1 +
>>  8 files changed, 178 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libxc/include/xenctrl.h
>b/tools/libxc/include/xenctrl.h
>> index 2cd982d..946ddc0 100644
>> --- a/tools/libxc/include/xenctrl.h
>> +++ b/tools/libxc/include/xenctrl.h
>> @@ -2860,6 +2860,7 @@ int xc_xsplice_apply(xc_interface *xch, char
>*id);
>>  int xc_xsplice_revert(xc_interface *xch, char *id);
>>  int xc_xsplice_unload(xc_interface *xch, char *id);
>>  int xc_xsplice_check(xc_interface *xch, char *id);
>> +int xc_xsplice_build_id(xc_interface *xch, char *build_id, unsigned
>int max);
>
>The build id of the current running hypervisor should belong in the
>xeninfo hypercall.  It is not specific to xsplice.


However in the previous reviews it was pointed out that it should only be 
accessible to dom0.

Or to any domains as long as the XSM allows (and is turned on) - so not the 
default dummy one.

That is a bit of 'if' extra complexity which I am not sure is worth it?

>
>> diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
>> index d330efe..5728c4b 100644
>> --- a/xen/common/xsplice.c
>> +++ b/xen/common/xsplice.c
>> @@ -14,6 +14,8 @@
>>  #include <xen/sched.h>
>>  #include <xen/lib.h>
>>  #include <xen/xsplice.h>
>> +#include <xen/elf.h>
>> +#include <xen/types.h>
>>  #include <public/sysctl.h>
>>  
>>  #include <asm/event.h>
>> @@ -44,6 +46,36 @@ struct payload {
>>      struct tasklet tasklet;
>>  };
>>  
>> +#ifdef CONFIG_ARM
>> +static int build_id(char **p, unsigned int *len)
>> +{
>> +    return 0;
>> +}
>> +#else
>> +#define NT_GNU_BUILD_ID 3
>> +
>> +extern char * __note_gnu_build_id_start;  /* defined in linker
>script */
>
>This is liable to cause you to deference the first 8 bytes of the note,
>as the symbol is not a char*.
>
>Use extern Elf_Note __note_gnu_build_id_start[]; instead.
>
>Also, it is probably worth having an _end symbol as well and check for
>end > start to confirm that the linker has actually put something in
>there.
>
>~Andrew



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