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

[Konrad Rzeszutek Wilk]

On Fri, Aug 14, 2015 at 9:57 AM, Martin Pohlack  wrote:
> On 14.08.2015 15:54, Jan Beulich wrote:
> On 14.08.15 at 14:59,  wrote:
>>> On 11.08.2015 16:12, Jan Beulich wrote:
>>> On 05.08.15 at 16:09,  wrote:
> Todo:
>   * Should be moved to sysctl to only allow Dom0 access

 Because of?
>>>
>>> The discussion in this thread:
>>>
>>> [Xen-devel] [RFC PATCH v3.1 2/2] xsplice: Add hook for build_id
>>>
>>> was:
>>> --
> Martin Pohlack:
> We should not expose the build_id to normal guests, but only to Dom0.
>
> A build_id uniquely identifies a specific build and I don't see how that
> information would be required from DomU.  It might actually help an
> attacker to build his return-oriented programming exploit against a
> specific build.
>
> The normal version numbers should be enough to know about capabilities
> and API.

 Andrew Cooper:

 It will need its own XSM hook, but need not be strictly limited to just
 dom0.
>>> --
>>
>> So I'm confused - I asked "why Dom0 only" and then you point me to
>> Andrew saying it doesn't need to be Dom0 only?
>
> Sorry about that, my (not expressed) thinking was that we should
> restrict that to Dom0 for the XSM-disabled case.
>

That may make this more complex.

If we want to restrict it to this we may as well just stick this in sysctl
and have it be part of the xsplice ops.

Let me do that.

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

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

[Martin Pohlack]

On 14.08.2015 15:54, Jan Beulich wrote:
 On 14.08.15 at 14:59, mpohl...@amazon.com wrote:
 On 11.08.2015 16:12, Jan Beulich wrote:
 On 05.08.15 at 16:09, mpohl...@amazon.de wrote:
 Todo:
   * Should be moved to sysctl to only allow Dom0 access

 Because of?

 The discussion in this thread:

 [Xen-devel] [RFC PATCH v3.1 2/2] xsplice: Add hook for build_id

 was:
 --
 Martin Pohlack:
 We should not expose the build_id to normal guests, but only to Dom0.

 A build_id uniquely identifies a specific build and I don't see how that
 information would be required from DomU.  It might actually help an
 attacker to build his return-oriented programming exploit against a
 specific build.

 The normal version numbers should be enough to know about capabilities
 and API.

 Andrew Cooper:

 It will need its own XSM hook, but need not be strictly limited to just
 dom0.
 --
 
 So I'm confused - I asked why Dom0 only and then you point me to
 Andrew saying it doesn't need to be Dom0 only?

Sorry about that, my (not expressed) thinking was that we should
restrict that to Dom0 for the XSM-disabled case.

 @@ -360,11 +366,30 @@ DO(xen_version)(int cmd, 
 XEN_GUEST_HANDLE_PARAM(void) arg)
  
  case XENVER_build_id:
  {
 -xen_build_id_t build_id;
 +xen_build_id_t ascii_id;
 +Elf_Note * n = (Elf_Note *)__note_gnu_build_id_start;
 +char * binary_id;
 +int i;
 +
 +memset(ascii_id, 0, sizeof(ascii_id));
 +
 +/* check if we really have a build-id */
 +if ( NT_GNU_BUILD_ID != n-type )
 +return 0;

 This needs to signal an error.

 Yes, ENOSYS, (or ENOENT, ENODATA)?
 
 Definitely not ENOSYS. ENODATA or EOPNOTSUPP.
 
 Jan
 

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] xsplice: Use ld-embedded build-ids

[Jan Beulich]

 On 14.08.15 at 14:59, mpohl...@amazon.com wrote:
 On 11.08.2015 16:12, Jan Beulich wrote:
 On 05.08.15 at 16:09, mpohl...@amazon.de wrote:
 Todo:
   * Should be moved to sysctl to only allow Dom0 access
 
 Because of?
 
 The discussion in this thread:
 
 [Xen-devel] [RFC PATCH v3.1 2/2] xsplice: Add hook for build_id
 
 was:
 --
 Martin Pohlack:
 We should not expose the build_id to normal guests, but only to Dom0.

 A build_id uniquely identifies a specific build and I don't see how that
 information would be required from DomU.  It might actually help an
 attacker to build his return-oriented programming exploit against a
 specific build.

 The normal version numbers should be enough to know about capabilities
 and API.

 Andrew Cooper:
 
 It will need its own XSM hook, but need not be strictly limited to just
 dom0.
 --

