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

[Jan Beulich]

>>> On 16.09.15 at 23:01,  wrote:
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -108,11 +108,11 @@ $(TARGET)-syms: prelink.o xen.lds 
> $(BASEDIR)/common/symbols-dummy.o
>   $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
>   $(NM) -n $(@D)/.$(@F).0 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).0.S
>   $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
> - $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
> + $(LD) $(LDFLAGS) -T xen.lds -N prelink.o --build-id=sha1 \
>   $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
>   $(NM) -n $(@D)/.$(@F).1 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).1.S
>   $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
> - $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
> + $(LD) $(LDFLAGS) -T xen.lds -N prelink.o --build-id=sha1 \
>   $(@D)/.$(@F).1.o -o $@
>   rm -f $(@D)/.$(@F).[0-9]*

Which raises the question: What about xen.efi?

> @@ -44,6 +46,36 @@ struct payload {
>  struct tasklet tasklet;
>  };
>  
> +#ifdef CONFIG_ARM
> +static int build_id(char **p, unsigned int *len)
> +{
> +return 0;
> +}
> +#else

Any reason not to make the build logic common, in order to avoid such
#ifdef-ery?


> +extern char * __note_gnu_build_id_start;  /* defined in linker script */

const, and I think you mean the type to be Elf_Note[].

> @@ -68,9 +100,31 @@ static const char *status2str(int64_t status)
>  return names[status];
>  }
>  
> +#define LEN 128

Because of?

