LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCHv3 0/2] Salted build ids via linker sections
@ 2018-05-23  0:19 Laura Abbott
  2018-05-23  0:19 ` [PATCHv3 1/2] kbuild: Introduce build-salt linker script Laura Abbott
  2018-05-23  0:19 ` [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO Laura Abbott
  0 siblings, 2 replies; 13+ messages in thread
From: Laura Abbott @ 2018-05-23  0:19 UTC (permalink / raw)
  To: Andy Lutomirski, mjw, H . J . Lu, Masahiro Yamada
  Cc: Laura Abbott, Linus Torvalds, X86 ML, linux-kernel, Nick Clifton,
	Cary Coutant, linux-kbuild

Hi,

This is v3 of the series to allow unique build-ids in the kernel. As
a reminder of the context:

""
In Fedora, the debug information is packaged separately (foo-debuginfo) and
can be installed separately. There's been a long standing issue where only one
version of a debuginfo info package can be installed at a time. Mark Wielaard
made an effort for Fedora 27 to allow parallel installation of debuginfo (see
https://fedoraproject.org/wiki/Changes/ParallelInstallableDebuginfo for
more details)

Part of the requirement to allow this to work is that build ids are
unique between builds. The existing upstream rpm implementation ensures
this by re-calculating the build-id using the version and release as a
seed. This doesn't work 100% for the kernel because of the vDSO which is
its own binary and doesn't get updated. After poking holes in a few of my
ideas, there was a discussion with some people from the binutils team about
adding --build-id-salt to let ld do the calculation debugedit is doing. There
was a counter proposal made about adding some extra information via a .comment
which will affect the build id calculation but just get stripped out.
""

I mentioned in the previous version that there were problems with
sporadic build failures. v3 switches to generating the linker script
directly instead of a header + a linker script to avoid this problem. I
also dropped the Kconfig completely (I can add it back as a guard if
this seems not small enough to want all the time). I've also dropped the
RFC tag since it's well formed enough at this point.

Laura Abbott (2):
  kbuild: Introduce build-salt linker script
  x86/vdso: Add build salt to the vDSO

 Makefile                     |  4 +++-
 arch/x86/entry/vdso/Makefile |  4 +++-
 scripts/.gitignore           |  1 +
 scripts/Makefile             |  9 ++++++++-
 scripts/gensalt              | 22 ++++++++++++++++++++++
 scripts/link-vmlinux.sh      |  3 ++-
 6 files changed, 39 insertions(+), 4 deletions(-)
 create mode 100755 scripts/gensalt

-- 
2.17.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCHv3 1/2] kbuild: Introduce build-salt linker script
  2018-05-23  0:19 [PATCHv3 0/2] Salted build ids via linker sections Laura Abbott
@ 2018-05-23  0:19 ` Laura Abbott
  2018-05-23  0:19 ` [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO Laura Abbott
  1 sibling, 0 replies; 13+ messages in thread
From: Laura Abbott @ 2018-05-23  0:19 UTC (permalink / raw)
  To: Andy Lutomirski, mjw, H . J . Lu, Masahiro Yamada
  Cc: Laura Abbott, Linus Torvalds, X86 ML, linux-kernel, Nick Clifton,
	Cary Coutant, linux-kbuild


The build id generated from --build-id can be generated in several different
ways, with the default being the sha1 on the output of the linked file. For
distributions, it can be useful to make sure this ID is unique, even if the
actual file contents don't change. The easiest way to do this is to insert
a comment section with some data.

Introduce a generated linker script to link against the kernel/modules.
This puts the kernel version in a .comment section which will generate a
unique build id if the kernel version changes.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v3: Generate the linker script directly instead of just a header.
---
 Makefile                |  4 +++-
 scripts/.gitignore      |  1 +
 scripts/Makefile        |  9 ++++++++-
 scripts/gensalt         | 22 ++++++++++++++++++++++
 scripts/link-vmlinux.sh |  3 ++-
 5 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100755 scripts/gensalt

diff --git a/Makefile b/Makefile
index ec6f45928fd4..87fe8992afd8 100644
--- a/Makefile
+++ b/Makefile
@@ -428,7 +428,8 @@ KBUILD_AFLAGS_KERNEL :=
 KBUILD_CFLAGS_KERNEL :=
 KBUILD_AFLAGS_MODULE  := -DMODULE
 KBUILD_CFLAGS_MODULE  := -DMODULE
-KBUILD_LDFLAGS_MODULE := -T $(srctree)/scripts/module-common.lds
+KBUILD_LDFLAGS_MODULE := -T $(srctree)/scripts/module-common.lds \
+			 -T $(obj)/scripts/build-salt.lds
 LDFLAGS :=
 GCC_PLUGINS_CFLAGS :=
 
@@ -997,6 +998,7 @@ export KBUILD_VMLINUX_INIT := $(head-y) $(init-y)
 export KBUILD_VMLINUX_MAIN := $(core-y) $(libs-y2) $(drivers-y) $(net-y) $(virt-y)
 export KBUILD_VMLINUX_LIBS := $(libs-y1)
 export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux.lds
+export EXTRA_LDS           := scripts/build-salt.lds
 export LDFLAGS_vmlinux
 # used by scripts/package/Makefile
 export KBUILD_ALLDIRS := $(sort $(filter-out arch/%,$(vmlinux-alldirs)) arch Documentation include samples scripts tools)
diff --git a/scripts/.gitignore b/scripts/.gitignore
index 0442c06eefcb..1c840ef4f0c8 100644
--- a/scripts/.gitignore
+++ b/scripts/.gitignore
@@ -13,3 +13,4 @@ asn1_compiler
 extract-cert
 sign-file
 insert-sys-cert
+build-salt.lds
diff --git a/scripts/Makefile b/scripts/Makefile
index 25ab143cbe14..019b0749ff46 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -25,7 +25,14 @@ HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
 HOSTLOADLIBES_sign-file = -lcrypto
 HOSTLOADLIBES_extract-cert = -lcrypto
 
-always		:= $(hostprogs-y) $(hostprogs-m)
+always		:= $(hostprogs-y) $(hostprogs-m) build-salt.lds
+
+define filechk_build-salt.lds
+	($(CONFIG_SHELL) $(srctree)/scripts/gensalt $(KERNELRELEASE))
+endef
+
+$(obj)/build-salt.lds: $(src)/gensalt FORCE
+	$(call filechk,build-salt.lds)
 
 # The following hostprogs-y programs are only build on demand
 hostprogs-y += unifdef
diff --git a/scripts/gensalt b/scripts/gensalt
new file mode 100755
index 000000000000..846c0407cc43
--- /dev/null
+++ b/scripts/gensalt
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+if [[ $1 = "" ]]; then
+	echo "#define BUILD_ID_SALT"
+	exit 0
+fi
+
+BUILD_ID_SALT=$1
+
+echo "SECTIONS {"
+echo ".comment (INFO) :"
+echo " {"
+
+_TAG=`echo $BUILD_ID_SALT | sed -e 's/\(.\)/\1 /g'`
+for c in $_TAG; do
+	_HEX=`echo -n $c | od -A n -t x1 | tr -d ' ' `
+	echo "BYTE(0x$_HEX);"
+done
+echo "BYTE(0x00);"
+
+echo " } "
+echo " } "
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 9045823c7be7..588946dde658 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -84,6 +84,7 @@ modpost_link()
 vmlinux_link()
 {
 	local lds="${objtree}/${KBUILD_LDS}"
+	local extra_lds="${objtree}/${EXTRA_LDS}"
 	local objects
 
 	if [ "${SRCARCH}" != "um" ]; then
@@ -96,7 +97,7 @@ vmlinux_link()
 			${1}"
 
 		${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2}	\
-			-T ${lds} ${objects}
+			-T ${lds} -T ${extra_lds} ${objects}
 	else
 		objects="-Wl,--whole-archive			\
 			built-in.a				\
-- 
2.17.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-23  0:19 [PATCHv3 0/2] Salted build ids via linker sections Laura Abbott
  2018-05-23  0:19 ` [PATCHv3 1/2] kbuild: Introduce build-salt linker script Laura Abbott
@ 2018-05-23  0:19 ` Laura Abbott
  2018-05-23  0:33   ` Andy Lutomirski
  2018-05-24  1:55   ` Masahiro Yamada
  1 sibling, 2 replies; 13+ messages in thread
From: Laura Abbott @ 2018-05-23  0:19 UTC (permalink / raw)
  To: Andy Lutomirski, mjw, H . J . Lu, Masahiro Yamada
  Cc: Laura Abbott, Linus Torvalds, X86 ML, linux-kernel, Nick Clifton,
	Cary Coutant, linux-kbuild


The vDSO is linked separately from the kernel and modules. Ensure it picks
up the comment section, if available.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v3: Invoke the generated linker script. The ".." nightmare is pretty
ugly but I didn't see an easier way to pick up the generated file.
That was actually part of my motivation for using an #include since
paths for those are standardized.
---
 arch/x86/entry/vdso/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index d998a487c9b1..f54aa97dc9f0 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -162,7 +162,9 @@ $(obj)/vdso32.so.dbg: FORCE \
 quiet_cmd_vdso = VDSO    $@
       cmd_vdso = $(CC) -nostdlib -o $@ \
 		       $(VDSO_LDFLAGS) $(VDSO_LDFLAGS_$(filter %.lds,$(^F))) \
-		       -Wl,-T,$(filter %.lds,$^) $(filter %.o,$^) && \
+		       -Wl,-T,$(filter %.lds,$^) \
+		       -Wl,-T$(obj)/../../../../scripts/build-salt.lds \
+		       $(filter %.o,$^) && \
 		 sh $(srctree)/$(src)/checkundef.sh '$(NM)' '$@'
 
 VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=both) \
-- 
2.17.0

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-23  0:19 ` [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO Laura Abbott
@ 2018-05-23  0:33   ` Andy Lutomirski
  2018-05-23  1:23     ` Laura Abbott
  2018-05-23 22:53     ` Laura Abbott
  2018-05-24  1:55   ` Masahiro Yamada
  1 sibling, 2 replies; 13+ messages in thread
From: Andy Lutomirski @ 2018-05-23  0:33 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Andrew Lutomirski, Mark Wielaard, H. J. Lu, Masahiro Yamada,
	Linus Torvalds, X86 ML, LKML, Nick Clifton, Cary Coutant,
	linux-kbuild

On Tue, May 22, 2018 at 5:19 PM Laura Abbott <labbott@redhat.com> wrote:


> The vDSO is linked separately from the kernel and modules. Ensure it picks
> up the comment section, if available.

Did you end up preferring this to just sticking the kernel version in a
.comment in the vDSO for some reason?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-23  0:33   ` Andy Lutomirski
@ 2018-05-23  1:23     ` Laura Abbott
  2018-05-23 22:53     ` Laura Abbott
  1 sibling, 0 replies; 13+ messages in thread
From: Laura Abbott @ 2018-05-23  1:23 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Mark Wielaard, H. J. Lu, Masahiro Yamada, Linus Torvalds, X86 ML,
	LKML, Nick Clifton, Cary Coutant, linux-kbuild

On 05/22/2018 05:33 PM, Andy Lutomirski wrote:
> On Tue, May 22, 2018 at 5:19 PM Laura Abbott <labbott@redhat.com> wrote:
> 
> 
>> The vDSO is linked separately from the kernel and modules. Ensure it picks
>> up the comment section, if available.
> 
> Did you end up preferring this to just sticking the kernel version in a
> .comment in the vDSO for some reason?
> 

That's a good idea I hadn't considered. I'll do that for the next version.

Thanks,
Laura

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-23  0:33   ` Andy Lutomirski
  2018-05-23  1:23     ` Laura Abbott
@ 2018-05-23 22:53     ` Laura Abbott
  2018-05-23 23:55       ` Linus Torvalds
  2018-05-24  0:11       ` Masahiro Yamada
  1 sibling, 2 replies; 13+ messages in thread
From: Laura Abbott @ 2018-05-23 22:53 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Mark Wielaard, H. J. Lu, Masahiro Yamada, Linus Torvalds, X86 ML,
	LKML, Nick Clifton, Cary Coutant, linux-kbuild

On 05/22/2018 05:33 PM, Andy Lutomirski wrote:
> On Tue, May 22, 2018 at 5:19 PM Laura Abbott <labbott@redhat.com> wrote:
> 
> 
>> The vDSO is linked separately from the kernel and modules. Ensure it picks
>> up the comment section, if available.
> 
> Did you end up preferring this to just sticking the kernel version in a
> .comment in the vDSO for some reason?
> 

Actually I remember now why this is necessary: there is not a simple way
to encode a string into a linker file as it has to be spit out byte
by byte. The autogeneration was the easiest way to make that happen.
Maybe there's some horrific c preprocessing or other generation that
could happen but I doubt that's any worse than the generated linker
script.

Thanks,
Laura

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-23 22:53     ` Laura Abbott
@ 2018-05-23 23:55       ` Linus Torvalds
  2018-05-24  0:01         ` Andy Lutomirski
  2018-05-24  0:11       ` Masahiro Yamada
  1 sibling, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2018-05-23 23:55 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Andrew Lutomirski, mjw, H.J. Lu, Masahiro Yamada,
	the arch/x86 maintainers, Linux Kernel Mailing List, nickc,
	ccoutant, Linux Kbuild mailing list

On Wed, May 23, 2018 at 3:53 PM Laura Abbott <labbott@redhat.com> wrote:

> Actually I remember now why this is necessary: there is not a simple way
> to encode a string into a linker file as it has to be spit out byte
> by byte.

I think you can use the "fill" thing to basically add any random data to a
section.

So you can do something like

        . = ALIGN(16);
        .salt : AT(ADDR(.salt) - LOAD_OFFSET) {
                LONG(0xffaa5500);
                . = ALIGN(16);
        } =0x01234567890abcdef

in the lds file, and you'll get a section that looks like this:

   [torvalds@i7 linux]$ objdump -h vmlinux -j .salt -s

   vmlinux:     file format elf64-x86-64

   Sections:
    Idx Name          Size      VMA               LMA               File off
  Algn
     15 .salt         00000010  ffffffff8432b000  000000000432b000  0352b000
  2**0
                      CONTENTS, ALLOC, LOAD, DATA
    Contents of section .salt:
     ffffffff8432b000 0055aaff 00123456 7890abcd ef001234  .U....4Vx......4

Now whether that is sufficient for your needs, I dunno.

               Linus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-23 23:55       ` Linus Torvalds
@ 2018-05-24  0:01         ` Andy Lutomirski
       [not found]           ` <CA+55aFxXCWc5qCAsuRmrCGqL7ws4f6B_zy1WR98kHgr_XZs3fw@mail.gmail.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Andy Lutomirski @ 2018-05-24  0:01 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Laura Abbott, Andrew Lutomirski, mjw, H.J. Lu, Masahiro Yamada,
	the arch/x86 maintainers, Linux Kernel Mailing List, nickc,
	ccoutant, Linux Kbuild mailing list



> On May 23, 2018, at 4:55 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
>> On Wed, May 23, 2018 at 3:53 PM Laura Abbott <labbott@redhat.com> wrote:
>> 
>> Actually I remember now why this is necessary: there is not a simple way
>> to encode a string into a linker file as it has to be spit out byte
>> by byte.
> 
> I think you can use the "fill" thing to basically add any random data to a
> section.
> 
> So you can do something like
> 
>        . = ALIGN(16);
>        .salt : AT(ADDR(.salt) - LOAD_OFFSET) {
>                LONG(0xffaa5500);
>                . = ALIGN(16);
>        } =0x01234567890abcdef
> 
> in the lds file, and you'll get a section that looks like this:
> 
>   [torvalds@i7 linux]$ objdump -h vmlinux -j .salt -s
> 
>   vmlinux:     file format elf64-x86-64
> 
>   Sections:
>    Idx Name          Size      VMA               LMA               File off
>  Algn
>     15 .salt         00000010  ffffffff8432b000  000000000432b000  0352b000
>  2**0
>                      CONTENTS, ALLOC, LOAD, DATA
>    Contents of section .salt:
>     ffffffff8432b000 0055aaff 00123456 7890abcd ef001234  .U....4Vx......4
> 
> Now whether that is sufficient for your needs, I dunno.
> 

I don’t know whether I’m missing something obvious, but can’t this be in C?

asm (“.pushsection \”.comment\”; .ascii \”” WHATEVER “\”; .popsection”);

Or the .S equivalent.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-23 22:53     ` Laura Abbott
  2018-05-23 23:55       ` Linus Torvalds
@ 2018-05-24  0:11       ` Masahiro Yamada
  2018-05-24  1:43         ` Masahiro Yamada
  1 sibling, 1 reply; 13+ messages in thread
From: Masahiro Yamada @ 2018-05-24  0:11 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Andy Lutomirski, Mark Wielaard, H. J. Lu, Linus Torvalds, X86 ML,
	LKML, Nick Clifton, Cary Coutant, Linux Kbuild mailing list

2018-05-24 7:53 GMT+09:00 Laura Abbott <labbott@redhat.com>:
> On 05/22/2018 05:33 PM, Andy Lutomirski wrote:
>>
>> On Tue, May 22, 2018 at 5:19 PM Laura Abbott <labbott@redhat.com> wrote:
>>
>>
>>> The vDSO is linked separately from the kernel and modules. Ensure it
>>> picks
>>> up the comment section, if available.
>>
>>
>> Did you end up preferring this to just sticking the kernel version in a
>> .comment in the vDSO for some reason?
>>
>
> Actually I remember now why this is necessary: there is not a simple way
> to encode a string into a linker file as it has to be spit out byte
> by byte. The autogeneration was the easiest way to make that happen.
> Maybe there's some horrific c preprocessing or other generation that
> could happen but I doubt that's any worse than the generated linker
> script.
>


I am personally prefer CONFIG option (as you did in v2) to KERNELVERSION.


If you use "hex" type instead of "string" type in Kconfig,
and LONG() instead of BYTE() in the script script,
this can be much simpler, right?





config BUILD_ID_SALT
        hex "Build ID Salt"
        help
           ...




Then, in scripts/Makefile,


define filechk_build-salt.lds
        { \
                echo "SECTIONS {"; \
                echo ".comment (INFO) : { LONG($(CONFIG_BUILD_ID_SALT)); }"; \
                echo "}"; \
        }
endef

$(obj)/build-salt.lds: $(src)/Makefile FORCE
        $(call filechk,build-salt.lds)




This is now so simple that we can even remove the shell script.





-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
       [not found]           ` <CA+55aFxXCWc5qCAsuRmrCGqL7ws4f6B_zy1WR98kHgr_XZs3fw@mail.gmail.com>
@ 2018-05-24  1:36             ` Laura Abbott
  0 siblings, 0 replies; 13+ messages in thread
From: Laura Abbott @ 2018-05-24  1:36 UTC (permalink / raw)
  To: Linus Torvalds, Andy Lutomirski
  Cc: Andrew Lutomirski, mjw, H.J. Lu, Masahiro Yamada,
	the arch/x86 maintainers, Linux Kernel Mailing List, nickc,
	ccoutant, Linux Kbuild mailing list

On 05/23/2018 05:06 PM, Linus Torvalds wrote:
> 
> 
> On Wed, May 23, 2018, 17:01 Andy Lutomirski <luto@amacapital.net <mailto:luto@amacapital.net>> wrote:
> 
> 
>     I don’t know whether I’m missing something obvious, but can’t this be in C?
> 
> 
> Yes, but I thought Laura wanted to limit it to linker file tricks (this thread has gone on for so long that I've forgotten the details of why).
> 
>         Linus
> 
> 

So we have to update the kernel and every module and the easiest way
to do that was the linker script. I was assuming I'd just use the
same approach for the vDSO but you're right that there's no reason
we can't apply a different technique. I notice there's already a
vdso-note.S which adds LINUX_VERSION_CODE as a note. This doesn't
include the extra version so it doesn't quite meet our needs.
There's no reason why we can't throw something else in there for
good measure.

Thanks,
Laura

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-24  0:11       ` Masahiro Yamada
@ 2018-05-24  1:43         ` Masahiro Yamada
  2018-05-24  2:16           ` Laura Abbott
  0 siblings, 1 reply; 13+ messages in thread
From: Masahiro Yamada @ 2018-05-24  1:43 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Andy Lutomirski, Mark Wielaard, H. J. Lu, Linus Torvalds, X86 ML,
	LKML, Nick Clifton, Cary Coutant, Linux Kbuild mailing list

2018-05-24 9:11 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> 2018-05-24 7:53 GMT+09:00 Laura Abbott <labbott@redhat.com>:
>> On 05/22/2018 05:33 PM, Andy Lutomirski wrote:
>>>
>>> On Tue, May 22, 2018 at 5:19 PM Laura Abbott <labbott@redhat.com> wrote:
>>>
>>>
>>>> The vDSO is linked separately from the kernel and modules. Ensure it
>>>> picks
>>>> up the comment section, if available.
>>>
>>>
>>> Did you end up preferring this to just sticking the kernel version in a
>>> .comment in the vDSO for some reason?
>>>
>>
>> Actually I remember now why this is necessary: there is not a simple way
>> to encode a string into a linker file as it has to be spit out byte
>> by byte. The autogeneration was the easiest way to make that happen.
>> Maybe there's some horrific c preprocessing or other generation that
>> could happen but I doubt that's any worse than the generated linker
>> script.
>>
>
>
> I am personally prefer CONFIG option (as you did in v2) to KERNELVERSION.
>
>
> If you use "hex" type instead of "string" type in Kconfig,
> and LONG() instead of BYTE() in the script script,
> this can be much simpler, right?
>
>
>
>
>
> config BUILD_ID_SALT
>         hex "Build ID Salt"
>         help
>            ...
>
>
>
>
> Then, in scripts/Makefile,
>
>
> define filechk_build-salt.lds
>         { \
>                 echo "SECTIONS {"; \
>                 echo ".comment (INFO) : { LONG($(CONFIG_BUILD_ID_SALT)); }"; \
>                 echo "}"; \
>         }
> endef
>
> $(obj)/build-salt.lds: $(src)/Makefile FORCE
>         $(call filechk,build-salt.lds)
>
>
>
>
> This is now so simple that we can even remove the shell script.



I had not noticed the comments from Linus and Andy
before I posted mine.


Maybe, we should not add binary data into the .comment section.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-23  0:19 ` [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO Laura Abbott
  2018-05-23  0:33   ` Andy Lutomirski
@ 2018-05-24  1:55   ` Masahiro Yamada
  1 sibling, 0 replies; 13+ messages in thread
From: Masahiro Yamada @ 2018-05-24  1:55 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Andy Lutomirski, Mark Wielaard, H . J . Lu, Linus Torvalds,
	X86 ML, Linux Kernel Mailing List, Nick Clifton, Cary Coutant,
	Linux Kbuild mailing list

2018-05-23 9:19 GMT+09:00 Laura Abbott <labbott@redhat.com>:
>
> The vDSO is linked separately from the kernel and modules. Ensure it picks
> up the comment section, if available.
>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v3: Invoke the generated linker script. The ".." nightmare is pretty
> ugly but I didn't see an easier way to pick up the generated file.
> That was actually part of my motivation for using an #include since
> paths for those are standardized.


Maybe, you found a different approach.

FWIW,

If you use the linker script approach, you could simply use

scripts/build-salt.lds

  instead of

$(obj)/../../../../scripts/build-salt.lds




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO
  2018-05-24  1:43         ` Masahiro Yamada
@ 2018-05-24  2:16           ` Laura Abbott
  0 siblings, 0 replies; 13+ messages in thread
From: Laura Abbott @ 2018-05-24  2:16 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Andy Lutomirski, Mark Wielaard, H. J. Lu, Linus Torvalds, X86 ML,
	LKML, Nick Clifton, Cary Coutant, Linux Kbuild mailing list

On 05/23/2018 06:43 PM, Masahiro Yamada wrote:
> 2018-05-24 9:11 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
>> 2018-05-24 7:53 GMT+09:00 Laura Abbott <labbott@redhat.com>:
>>> On 05/22/2018 05:33 PM, Andy Lutomirski wrote:
>>>>
>>>> On Tue, May 22, 2018 at 5:19 PM Laura Abbott <labbott@redhat.com> wrote:
>>>>
>>>>
>>>>> The vDSO is linked separately from the kernel and modules. Ensure it
>>>>> picks
>>>>> up the comment section, if available.
>>>>
>>>>
>>>> Did you end up preferring this to just sticking the kernel version in a
>>>> .comment in the vDSO for some reason?
>>>>
>>>
>>> Actually I remember now why this is necessary: there is not a simple way
>>> to encode a string into a linker file as it has to be spit out byte
>>> by byte. The autogeneration was the easiest way to make that happen.
>>> Maybe there's some horrific c preprocessing or other generation that
>>> could happen but I doubt that's any worse than the generated linker
>>> script.
>>>
>>
>>
>> I am personally prefer CONFIG option (as you did in v2) to KERNELVERSION.
>>
>>
>> If you use "hex" type instead of "string" type in Kconfig,
>> and LONG() instead of BYTE() in the script script,
>> this can be much simpler, right?
>>
>>
>>
>>
>>
>> config BUILD_ID_SALT
>>          hex "Build ID Salt"
>>          help
>>             ...
>>
>>
>>
>>
>> Then, in scripts/Makefile,
>>
>>
>> define filechk_build-salt.lds
>>          { \
>>                  echo "SECTIONS {"; \
>>                  echo ".comment (INFO) : { LONG($(CONFIG_BUILD_ID_SALT)); }"; \
>>                  echo "}"; \
>>          }
>> endef
>>
>> $(obj)/build-salt.lds: $(src)/Makefile FORCE
>>          $(call filechk,build-salt.lds)
>>
>>
>>
>>
>> This is now so simple that we can even remove the shell script.
> 
> 
> 
> I had not noticed the comments from Linus and Andy
> before I posted mine.
> 
> 
> Maybe, we should not add binary data into the .comment section.
> 
> 
> 

The comments from Linus and Andy apply to the vDSO but I don't
think they work for the kernel/modules. We need something that
can apply to every module and the kernel and the linker script
seems like easiest way to do that. The vDSO is a self-contained
binary so it makes sense to not use the linker script there and
instead throw something in one of the existing files.

I'm kind of iffy about making the build-id salt a hex string
since that requires bit more work to generate. I'll experiment
in a new version.

Thanks,
Laura

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-05-24  2:16 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-23  0:19 [PATCHv3 0/2] Salted build ids via linker sections Laura Abbott
2018-05-23  0:19 ` [PATCHv3 1/2] kbuild: Introduce build-salt linker script Laura Abbott
2018-05-23  0:19 ` [PATCHv3 2/2] x86/vdso: Add build salt to the vDSO Laura Abbott
2018-05-23  0:33   ` Andy Lutomirski
2018-05-23  1:23     ` Laura Abbott
2018-05-23 22:53     ` Laura Abbott
2018-05-23 23:55       ` Linus Torvalds
2018-05-24  0:01         ` Andy Lutomirski
     [not found]           ` <CA+55aFxXCWc5qCAsuRmrCGqL7ws4f6B_zy1WR98kHgr_XZs3fw@mail.gmail.com>
2018-05-24  1:36             ` Laura Abbott
2018-05-24  0:11       ` Masahiro Yamada
2018-05-24  1:43         ` Masahiro Yamada
2018-05-24  2:16           ` Laura Abbott
2018-05-24  1:55   ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).