So I'm confused - I asked why Dom0 only and then you point me to
Andrew saying it doesn't need to be Dom0 only?

 @@ -360,11 +366,30 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
 arg)
  
  case XENVER_build_id:
  {
 -xen_build_id_t build_id;
 +xen_build_id_t ascii_id;
 +Elf_Note * n = (Elf_Note *)__note_gnu_build_id_start;
 +char * binary_id;
 +int i;
 +
 +memset(ascii_id, 0, sizeof(ascii_id));
 +
 +/* check if we really have a build-id */
 +if ( NT_GNU_BUILD_ID != n-type )
 +return 0;
 
 This needs to signal an error.
 
 Yes, ENOSYS, (or ENOENT, ENODATA)?

Definitely not ENOSYS. ENODATA or EOPNOTSUPP.

Jan


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

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

[Martin Pohlack]

On 11.08.2015 16:12, Jan Beulich wrote:
 On 05.08.15 at 16:09, mpohl...@amazon.de wrote:
 Todo:
   * Should be moved to sysctl to only allow Dom0 access
 
 Because of?

The discussion in this thread:

[Xen-devel] [RFC PATCH v3.1 2/2] xsplice: Add hook for build_id

was:
--
 Martin Pohlack:
 We should not expose the build_id to normal guests, but only to Dom0.

 A build_id uniquely identifies a specific build and I don't see how that
 information would be required from DomU.  It might actually help an
 attacker to build his return-oriented programming exploit against a
 specific build.

 The normal version numbers should be enough to know about capabilities
 and API.

 Andrew Cooper:
 
 It will need its own XSM hook, but need not be strictly limited to just
 dom0.
--

   * Maybe convert to binary transport to userland instead of printable form
 
 Indeed.
 
 @@ -360,11 +366,30 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
 arg)
  
  case XENVER_build_id:
  {
 -xen_build_id_t build_id;
 +xen_build_id_t ascii_id;
 +Elf_Note * n = (Elf_Note *)__note_gnu_build_id_start;
 +char * binary_id;
 +int i;
 +
 +memset(ascii_id, 0, sizeof(ascii_id));
 +
 +/* check if we really have a build-id */
 +if ( NT_GNU_BUILD_ID != n-type )
 +return 0;
 
 This needs to signal an error.

Yes, ENOSYS, (or ENOENT, ENODATA)?

 +
 +/* sanity check, name should be GNU for ld-generated build-id */
 +if ( 0 != strncmp(ELFNOTE_NAME(n), GNU, n-namesz))
 +return 0;
 
 Same here.
 
 +binary_id = (char *)ELFNOTE_DESC(n);
 +
 +/* convert to printable format */
 +for (i = 0; i  n-descsz  (i + 1) * 2  sizeof(xen_build_id_t); 
 i++)
 +{
 +snprintf(ascii_id[i * 2], 3, %02hhx, binary_id[i]);
 +}
 
 No need for the braces, and no need for thehh modifier.
 
 Jan
 

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] xsplice: Use ld-embedded build-ids

[Jan Beulich]

 On 05.08.15 at 16:09, mpohl...@amazon.de wrote:
 Todo:
   * Should be moved to sysctl to only allow Dom0 access

Because of?

   * Maybe convert to binary transport to userland instead of printable form

