| Message ID | 1452219920-14043-3-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 08.01.16 at 03:25, <konrad.wilk@oracle.com> wrote: > The mechanism to get this is via the XENVER hypercall and > we add a new sub-command to retrieve the binary build-id > called XENVER_build_id. The sub-hypercall parameter > allows an arbitrary size (the buffer and len is provided > to the hypervisor). A NULL parameter will probe the hypervisor > for the length of the build-id. > > One can also retrieve the value of the build-id by doing > 'readelf -h xen-syms'. Does this or something similar also work on xen.gz and xen.efi? I ask because xen-syms isn't present on the boot partition, and may not be present anywhere on an otherwise functioning system. > --- a/Config.mk > +++ b/Config.mk > @@ -126,6 +126,17 @@ endef > check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1") > $(eval $(check-y)) > > +ld-ver = $(shell if [ $$((`$(1) --version | head -1 | sed 's/[^0-9]/ /g' | awk \ > + '{ printf "0x%02x%02x", $$1, $$2}'`)) -ge $$(($(2))) ]; \ > + then echo y; else echo n; fi ;) > + > +# binutils 2.18 implement build-id. > +ifeq ($(call ld-ver,$(LD),0x0212),n) > +build_id := > +else > +build_id := --build-id=sha1 > +endif Wouldn't it be better to probe the linker for recognizing the --build-id command line option, along the lines of $(cc-option)? In any event the option should be added to LDFLAGS unless this conflicts with something else. > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -67,6 +67,12 @@ SECTIONS > *(.rodata.*) > } :text > > + .note.gnu.build-id : { > + __note_gnu_build_id_start = .; > + *(.note.gnu.build-id) > + __note_gnu_build_id_end = .; > + } :text Wouldn't this better be a generic .note section, with .note.gnu.build-id just special cased ahead of *(.note) *(.note.*)? > --- a/xen/common/kernel.c > +++ b/xen/common/kernel.c > @@ -366,6 +366,41 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > return -EFAULT; > return 0; > } > + case XENVER_build_id: Blank line ahead of case statements please when most/all others have it that way in a particular switch(). > + { > + xen_build_id_t build_id; > + unsigned int sz = 0; > + int rc = 0; > + char *p = NULL; > + > + if ( deny ) > + return -EPERM; > + > + /* Only return size. */ > + if ( !guest_handle_is_null(arg) ) > + { > + if ( copy_from_guest(&build_id, arg, 1) ) > + return -EFAULT; > + > + if ( build_id.len == 0 ) > + return -EINVAL; > + } > + > + rc = xen_build_id(&p, &sz); Perhaps use the helpers from xen/err.h to limit the amount of indirection needed? > + if ( rc ) > + return rc; > + > + if ( guest_handle_is_null(arg) ) > + return sz; > + > + if ( sz > build_id.len ) > + return -ENOBUFS; And how will the caller know how much is needed? > + if ( copy_to_guest_offset(arg, offsetof(xen_build_id_t, buf), p, sz) ) > + return -EFAULT; > + > + return sz; > + } Or how much got copied? > +int xen_build_id(char **p, unsigned int *len) > +{ > + const Elf_Note *n = __note_gnu_build_id_start; > + static bool_t checked = 0; Pointless initializer. Jan
On Tue, Jan 12, 2016 at 09:25:27AM -0700, Jan Beulich wrote: > >>> On 08.01.16 at 03:25, <konrad.wilk@oracle.com> wrote: > > The mechanism to get this is via the XENVER hypercall and > > we add a new sub-command to retrieve the binary build-id > > called XENVER_build_id. The sub-hypercall parameter > > allows an arbitrary size (the buffer and len is provided > > to the hypervisor). A NULL parameter will probe the hypervisor > > for the length of the build-id. > > > > One can also retrieve the value of the build-id by doing > > 'readelf -h xen-syms'. > > Does this or something similar also work on xen.gz and xen.efi? Sadly no. > I ask because xen-syms isn't present on the boot partition, and > may not be present anywhere on an otherwise functioning > system. Right. Some later patches (xsplice related) hook the build-id printing to a keyboard handler. Would that work ? Or are you suggesting that perhaps the kernel should at boot time print the build-id (like it does the changset)? > > > --- a/Config.mk > > +++ b/Config.mk > > @@ -126,6 +126,17 @@ endef > > check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1") > > $(eval $(check-y)) > > > > +ld-ver = $(shell if [ $$((`$(1) --version | head -1 | sed 's/[^0-9]/ /g' | awk \ > > + '{ printf "0x%02x%02x", $$1, $$2}'`)) -ge $$(($(2))) ]; \ > > + then echo y; else echo n; fi ;) > > + > > +# binutils 2.18 implement build-id. > > +ifeq ($(call ld-ver,$(LD),0x0212),n) > > +build_id := > > +else > > +build_id := --build-id=sha1 > > +endif > > Wouldn't it be better to probe the linker for recognizing the --build-id > command line option, along the lines of $(cc-option)? Let me see what happens with that. > > In any event the option should be added to LDFLAGS unless this > conflicts with something else. > > > --- a/xen/arch/x86/xen.lds.S > > +++ b/xen/arch/x86/xen.lds.S > > @@ -67,6 +67,12 @@ SECTIONS > > *(.rodata.*) > > } :text > > > > + .note.gnu.build-id : { > > + __note_gnu_build_id_start = .; > > + *(.note.gnu.build-id) > > + __note_gnu_build_id_end = .; > > + } :text > > Wouldn't this better be a generic .note section, with .note.gnu.build-id > just special cased ahead of *(.note) *(.note.*)? > > > --- a/xen/common/kernel.c > > +++ b/xen/common/kernel.c > > @@ -366,6 +366,41 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > > return -EFAULT; > > return 0; > > } > > + case XENVER_build_id: > > Blank line ahead of case statements please when most/all others > have it that way in a particular switch(). > > > + { > > + xen_build_id_t build_id; > > + unsigned int sz = 0; > > + int rc = 0; > > + char *p = NULL; > > + > > + if ( deny ) > > + return -EPERM; > > + > > + /* Only return size. */ > > + if ( !guest_handle_is_null(arg) ) > > + { > > + if ( copy_from_guest(&build_id, arg, 1) ) > > + return -EFAULT; > > + > > + if ( build_id.len == 0 ) > > + return -EINVAL; > > + } > > + > > + rc = xen_build_id(&p, &sz); > > Perhaps use the helpers from xen/err.h to limit the amount of > indirection needed? > > > + if ( rc ) > > + return rc; > > + > > + if ( guest_handle_is_null(arg) ) > > + return sz; > > + > > + if ( sz > build_id.len ) > > + return -ENOBUFS; > > And how will the caller know how much is needed? Duh. I shall update the build_id.len with the appropiate value. > > > + if ( copy_to_guest_offset(arg, offsetof(xen_build_id_t, buf), p, sz) ) > > + return -EFAULT; > > + > > + return sz; > > + } > > Or how much got copied? That one is easy - we do return 'sz' which tells us how much got copied. Or are you suggesting that the build_id.len should also be updated? > > > +int xen_build_id(char **p, unsigned int *len) > > +{ > > + const Elf_Note *n = __note_gnu_build_id_start; > > + static bool_t checked = 0; > > Pointless initializer. > > Jan >
>>> On 12.01.16 at 17:43, <konrad.wilk@oracle.com> wrote: > On Tue, Jan 12, 2016 at 09:25:27AM -0700, Jan Beulich wrote: >> >>> On 08.01.16 at 03:25, <konrad.wilk@oracle.com> wrote: >> > The mechanism to get this is via the XENVER hypercall and >> > we add a new sub-command to retrieve the binary build-id >> > called XENVER_build_id. The sub-hypercall parameter >> > allows an arbitrary size (the buffer and len is provided >> > to the hypervisor). A NULL parameter will probe the hypervisor >> > for the length of the build-id. >> > >> > One can also retrieve the value of the build-id by doing >> > 'readelf -h xen-syms'. >> >> Does this or something similar also work on xen.gz and xen.efi? > > Sadly no. >> I ask because xen-syms isn't present on the boot partition, and >> may not be present anywhere on an otherwise functioning >> system. > > Right. Some later patches (xsplice related) hook the build-id printing > to a keyboard handler. Would that work ? Key handlers are strictly a debugging facility. > Or are you suggesting that perhaps the kernel should at boot time > print the build-id (like it does the changset)? Perhaps, albeit to me that's a bit orthogonal to being able to find out the build ID for a given binary. >> > + if ( rc ) >> > + return rc; >> > + >> > + if ( guest_handle_is_null(arg) ) >> > + return sz; >> > + >> > + if ( sz > build_id.len ) >> > + return -ENOBUFS; >> >> And how will the caller know how much is needed? > > Duh. I shall update the build_id.len with the appropiate value. Ah, actually I now see you have Andrew's beloved NULL handle check up a few lines - that may suffice. Albeit I'm not generally in favor of this model; I prefer a first attempt to succeed if possible, and a second one only to be needed if the caller estimated size in fact didn't suffice (and then no 3rd one being necessary in order to obtain the needed size). >> > + if ( copy_to_guest_offset(arg, offsetof(xen_build_id_t, buf), p, sz) ) >> > + return -EFAULT; >> > + >> > + return sz; >> > + } >> >> Or how much got copied? > > That one is easy - we do return 'sz' which tells us how much got copied. Ah, right. Perhaps I shouldn't be reviewing patches a few minutes before heading home... Jan
> > Or are you suggesting that perhaps the kernel should at boot time > > print the build-id (like it does the changset)? > > Perhaps, albeit to me that's a bit orthogonal to being able to find out > the build ID for a given binary. I can make some magic objdump + awk to extract the buildid from the hypervisor and deposit as a text file in /usr/../xen/debug? Similar to how xen-syms is copied there? Would that work? But I am a bit lost of what your use-case is? The third patch implements 'xl info' to display it. > > >> > + if ( rc ) > >> > + return rc; > >> > + > >> > + if ( guest_handle_is_null(arg) ) > >> > + return sz; > >> > + > >> > + if ( sz > build_id.len ) > >> > + return -ENOBUFS; > >> > >> And how will the caller know how much is needed? > > > > Duh. I shall update the build_id.len with the appropiate value. > > Ah, actually I now see you have Andrew's beloved NULL handle > check up a few lines - that may suffice. Albeit I'm not generally in > favor of this model; I prefer a first attempt to succeed if possible, > and a second one only to be needed if the caller estimated size in > fact didn't suffice (and then no 3rd one being necessary in order > to obtain the needed size). The code I wrote (libxl) tries with a large buffer (1020 bytes) and if that didn't work just reports the error. I shall change the code to follow your steps.
>>> On 14.01.16 at 03:02, <konrad@kernel.org> wrote: >> > Or are you suggesting that perhaps the kernel should at boot time >> > print the build-id (like it does the changset)? >> >> Perhaps, albeit to me that's a bit orthogonal to being able to find out >> the build ID for a given binary. > > I can make some magic objdump + awk to extract the buildid from > the hypervisor and deposit as a text file in /usr/../xen/debug? > Similar to how xen-syms is copied there? > > Would that work? No, because of ... > But I am a bit lost of what your use-case is? ... this is about holding a standalone Xen binary in hands, and wanting to identify its build ID. Not sure how limited the range of permitted things is in the simplified ELF binary we generate - perhaps adding a proper PT_NOTE segment there would do? Jan
On 14.01.2016 09:58, Jan Beulich wrote: >>>> On 14.01.16 at 03:02, <konrad@kernel.org> wrote: >>>> Or are you suggesting that perhaps the kernel should at boot time >>>> print the build-id (like it does the changset)? >>> >>> Perhaps, albeit to me that's a bit orthogonal to being able to find out >>> the build ID for a given binary. >> >> I can make some magic objdump + awk to extract the buildid from >> the hypervisor and deposit as a text file in /usr/../xen/debug? >> Similar to how xen-syms is copied there? >> >> Would that work? > > No, because of ... > >> But I am a bit lost of what your use-case is? > > ... this is about holding a standalone Xen binary in hands, and > wanting to identify its build ID. Not sure how limited the range of > permitted things is in the simplified ELF binary we generate - > perhaps adding a proper PT_NOTE segment there would do? To clarify your question: readelf -n xen-syms should result in finding and dumping the build ID. The original patch to xen.lds did achieve that. Is that enough for you or do you want that to work as well for the stripped xen file? Martin Amazon Development Center Germany GmbH Berlin - Dresden - Aachen main office: Krausenstr. 38, 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
>>> On 14.01.16 at 10:07, <mpohlack@amazon.com> wrote: > On 14.01.2016 09:58, Jan Beulich wrote: >>>>> On 14.01.16 at 03:02, <konrad@kernel.org> wrote: >>>>> Or are you suggesting that perhaps the kernel should at boot time >>>>> print the build-id (like it does the changset)? >>>> >>>> Perhaps, albeit to me that's a bit orthogonal to being able to find out >>>> the build ID for a given binary. >>> >>> I can make some magic objdump + awk to extract the buildid from >>> the hypervisor and deposit as a text file in /usr/../xen/debug? >>> Similar to how xen-syms is copied there? >>> >>> Would that work? >> >> No, because of ... >> >>> But I am a bit lost of what your use-case is? >> >> ... this is about holding a standalone Xen binary in hands, and >> wanting to identify its build ID. Not sure how limited the range of >> permitted things is in the simplified ELF binary we generate - >> perhaps adding a proper PT_NOTE segment there would do? > > To clarify your question: > > readelf -n xen-syms > > should result in finding and dumping the build ID. The original patch > to xen.lds did achieve that. > > Is that enough for you or do you want that to work as well for the > stripped xen file? "Stripped" isn't the right term, because the uncompressed variant of xen.gz is farther away from xen-syms than just being stripped. But yes, all this is about is that it should be possible to obtain xen.gz's and xen.efi's build IDs as well. Jan
>> --- a/Config.mk >> +++ b/Config.mk >> @@ -126,6 +126,17 @@ endef >> check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1") >> $(eval $(check-y)) >> >> +ld-ver = $(shell if [ $$((`$(1) --version | head -1 | sed 's/[^0-9]/ /g' | awk \ >> + '{ printf "0x%02x%02x", $$1, $$2}'`)) -ge $$(($(2))) ]; \ >> + then echo y; else echo n; fi ;) >> + >> +# binutils 2.18 implement build-id. >> +ifeq ($(call ld-ver,$(LD),0x0212),n) >> +build_id := >> +else >> +build_id := --build-id=sha1 >> +endif > > Wouldn't it be better to probe the linker for recognizing the --build-id > command line option, along the lines of $(cc-option)? +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ + grep -q unrecognized && echo n || echo y) -ish ? > > In any event the option should be added to LDFLAGS unless this > conflicts with something else. That had some interesting side-effect: $ find . -name *.o | xargs readelf -n | more File: ./arch/x86/built_in.o Displaying notes found at file offset 0x00000040 with length 0x00000240: Owner Data size Description GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: a114d1fdec2ace38448f141013f5a659122f2390 GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: 4a913d3d1ece4d175fc0df0b36745b801f88bfab GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: ead89ba3aa9a8257cfc863966b7a9a725ecce133 GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: 9b034a93573015c611d0900e949370d9f776bac1 GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: 0e5fab6126ce69edd5720b96e2d5414618259c78 GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: 49eca2138553b5e85ee45bb47c52aca394c31c31 GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: f1cc2c8ae09fefe1440662efc44208a0b9ff8ddd GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: 29991f03f7a1aeeb59c8f83c0e7a349384a7262c GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: 3e2c4df9f60fdbd970bd1a35d03c7b68fd794385 GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: 2c4f5cd9bcf553a022dace08b7741d85a4eb657f GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: 41c502289e5b28722ab5df6ae5bd7b99fb90c09e GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: f268afebdf211de6bb6d12e513520bba969130cc GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: b73b7296f6165d3dcacee36691d92ab1996d9908 GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: ff8e080d8e01966cf8d893ce6cd258dd0d1c6124 GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: dcc8964716f74bb710911d80faaae016820f72d9 GNU 0x00000014 NT_GNU_BUILD_ID (unique build ID bitstring) Build ID: 649f4bb66df2fead426edda62d0c90ab088c5fd4 And during build: ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored. ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored. ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored. With: [konrad@char xen]$ readelf -n xen-syms | grep Build | wc 38 114 2090 I think we should skip on the LDFLAGS idea.
>>> On 15.01.16 at 22:49, <konrad@kernel.org> wrote: >> > --- a/Config.mk >>> +++ b/Config.mk >>> @@ -126,6 +126,17 @@ endef >>> check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least > gcc-4.1") >>> $(eval $(check-y)) >>> >>> +ld-ver = $(shell if [ $$((`$(1) --version | head -1 | sed 's/[^0-9]/ /g' | > awk \ >>> + '{ printf "0x%02x%02x", $$1, $$2}'`)) -ge $$(($(2))) ]; \ >>> + then echo y; else echo n; fi ;) >>> + >>> +# binutils 2.18 implement build-id. >>> +ifeq ($(call ld-ver,$(LD),0x0212),n) >>> +build_id := >>> +else >>> +build_id := --build-id=sha1 >>> +endif >> >> Wouldn't it be better to probe the linker for recognizing the --build-id >> command line option, along the lines of $(cc-option)? > > +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ > + grep -q unrecognized && echo n > || echo y) > > -ish ? If that fulfills the purpose - why not? >> In any event the option should be added to LDFLAGS unless this >> conflicts with something else. > > That had some interesting side-effect: > > $ find . -name *.o | xargs readelf -n | more > File: ./arch/x86/built_in.o > > Displaying notes found at file offset 0x00000040 with length 0x00000240: > Owner Data size Description > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: a114d1fdec2ace38448f141013f5a659122f2390 > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: 4a913d3d1ece4d175fc0df0b36745b801f88bfab > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: ead89ba3aa9a8257cfc863966b7a9a725ecce133 > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: 9b034a93573015c611d0900e949370d9f776bac1 > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: 0e5fab6126ce69edd5720b96e2d5414618259c78 > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: 49eca2138553b5e85ee45bb47c52aca394c31c31 > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: f1cc2c8ae09fefe1440662efc44208a0b9ff8ddd > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: 29991f03f7a1aeeb59c8f83c0e7a349384a7262c > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: 3e2c4df9f60fdbd970bd1a35d03c7b68fd794385 > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: 2c4f5cd9bcf553a022dace08b7741d85a4eb657f > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: 41c502289e5b28722ab5df6ae5bd7b99fb90c09e > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: f268afebdf211de6bb6d12e513520bba969130cc > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: b73b7296f6165d3dcacee36691d92ab1996d9908 > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: ff8e080d8e01966cf8d893ce6cd258dd0d1c6124 > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: dcc8964716f74bb710911d80faaae016820f72d9 > GNU 0x00000014 NT_GNU_BUILD_ID (unique build > ID bitstring) > Build ID: 649f4bb66df2fead426edda62d0c90ab088c5fd4 > > > And during build: > ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored. > ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored. > ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored. > > With: > [konrad@char xen]$ readelf -n xen-syms | grep Build | wc > 38 114 2090 > > I think we should skip on the LDFLAGS idea. Isn't that merely a result of LDFLAGS being used for both the linking of the various built_in.o and the final binaries? A fully shared set of flags for these two pretty distinct operations doesn't seem very sensible anyway. In fact I wonder how much of LDFLAGS is actually relevant when passing -r to ld. Jan
>>> Wouldn't it be better to probe the linker for recognizing the --build-id >>> command line option, along the lines of $(cc-option)? >> >> +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ >> + grep -q unrecognized && echo n >> || echo y) >> >> -ish ? > > If that fulfills the purpose - why not? Great. Will use that (after checking it on on SLES10). > >>> In any event the option should be added to LDFLAGS unless this >>> conflicts with something else. >> >> That had some interesting side-effect: >> >> $ find . -name *.o | xargs readelf -n | more >> File: ./arch/x86/built_in.o >> >> Displaying notes found at file offset 0x00000040 with length 0x00000240: >> Owner Data size Description >> GNU 0x00000014 NT_GNU_BUILD_ID (unique build >> ID bitstring) >> Build ID: a114d1fdec2ace38448f141013f5a659122f2390 >> GNU 0x00000014 NT_GNU_BUILD_ID (unique build >> ID bitstring) .. snip.. >> And during build: >> ld: warning: Cannot create .note.gnu.build-id section, --build-id ignored. .. snip.. >> I think we should skip on the LDFLAGS idea. > > Isn't that merely a result of LDFLAGS being used for both the > linking of the various built_in.o and the final binaries? A fully True. > shared set of flags for these two pretty distinct operations > doesn't seem very sensible anyway. In fact I wonder how > much of LDFLAGS is actually relevant when passing -r to ld. > That would imply that the idea of putting --build-id should be part only of the final binaries. Which comes back to how this patch was done originally - only have the --build-id on specific $(LD) lines. Are you OK if I re-institute the $(build_id) back in the Makefile, perhaps rename it to $(build_id_linker) to make it more clear? Thanks.
>>> On 19.01.16 at 17:05, <konrad@kernel.org> wrote: > Are you OK if I re-institute the $(build_id) back in the Makefile, perhaps > rename it to $(build_id_linker) to make it more clear? Well, perhaps that's the least troublesome route right now, even if I don't really like it. Jan
On Tue, Jan 19, 2016 at 09:36:27AM -0700, Jan Beulich wrote: > >>> On 19.01.16 at 17:05, <konrad@kernel.org> wrote: > > Are you OK if I re-institute the $(build_id) back in the Makefile, perhaps > > rename it to $(build_id_linker) to make it more clear? > > Well, perhaps that's the least troublesome route right now, even > if I don't really like it. :-) Thank you for being OK with this. I can poke at the LD_FLAGS and the prolifieration of various flags in parallel. Perhaps after the cleanup of the C-and-P states and now the T-states being trapped? > > Jan >
> > > Or are you suggesting that perhaps the kernel should at boot time > > print the build-id (like it does the changset)? > > Perhaps, albeit to me that's a bit orthogonal to being able to find out > the build ID for a given binary. I looked in the mkelf32 and it looks quite easy to make the resulting xen.gz have the .note section. But I am at lost for the EFI file. Could you suggest some ideas of what type of PE/COFF section it should be put in? Or good specs to consult? Thank you!
>>> On 25.01.16 at 16:16, <konrad.wilk@oracle.com> wrote: >> >> > Or are you suggesting that perhaps the kernel should at boot time >> > print the build-id (like it does the changset)? >> >> Perhaps, albeit to me that's a bit orthogonal to being able to find out >> the build ID for a given binary. > > > I looked in the mkelf32 and it looks quite easy to make the resulting > xen.gz have the .note section. > > But I am at lost for the EFI file. Could you suggest some ideas of what > type of PE/COFF section it should be put in? Or good specs to consult? Well, for xen.efi I really don't have any good idea either. Perhaps we simply need to declare this as going to be deal with by the intended re-unification of both binaries which Daniel has been planning? Jan
Konrad Rzeszutek Wilk writes ("[PATCH v3 2/3] XENVER_build_id: Provide ld-embedded build-ids (v8)"):
> The mechanism to get this is via the XENVER hypercall and
> we add a new sub-command to retrieve the binary build-id
> called XENVER_build_id. The sub-hypercall parameter
> allows an arbitrary size (the buffer and len is provided
> to the hypervisor). A NULL parameter will probe the hypervisor
> for the length of the build-id.
>
> One can also retrieve the value of the build-id by doing
> 'readelf -h xen-syms'.
>
> For EFI builds we re-use the same build-id that the xen-syms
> was built with.
>
> Note that there are no changes to the XSM files (dummy.c
> and hooks.c) as the privileged subops fall in the default case.
>
> The version of ld that first implemented --build-id is v2.18.
> Hence we check for that or later version - if older version
> found we do not build the hypervisor with the build-id
> (and the return code is -ENODATA for that case).
I think this commit message is missing some important explanation for
poor ignorant souls like me.
Something like:
It is possible to have ld record an arbitrary identifier in an ELF,
which can then be read out with `readelf'. We also want to provide
a way for guests to retrieve the same identifier.
[ xxx do we? Maybe hosting providers would want to hide which of
their hosts had been updated ]
We construct the build-id out of [ ???? ]
There is ???? no impact on users who want reproducible builds [or]
??? users who want reproducible builds are expected to use the
facilities documented in ??? comments in Config.mk [or something]
Ian.
diff --git a/Config.mk b/Config.mk index 62f8209..80d6972 100644 --- a/Config.mk +++ b/Config.mk @@ -126,6 +126,17 @@ endef check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1") $(eval $(check-y)) +ld-ver = $(shell if [ $$((`$(1) --version | head -1 | sed 's/[^0-9]/ /g' | awk \ + '{ printf "0x%02x%02x", $$1, $$2}'`)) -ge $$(($(2))) ]; \ + then echo y; else echo n; fi ;) + +# binutils 2.18 implement build-id. +ifeq ($(call ld-ver,$(LD),0x0212),n) +build_id := +else +build_id := --build-id=sha1 +endif + # as-insn: Check whether assembler supports an instruction. # Usage: cflags-y += $(call as-insn "insn",option-yes,option-no) as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \ diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c index 6c0c0d6..876e0df 100644 --- a/tools/libxc/xc_private.c +++ b/tools/libxc/xc_private.c @@ -712,6 +712,13 @@ int xc_version(xc_interface *xch, int cmd, void *arg) case XENVER_commandline: sz = sizeof(xen_commandline_t); break; + case XENVER_build_id: + { + xen_build_id_t *build_id = (xen_build_id_t *)arg; + sz = sizeof(*build_id) + build_id->len; + HYPERCALL_BOUNCE_SET_DIR(arg, XC_HYPERCALL_BUFFER_BOUNCE_BOTH); + break; + } default: ERROR("xc_version: unknown command %d\n", cmd); return -EINVAL; diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h index f603c15..b1b1432 100644 --- a/tools/libxc/xc_private.h +++ b/tools/libxc/xc_private.h @@ -202,6 +202,16 @@ enum { #define DECLARE_HYPERCALL_BOUNCE(_ubuf, _sz, _dir) DECLARE_NAMED_HYPERCALL_BOUNCE(_ubuf, _ubuf, _sz, _dir) /* + * Change the direction. + * + * Can only be used if the bounce_pre/bounce_post commands have + * not been used. + */ +#define HYPERCALL_BOUNCE_SET_DIR(_buf, _dir) do { if ((HYPERCALL_BUFFER(_buf))->hbuf) \ + assert(1); \ + (HYPERCALL_BUFFER(_buf))->dir = _dir; \ + } while (0) +/* * Set the size of data to bounce. Useful when the size is not known * when the bounce buffer is declared. */ diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 2f050f5..fdf0800 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -79,17 +79,17 @@ $(BASEDIR)/common/symbols-dummy.o: $(MAKE) -f $(BASEDIR)/Rules.mk -C $(BASEDIR)/common symbols-dummy.o $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o - $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ + $(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id) \ $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ | $(BASEDIR)/tools/symbols --sysv --sort >$(@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) \ $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ | $(BASEDIR)/tools/symbols --sysv --sort >$(@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/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index 0488f37..1d262c4 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -53,6 +53,12 @@ SECTIONS _erodata = .; /* End of read-only data */ } :text + .note.gnu.build-id : { + __note_gnu_build_id_start = .; + *(.note.gnu.build-id) + __note_gnu_build_id_end = .; + } :text + .data : { /* Data */ . = ALIGN(PAGE_SIZE); *(.data.page_aligned) diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 8e6e901..b88a84f 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -103,20 +103,23 @@ $(BASEDIR)/common/symbols-dummy.o: $(MAKE) -f $(BASEDIR)/Rules.mk -C $(BASEDIR)/common symbols-dummy.o $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o - $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ + $(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id) \ $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ | $(BASEDIR)/tools/symbols --sysv --sort >$(@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) \ $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ | $(BASEDIR)/tools/symbols --sysv --sort --warn-dup >$(@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]* +build_id.o: $(TARGET)-syms + $(OBJCOPY) --only-section=.note.gnu.build-id $< build_id.o + EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(LDFLAGS)) --subsystem=10 EFI_LDFLAGS += --image-base=$(1) --stack=0,0 --heap=0,0 --strip-debug EFI_LDFLAGS += --section-alignment=0x200000 --file-alignment=0x20 @@ -129,6 +132,9 @@ $(TARGET).efi: VIRT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A VIR $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A ALT_START$$,,p') # Don't use $(wildcard ...) here - at least make 3.80 expands this too early! $(TARGET).efi: guard = $(if $(shell echo efi/dis* | grep disabled),:) +ifneq ($(build_id),) +$(TARGET).efi: build_id.o +endif $(TARGET).efi: prelink-efi.o efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbols-dummy.o efi/mkreloc $(foreach base, $(VIRT_BASE) $(ALT_BASE), \ $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< efi/relocs-dummy.o \ @@ -144,8 +150,8 @@ $(TARGET).efi: prelink-efi.o efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbol $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \ | $(guard) $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).1s.S $(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o - $(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \ - $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o -o $@ + $(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds $(build_id) -N $< \ + $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o build_id.o -o $@ if $(guard) false; then rm -f $@; echo 'EFI support disabled'; fi rm -f $(@D)/.$(@F).[0-9]* diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index e18e08f..329eab6 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -67,6 +67,12 @@ SECTIONS *(.rodata.*) } :text + .note.gnu.build-id : { + __note_gnu_build_id_start = .; + *(.note.gnu.build-id) + __note_gnu_build_id_end = .; + } :text + . = ALIGN(SMP_CACHE_BYTES); .data.read_mostly : { /* Exception table */ diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 2b3ccc4..62a95a5 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -366,6 +366,41 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) return -EFAULT; return 0; } + case XENVER_build_id: + { + xen_build_id_t build_id; + unsigned int sz = 0; + int rc = 0; + char *p = NULL; + + if ( deny ) + return -EPERM; + + /* Only return size. */ + if ( !guest_handle_is_null(arg) ) + { + if ( copy_from_guest(&build_id, arg, 1) ) + return -EFAULT; + + if ( build_id.len == 0 ) + return -EINVAL; + } + + rc = xen_build_id(&p, &sz); + if ( rc ) + return rc; + + if ( guest_handle_is_null(arg) ) + return sz; + + if ( sz > build_id.len ) + return -ENOBUFS; + + if ( copy_to_guest_offset(arg, offsetof(xen_build_id_t, buf), p, sz) ) + return -EFAULT; + + return sz; + } } return -ENOSYS; diff --git a/xen/common/version.c b/xen/common/version.c index 95332a0..f7e120f 100644 --- a/xen/common/version.c +++ b/xen/common/version.c @@ -1,5 +1,9 @@ #include <xen/compile.h> #include <xen/version.h> +#include <xen/types.h> +#include <xen/string.h> +#include <xen/elf.h> +#include <xen/errno.h> const char *xen_compile_date(void) { @@ -60,3 +64,41 @@ const char *xen_deny(void) { return "<denied>\0"; } + +#define NT_GNU_BUILD_ID 3 +/* Defined in linker script. */ +extern const Elf_Note __note_gnu_build_id_start[], __note_gnu_build_id_end[]; + +int xen_build_id(char **p, unsigned int *len) +{ + const Elf_Note *n = __note_gnu_build_id_start; + static bool_t checked = 0; + + if ( checked ) + { + *len = n->descsz; + *p = ELFNOTE_DESC(n); + return 0; + } + /* --build-id invoked with wrong parameters. */ + if ( __note_gnu_build_id_end <= __note_gnu_build_id_start ) + return -ENODATA; + + /* Check for full Note header. */ + if ( &n[1] > __note_gnu_build_id_end ) + return -ENODATA; + + /* Check if we really have a build-id. */ + if ( NT_GNU_BUILD_ID != n->type ) + return -ENODATA; + + /* Sanity check, name should be "GNU" for ld-generated build-id. */ + if ( strncmp(ELFNOTE_NAME(n), "GNU", n->namesz) != 0 ) + return -ENODATA; + + *len = n->descsz; + *p = ELFNOTE_DESC(n); + + checked = 1; + return 0; +} diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h index 1520b41..b863c80 100644 --- a/xen/include/asm-arm/config.h +++ b/xen/include/asm-arm/config.h @@ -15,8 +15,10 @@ #if defined(CONFIG_ARM_64) # define LONG_BYTEORDER 3 +# define ELFSIZE 64 #else # define LONG_BYTEORDER 2 +# define ELFSIZE 32 #endif #define BYTES_PER_LONG (1 << LONG_BYTEORDER) diff --git a/xen/include/public/version.h b/xen/include/public/version.h index 44f26b0..adca602 100644 --- a/xen/include/public/version.h +++ b/xen/include/public/version.h @@ -30,7 +30,8 @@ #include "xen.h" -/* NB. All ops return zero on success, except XENVER_{version,pagesize} */ +/* NB. All ops return zero on success, except + * XENVER_{version,pagesize,build_id} */ /* arg == NULL; returns major:minor (16:16). */ #define XENVER_version 0 @@ -83,6 +84,19 @@ typedef struct xen_feature_info xen_feature_info_t; #define XENVER_commandline 9 typedef char xen_commandline_t[1024]; +/* Return value is the number of bytes written, or XEN_Exx on error. + * Calling with empty parameter returns the size of build_id. */ +#define XENVER_build_id 10 +struct xen_build_id { + uint32_t len; /* IN: size of buf[]. */ +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L + unsigned char buf[]; +#elif defined(__GNUC__) + unsigned char buf[1]; /* OUT: Variable length buffer with build_id. */ +#endif +}; +typedef struct xen_build_id xen_build_id_t; + #endif /* __XEN_PUBLIC_VERSION_H__ */ /* diff --git a/xen/include/xen/version.h b/xen/include/xen/version.h index 2015c0b..466c977 100644 --- a/xen/include/xen/version.h +++ b/xen/include/xen/version.h @@ -13,5 +13,6 @@ const char *xen_extra_version(void); const char *xen_changeset(void); const char *xen_banner(void); const char *xen_deny(void); +int xen_build_id(char **p, unsigned int *len); #endif /* __XEN_VERSION_H__ */ diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors index 44a106e..c123256 100644 --- a/xen/xsm/flask/policy/access_vectors +++ b/xen/xsm/flask/policy/access_vectors @@ -93,8 +93,8 @@ class xen2 pmu_ctrl # PMU use (domains, including unprivileged ones, will be using this operation) pmu_use -# XENVER_commandline usage. - version_priv +# XENVER_[commandline|build_id] usage. + version_priv } # Classes domain and domain2 consist of operations that a domain performs on