>  void xsplice_printall(unsigned char key)
>  {
>  struct payload *data;
> +char *binary_id = NULL;
> +unsigned int len = 0;
> +int rc;
> +
> +rc = build_id(_id, );
> +printk("build-id: ");
> +if ( !rc )
> +{
> +unsigned int i;
> +
> +if ( len > LEN )
> +len = LEN;
> +
> +for ( i = 0; i < len; i++ )
> +{
> + uint8_t c = binary_id[i];
> + printk("%02x", c);

Hard tabs.

> +}
> + printk("\n");
> +} else if ( rc < 0 )

Coding style.

> @@ -415,6 +470,34 @@ static int xsplice_action(xen_sysctl_xsplice_action_t 
> *action)
>  return rc;
>  }
>  
> +static int xsplice_info(xen_sysctl_xsplice_info_t *info)
> +{
> +int rc;
> +unsigned int len = 0;
> +char *p = NULL;
> +
> +if ( info->cmd != XEN_SYSCTL_XSPLICE_INFO_BUILD_ID )
> +return -EINVAL;
> +
> +if ( info->size == 0 )
> +return -EINVAL;
> +
> +if ( !guest_handle_okay(info->u.info, info->size) )
> +return -EFAULT;
> +
> +rc = build_id(, );
> +if ( rc )
> +return rc;
> +
> +if ( len > info->size )
> +return -ENOMEM;
> +
> +if ( copy_to_guest(info->u.info, p, len) )
> +return -EFAULT;

So how does the caller know how much data got copied?

Jan


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

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

[Martin Pohlack]

On 17.09.2015 00:31, Andrew Cooper wrote:
> On 16/09/2015 22:59, Konrad Rzeszutek Wilk wrote:
>> On September 16, 2015 5:41:26 PM EDT, Andrew Cooper 
>>  wrote:
>>> On 16/09/2015 22:01, Konrad Rzeszutek Wilk wrote:
 From: Martin Pohlack 

 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 
 Signed-off-by: Konrad Rzeszutek Wilk 
 ---
  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?
> 
> DomU can already read the build information such as changeset, compile
> time, etc.  Build-id is no more special or revealing.

I would take this as an argument *against* giving DomU access to those
pieces of information in details and not as an argument for
*additionally* giving it access to build-id.

With build-id we have the chance to shape a not-yet-established API and
I would like to follow the Principle of least privilege wherever it
makes sense.

To reach a similar security level with the existing API, it might make
sense to limit DomU access to compile date, compile time, compiled by,
compiled domain, compiler version and command line details, xen extra
version, and xen changeset.  Basically anything that might help DomUs to
uniquely identify a Xen build.

The approach can not directly drop access to those fields, as that would
break an existing API, but it could restrict the detail level handed out
to DomU.

Martin
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B


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

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

[Andrew Cooper]

On 17/09/15 07:41, Martin Pohlack wrote:
> On 17.09.2015 00:31, Andrew Cooper wrote:
>> On 16/09/2015 22:59, Konrad Rzeszutek Wilk wrote:
>>> On September 16, 2015 5:41:26 PM EDT, Andrew Cooper 
>>>  wrote:
 On 16/09/2015 22:01, Konrad Rzeszutek Wilk wrote:
> From: Martin Pohlack 
>
> 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 
> Signed-off-by: Konrad Rzeszutek Wilk 
> ---
>  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?
>> DomU can already read the build information such as changeset, compile
>> time, etc.  Build-id is no more special or revealing.
> I would take this as an argument *against* giving DomU access to those
> pieces of information in details and not as an argument for
> *additionally* giving it access to build-id.
>
> With build-id we have the chance to shape a not-yet-established API and
> I would like to follow the Principle of least privilege wherever it
> makes sense.
>
> To reach a similar security level with the existing API, it might make
> sense to limit DomU access to compile date, compile time, compiled by,
> compiled domain, compiler version and command line details, xen extra
> version, and xen changeset.  Basically anything that might help DomUs to
> uniquely identify a Xen build.
>
> The approach can not directly drop access to those fields, as that would
> break an existing API, but it could restrict the detail level handed out
> to DomU.

These are all valid arguments to be made, but please lets fix the issue
properly rather than hacking build-id on the side of an unrelated component.

>From my point of view, the correct course of action is this:

* Split the current XSM model to contain separate attributes for general
and privileged information.
** For current compatibility, all existing XENVER_* subops fall into general
* Apply an XSM check at the very start of the hypercall.
* Extend do_xen_version() to take 3 parameters.  (It is curious that it
didn't take a length parameter before)
** This is still ABI compatible, as existing subops simply ignore the
parameter.
* Introduce new XENVER_build_id subop which is documented to require the
3-parameter version of the hypercall.
** This subop falls into straight into privileged information.

This will introduce build-id in its correct location, with appropriate
restrictions.

Moving forwards, we really should have an audit of the existing XENVER_*
subops.  Some are clearly safe/required for domU to read, but some such
as XENVER_commandline have no business being accessible.  A separate
argument, from the repeatable build point of view, says that compilation
information isn't useful at all.

Depending on how we wish to fix the issue, we could either do a blanket
move of the subops 

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

[Andrew Cooper]

On 16/09/2015 22:59, Konrad Rzeszutek Wilk wrote:
> On September 16, 2015 5:41:26 PM EDT, Andrew Cooper 
>  wrote:
>> On 16/09/2015 22:01, Konrad Rzeszutek Wilk wrote:
>>> From: Martin Pohlack 
>>>
>>> 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 
>>> Signed-off-by: Konrad Rzeszutek Wilk 
>>> ---
>>>  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?

DomU can already read the build information such as changeset, compile
time, etc.  Build-id is no more special or revealing.

~Andrew

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

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

[Andrew Cooper]

On 16/09/2015 22:01, Konrad Rzeszutek Wilk wrote:
> From: Martin Pohlack 
>
> 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 
> Signed-off-by: Konrad Rzeszutek Wilk 
> ---
>  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.

> 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 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  
>  #include 
> @@ -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@lists.xen.org
http://lists.xen.org/xen-devel

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

[Konrad Rzeszutek Wilk]

From: Martin Pohlack 

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 
Signed-off-by: Konrad Rzeszutek Wilk 
---
 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);
 
 #endif /* XENCTRL_H */
 
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index e59f0a1..f67ff16 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -992,6 +992,32 @@ int xc_xsplice_check(xc_interface *xch, char *id)
 return _xc_xsplice_action(xch, id, XSPLICE_ACTION_CHECK);
 }
 
+int xc_xsplice_build_id(xc_interface *xch, char *build_id, unsigned int max)
+{
+int rc;
+DECLARE_SYSCTL;
+DECLARE_HYPERCALL_BOUNCE(build_id, max, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
+
+if ( !build_id || !max )
+return -1;
+
+if ( xc_hypercall_bounce_pre(xch, build_id) )
+return -1;
+
+sysctl.cmd = XEN_SYSCTL_xsplice_op;
+sysctl.u.xsplice.cmd = XEN_SYSCTL_XSPLICE_INFO;
+
+sysctl.u.xsplice.u.info.cmd = XEN_SYSCTL_XSPLICE_INFO_BUILD_ID;
+sysctl.u.xsplice.u.info.size = max;
+
+set_xen_guest_handle(sysctl.u.xsplice.u.info.u.info, build_id);
+
+rc = do_sysctl(xch, );
+
+xc_hypercall_bounce_post(xch, build_id);
+
+return rc;
+}
 /*
  * Local variables:
  * mode: C
diff --git a/tools/misc/xen-xsplice.c b/tools/misc/xen-xsplice.c
index d8bd222..609d799 100644
--- a/tools/misc/xen-xsplice.c
+++ b/tools/misc/xen-xsplice.c
@@ -17,6 +17,7 @@ void show_help(void)
 "  An unique name of payload. Up to %d characters.\n"
 "Commands:\n"
 "  help display this help\n"
+"  build-id display build-id of hypervisor.\n"
 "  upload upload file  with  name\n"
 "  list list payloads uploaded.\n"
 "  applyapply  patch.\n"
@@ -361,12 +362,50 @@ unload:
 return rc;
 }
 
+
+#define MAX_LEN 1024
+static int build_id_func(int argc, char *argv[])
+{
+char binary_id[MAX_LEN];
+char ascii_id[MAX_LEN];
+int rc;
+unsigned int i;
+
+if ( argc )
+{
+show_help();
+return -1;
+}
+
+memset(binary_id, 0, sizeof(binary_id));
+
+rc = xc_xsplice_build_id(xch, binary_id, MAX_LEN);
+if ( rc < 0 )
+{
+printf("Failed to get build_id: %d(%s)\n", errno, strerror(errno));
+return -1;
+}
+/* Convert to printable format. */
+if ( rc > MAX_LEN )
+rc = MAX_LEN;
+
+for ( i = 0; i < rc && (i + 1) * 2 < sizeof(binary_id); i++ )
+snprintf(_id[i * 2], 3, "%02hhx", binary_id[i]);
+
+ascii_id[i*2]='\0';
+printf("%s", ascii_id);
+
+return 0;
+}
+#undef MAX
+
 struct {
 const char *name;
 int (*function)(int argc, char *argv[]);
 } main_options[] = {
 { "help", help_func },
 { "list", list_func },
+{ "build-id", build_id_func },
 { "upload", upload_func },
 { "all", all_func },
 };
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 39a8059..de11910 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -108,11 +108,11 @@ $(TARGET)-syms: prelink.o xen.lds 
$(BASEDIR)/common/symbols-dummy.o
$(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
$(NM) -n $(@D)/.$(@F).0 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).0.S
$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
-   $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
+   $(LD) $(LDFLAGS) -T xen.lds -N 

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

[Konrad Rzeszutek Wilk]

On September 16, 2015 5:41:26 PM EDT, Andrew Cooper  
wrote:
>On 16/09/2015 22:01, Konrad Rzeszutek Wilk wrote:
>> From: Martin Pohlack 
>>
>> 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 
>> Signed-off-by: Konrad Rzeszutek Wilk 
>> ---
>>  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 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>>  #include 
>>  
>>  #include 
>> @@ -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@lists.xen.org
http://lists.xen.org/xen-devel