Indeed.

 @@ -360,11 +366,30 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
 arg)
  
  case XENVER_build_id:
  {
 -xen_build_id_t build_id;
 +xen_build_id_t ascii_id;
 +Elf_Note * n = (Elf_Note *)__note_gnu_build_id_start;
 +char * binary_id;
 +int i;
 +
 +memset(ascii_id, 0, sizeof(ascii_id));
 +
 +/* check if we really have a build-id */
 +if ( NT_GNU_BUILD_ID != n-type )
 +return 0;

This needs to signal an error.

 +
 +/* sanity check, name should be GNU for ld-generated build-id */
 +if ( 0 != strncmp(ELFNOTE_NAME(n), GNU, n-namesz))
 +return 0;

Same here.

 +binary_id = (char *)ELFNOTE_DESC(n);
 +
 +/* convert to printable format */
 +for (i = 0; i  n-descsz  (i + 1) * 2  sizeof(xen_build_id_t); 
 i++)
 +{
 +snprintf(ascii_id[i * 2], 3, %02hhx, binary_id[i]);
 +}

No need for the braces, and no need for thehh modifier.

Jan


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

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

[Martin Pohlack]

Todo:
  * Should be moved to sysctl to only allow Dom0 access
  * Maybe convert to binary transport to userland instead of printable form

* use ld to actually embed the build ID
* convert to textual representation in hypervisor and report in
  printable form

Signed-off-by: Martin Pohlack mpohl...@amazon.de
---
 xen/arch/x86/Makefile|  4 ++--
 xen/arch/x86/xen.lds.S   |  5 +
 xen/common/kernel.c  | 33 +
 xen/common/version.c |  5 -
 xen/include/xen/compile.h.in |  1 -
 5 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 5f24951..f724bd8 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 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]*
 
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 6553cff..2176782 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -67,6 +67,11 @@ SECTIONS
*(.rodata.*)
   } :text
 
+  .note.gnu.build-id : {
+  __note_gnu_build_id_start = .;
+  *(.note.gnu.build-id)
+  } :text
+
   . = ALIGN(SMP_CACHE_BYTES);
   .data.read_mostly : {
/* Exception table */
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index e9d41b6..9814585 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -6,9 +6,11 @@
 
 #include xen/init.h
 #include xen/lib.h
+#include xen/elf.h
 #include xen/errno.h
 #include xen/version.h
 #include xen/sched.h
+#include xen/types.h
 #include xen/paging.h
 #include xen/nmi.h
 #include xen/guest_access.h
@@ -227,6 +229,10 @@ void __init do_initcalls(void)
  * Simple hypercalls.
  */
 
+#define NT_GNU_BUILD_ID 3
+
+extern char * __note_gnu_build_id_start;  /* defined in linker script */
+
 DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
 switch ( cmd )
@@ -360,11 +366,30 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 
 case XENVER_build_id:
 {
-xen_build_id_t build_id;
+xen_build_id_t ascii_id;
+Elf_Note * n = (Elf_Note *)__note_gnu_build_id_start;
+char * binary_id;
+int i;
+
+memset(ascii_id, 0, sizeof(ascii_id));
+
+/* check if we really have a build-id */
+if ( NT_GNU_BUILD_ID != n-type )
+return 0;
+
+/* sanity check, name should be GNU for ld-generated build-id */
+if ( 0 != strncmp(ELFNOTE_NAME(n), GNU, n-namesz))
+return 0;
+
+binary_id = (char *)ELFNOTE_DESC(n);
+
+/* convert to printable format */
+for (i = 0; i  n-descsz  (i + 1) * 2  sizeof(xen_build_id_t); i++)
+{
+snprintf(ascii_id[i * 2], 3, %02hhx, binary_id[i]);
+}
 
-memset(build_id, 0, sizeof(build_id));
-safe_strcpy(build_id, xen_build_id());
-if ( copy_to_guest(arg, build_id, ARRAY_SIZE(build_id)) )
+if ( copy_to_guest(arg, ascii_id, ARRAY_SIZE(ascii_id)) )
 return -EFAULT;
 return 0;
 }
diff --git a/xen/common/version.c b/xen/common/version.c
index 5c3dbb0..b152e27 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -55,8 +55,3 @@ const char *xen_banner(void)
 {
 return XEN_BANNER;
 }
-
-const char *xen_build_id(void)
-{
-return XEN_BUILD_ID;
-}
diff --git a/xen/include/xen/compile.h.in b/xen/include/xen/compile.h.in
index 939685e..440ecb2 100644
--- a/xen/include/xen/compile.h.in
+++ b/xen/include/xen/compile.h.in
@@ -10,5 +10,4 @@
 #define XEN_EXTRAVERSION   @@extraversion@@
 
 #define XEN_CHANGESET  @@changeset@@
-#define XEN_BUILD_ID@@changeset@@
 #define XEN_BANNER \
-- 
2.5.0

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