|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC PATCH v3.1 2/2] xsplice: Add hook for build_id
On 05.08.2015 10:58, Andrew Cooper wrote:
> On 05/08/15 09:50, Martin Pohlack wrote:
>> On 27.07.2015 21:20, Konrad Rzeszutek Wilk wrote:
>>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>>> ---
>>> tools/libxc/xc_private.c | 3 +++
>>> tools/misc/xen-xsplice.c | 25 +++++++++++++++++++++++++
>>> xen/common/kernel.c | 11 +++++++++++
>>> xen/common/version.c | 5 +++++
>>> xen/include/public/version.h | 4 ++++
>>> xen/include/xen/compile.h.in | 1 +
>>> xen/include/xen/version.h | 1 +
>>> 7 files changed, 50 insertions(+)
>>>
>>> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
>>> index 2ffebd9..7c039ca 100644
>>> --- a/tools/libxc/xc_private.c
>>> +++ b/tools/libxc/xc_private.c
>>> @@ -713,6 +713,9 @@ int xc_version(xc_interface *xch, int cmd, void *arg)
>>> case XENVER_commandline:
>>> sz = sizeof(xen_commandline_t);
>>> break;
>>> + case XENVER_build_id:
>>> + sz = sizeof(xen_build_id_t);
>>> + break;
>>> default:
>>> ERROR("xc_version: unknown command %d\n", cmd);
>>> return -EINVAL;
>>> diff --git a/tools/misc/xen-xsplice.c b/tools/misc/xen-xsplice.c
>>> index 7cf9879..dd8266c 100644
>>> --- a/tools/misc/xen-xsplice.c
>>> +++ b/tools/misc/xen-xsplice.c
>>> @@ -17,6 +17,7 @@ void show_help(void)
>>> " <id> An unique name of payload. Up to 40 characters.\n"
>>> "Commands:\n"
>>> " help display this help\n"
>>> + " build-id display build-id of hypervisor.\n"
>>> " upload <id> <file> upload file <cpuid> with <id> name\n"
>>> " list list payloads uploaded.\n"
>>> " apply <id> apply <id> patch.\n"
>>> @@ -306,12 +307,36 @@ int action_func(int argc, char *argv[], unsigned int
>>> idx)
>>>
>>> return rc;
>>> }
>>> +
>>> +static int build_id_func(int argc, char *argv[])
>>> +{
>>> + xen_build_id_t build_id;
>>> +
>>> + if ( argc )
>>> + {
>>> + show_help();
>>> + return -1;
>>> + }
>>> +
>>> + memset(build_id, 0, sizeof(*build_id));
>>> +
>>> + if ( xc_version(xch, XENVER_build_id, &build_id) < 0 )
>>> + {
>>> + printf("Failed to get build_id: %d(%s)\n", errno, strerror(errno));
>>> + return -1;
>>> + }
>>> +
>>> + printf("%s\n", build_id);
>>> + return 0;
>>> +}
>>> +
>>> 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 },
>>> };
>>>
>>> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
>>> index 6a3196a..e9d41b6 100644
>>> --- a/xen/common/kernel.c
>>> +++ b/xen/common/kernel.c
>>> @@ -357,6 +357,17 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void)
>>> arg)
>>> if ( copy_to_guest(arg, saved_cmdline, ARRAY_SIZE(saved_cmdline)) )
>>> return -EFAULT;
>>> return 0;
>>> +
>>> + case XENVER_build_id:
>>> + {
>>> + xen_build_id_t build_id;
>>> +
>>> + memset(build_id, 0, sizeof(build_id));
>>> + safe_strcpy(build_id, xen_build_id());
>> You seem to want to store and transfer the build_id as a string. Any
>> reason why we don't directly expose the build_id embedded by the linker
>> in binary format?
>>
>>> + if ( copy_to_guest(arg, build_id, ARRAY_SIZE(build_id)) )
>>> + return -EFAULT;
>>> + return 0;
>>> + }
>> 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.
>
> It will need its own XSM hook, but need not be strictly limited to just
> dom0.
>
>>
>>> }
>>>
>>> return -ENOSYS;
>>> diff --git a/xen/common/version.c b/xen/common/version.c
>>> index b152e27..5c3dbb0 100644
>>> --- a/xen/common/version.c
>>> +++ b/xen/common/version.c
>>> @@ -55,3 +55,8 @@ const char *xen_banner(void)
>>> {
>>> return XEN_BANNER;
>>> }
>>> +
>>> +const char *xen_build_id(void)
>>> +{
>>> + return XEN_BUILD_ID;
>>> +}
>>> diff --git a/xen/include/public/version.h b/xen/include/public/version.h
>>> index 44f26b0..c863393 100644
>>> --- a/xen/include/public/version.h
>>> +++ b/xen/include/public/version.h
>>> @@ -83,6 +83,10 @@ typedef struct xen_feature_info xen_feature_info_t;
>>> #define XENVER_commandline 9
>>> typedef char xen_commandline_t[1024];
>>>
>>> +#define XENVER_build_id 10
>>> +typedef char xen_build_id_t[1024];
>>> +#define XEN_BUILD_ID_LEN (sizeof(xen_build_id_t))
>>> +
>>> #endif /* __XEN_PUBLIC_VERSION_H__ */
>>>
>>> /*
>>> diff --git a/xen/include/xen/compile.h.in b/xen/include/xen/compile.h.in
>>> index 440ecb2..939685e 100644
>>> --- a/xen/include/xen/compile.h.in
>>> +++ b/xen/include/xen/compile.h.in
>>> @@ -10,4 +10,5 @@
>>> #define XEN_EXTRAVERSION "@@extraversion@@"
>>>
>>> #define XEN_CHANGESET "@@changeset@@"
>>> +#define XEN_BUILD_ID "@@changeset@@"
>> That leads to a chicken and egg problem when embedding a real build_id.
>> Some linker script magic seems to be required. I will try to refine
>> the patch.
>
> So funnily enough, I tried experimenting with this and it is fairly easy
> to get the basics done.
>
> Further TODO which I havn't done yet is make the --build-id optional on
> finding a compatible `ld`, and some symbol magic to directly locate
> .note.gnu.build-id
>
> However, this in addition to some of Konrad's original patch is a good
> start.
>
> ~Andrew
>
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index 5f24951..10938b2 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -112,7 +112,7 @@ $(TARGET)-syms: prelink.o xen.lds
> $(BASEDIR)/common/symbols-dummy.o
> $(@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 \
> $(@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..46e6546 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -68,6 +68,13 @@ SECTIONS
> } :text
>
> . = ALIGN(SMP_CACHE_BYTES);
> + .notes : {
> + __start_notes = .;
> + *(.note.*)
> + __end_notes = .;
> + } :text
> +
> + . = ALIGN(SMP_CACHE_BYTES);
> .data.read_mostly : {
> /* Exception table */
> __start___ex_table = .;
And here is my version, also on-top of Konrad's series:
----------------------------------------------------------------------
[PATCH] xsplice: Use ld-embedded build-ids
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 <mpohlack@xxxxxxxxx>
---
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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |