[RFC PATCH 0/3] Salted build ids via linker sections

[RFC PATCH 0/3] Salted build ids via linker sections

[Laura Abbott]

Hi,

This is a proposal that's come out of several discussion for Fedora but
may be of interest to other distributions.

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.

This is an implementation of the counter proposal. If an environment variable
is set, that value gets added as a .comment section to the necessary binaries.
This is RFC mostly to see if this approach is plausible enough to be merged or
if it's worth trying to update binutils with the --build-id-salt option.
I don't care about any names, I was pretty inconsistent. If people are
roughly satisifed with this I'll probably just change all the names to
build-salt or whatever gets bike shedded.

Thanks,
Laura

Laura Abbott (3):
  kbuild: Introduce build-salt generated header
  kbuild: Link with generated build-salt header
  x86/vdso: Add build salt to the vDSO

 Makefile                              | 13 +++++++++++--
 arch/x86/entry/vdso/vdso-layout.lds.S |  3 +++
 scripts/.gitignore                    |  1 +
 scripts/Makefile                      |  2 +-
 scripts/build-id.lds.S                |  5 +++++
 scripts/gencomment                    | 19 +++++++++++++++++++
 scripts/link-vmlinux.sh               |  3 ++-
 7 files changed, 42 insertions(+), 4 deletions(-)
 create mode 100644 scripts/build-id.lds.S
 create mode 100755 scripts/gencomment

-- 
2.16.2

[RFC PATCH 1/3] kbuild: Introduce build-salt generated header

[Laura Abbott]

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 header which is generated from an environment varible, BUILD_TAG.
If this variable is set, an appropriate .comment section is generated. If
the environment variable isn't set, the define is simply empty and there
is no change to the build.

Suggested-by: Nick Clifton <nickc@redhat.com>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 Makefile           |  9 ++++++++-
 scripts/gencomment | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100755 scripts/gencomment

diff --git a/Makefile b/Makefile
index d65e2e229017..de360625aec5 100644
--- a/Makefile
+++ b/Makefile
@@ -1087,7 +1087,7 @@ endif
 prepare2: prepare3 prepare-compiler-check outputmakefile asm-generic
 
 prepare1: prepare2 $(version_h) include/generated/utsrelease.h \
-                   include/config/auto.conf
+                   include/config/auto.conf include/generated/build-salt.h
 	$(cmd_crmodverdir)
 
 archprepare: archheaders archscripts prepare1 scripts_basic
@@ -1175,6 +1175,13 @@ $(version_h): $(srctree)/Makefile FORCE
 include/generated/utsrelease.h: include/config/kernel.release FORCE
 	$(call filechk,utsrelease.h)
 
+define filechk_build-salt.h
+	($(CONFIG_SHELL) $(srctree)/scripts/gencomment)
+endef
+
+include/generated/build-salt.h: $(srctree)/Makefile FORCE
+	$(call filechk,build-salt.h)
+
 PHONY += headerdep
 headerdep:
 	$(Q)find $(srctree)/include/ -name '*.h' | xargs --max-args 1 \
diff --git a/scripts/gencomment b/scripts/gencomment
new file mode 100755
index 000000000000..13b6e7739ef7
--- /dev/null
+++ b/scripts/gencomment
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+if [ -z $BUILD_TAG ]; then
+	echo "#define ID_TAG"
+	exit 0
+fi
+
+echo "#define ID_TAG \\"
+echo ".comment (INFO) : \\"
+echo " { \\"
+
+_TAG=`echo $BUILD_TAG | 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 " } "
-- 
2.16.2

[RFC PATCH 2/3] kbuild: Link with generated build-salt header

[Laura Abbott]

Now that we have a header with appropriate data, link it into the kernel
and modules. If BUILD_TAG isn't set, the script should be empty.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 Makefile                | 4 +++-
 scripts/.gitignore      | 1 +
 scripts/Makefile        | 2 +-
 scripts/build-id.lds.S  | 5 +++++
 scripts/link-vmlinux.sh | 3 ++-
 5 files changed, 12 insertions(+), 3 deletions(-)
 create mode 100644 scripts/build-id.lds.S

diff --git a/Makefile b/Makefile
index de360625aec5..8bef8bfe7a82 100644
--- a/Makefile
+++ b/Makefile
@@ -425,7 +425,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 $(srctree)/scripts/build-id.lds
 GCC_PLUGINS_CFLAGS :=
 
 export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
@@ -995,6 +996,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-id.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..6211a33c6f59 100644
--- a/scripts/.gitignore
+++ b/scripts/.gitignore
@@ -13,3 +13,4 @@ asn1_compiler
 extract-cert
 sign-file
 insert-sys-cert
+build-id.lds
diff --git a/scripts/Makefile b/scripts/Makefile
index 25ab143cbe14..d343dab5d934 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -25,7 +25,7 @@ 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-id.lds
 
 # The following hostprogs-y programs are only build on demand
 hostprogs-y += unifdef
diff --git a/scripts/build-id.lds.S b/scripts/build-id.lds.S
new file mode 100644
index 000000000000..bbc3a8e260c1
--- /dev/null
+++ b/scripts/build-id.lds.S
@@ -0,0 +1,5 @@
+#include <generated/build-salt.h>
+
+SECTIONS {
+	ID_TAG
+}
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index be56a1153014..2b57e0139acb 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -93,6 +93,7 @@ modpost_link()
 vmlinux_link()
 {
 	local lds="${objtree}/${KBUILD_LDS}"
+	local extra_lds="${objtree}/${EXTRA_LDS}"
 	local objects
 
 	if [ "${SRCARCH}" != "um" ]; then
@@ -114,7 +115,7 @@ vmlinux_link()
 		fi
 
 		${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2}		\
-			-T ${lds} ${objects}
+			-T ${lds} -T ${extra_lds} ${objects}
 	else
 		if [ -n "${CONFIG_THIN_ARCHIVES}" ]; then
 			objects="-Wl,--whole-archive			\
-- 
2.16.2

[RFC PATCH 3/3] x86/vdso: Add build salt to the vDSO

[Laura Abbott]

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>
---
I only added x86 in this series because that's my primary interest.
Other arches could easily follow suit if they care.
---
 arch/x86/entry/vdso/vdso-layout.lds.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/entry/vdso/vdso-layout.lds.S b/arch/x86/entry/vdso/vdso-layout.lds.S
index acfd5ba7d943..ab72f3c87ebb 100644
--- a/arch/x86/entry/vdso/vdso-layout.lds.S
+++ b/arch/x86/entry/vdso/vdso-layout.lds.S
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #include <asm/vdso.h>
+#include <generated/build-salt.h>
 
 /*
  * Linker script for vDSO.  This is an ELF shared object prelinked to
@@ -95,6 +96,8 @@ SECTIONS
 	.altinstructions	: { *(.altinstructions) }	:text
 	.altinstr_replacement	: { *(.altinstr_replacement) }	:text
 
+	ID_TAG
+
 	/DISCARD/ : {
 		*(.discard)
 		*(.discard.*)
-- 
2.16.2

Re: [RFC PATCH 0/3] Salted build ids via linker sections

[Nick Clifton]

Hi Laura,

> This is an implementation of the counter proposal. If an environment variable
> is set, that value gets added as a .comment section to the necessary binaries.

This is not really my purview but ... can I just say that I really do not like
using environment variables to control build behaviour.  It makes reproducing a
build much harder.  Especially when you are trying to reproduce a bug report from
a user who does not even know that the environment variable exits.

Cheers
  Nick

Re: [RFC PATCH 0/3] Salted build ids via linker sections

[Masahiro Yamada]

2018-03-21 10:46 GMT+09:00 Laura Abbott <labbott@redhat.com>:
> Hi,
>
> This is a proposal that's come out of several discussion for Fedora but
> may be of interest to other distributions.
>
> 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)

Is this a kernel-specific problem?

IIUC, the URL above is discussing packages in general.

Any executable from slightly different versions
could result in identical ELF.
If so, will you tweak link process of every package
that needs parallel installation?




> 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.
>
> This is an implementation of the counter proposal. If an environment variable
> is set, that value gets added as a .comment section to the necessary binaries.
> This is RFC mostly to see if this approach is plausible enough to be merged or
> if it's worth trying to update binutils with the --build-id-salt option.
> I don't care about any names, I was pretty inconsistent. If people are
> roughly satisifed with this I'll probably just change all the names to
> build-salt or whatever gets bike shedded.
>
> Thanks,
> Laura
>
> Laura Abbott (3):
>   kbuild: Introduce build-salt generated header
>   kbuild: Link with generated build-salt header
>   x86/vdso: Add build salt to the vDSO
>
>  Makefile                              | 13 +++++++++++--
>  arch/x86/entry/vdso/vdso-layout.lds.S |  3 +++
>  scripts/.gitignore                    |  1 +
>  scripts/Makefile                      |  2 +-
>  scripts/build-id.lds.S                |  5 +++++
>  scripts/gencomment                    | 19 +++++++++++++++++++
>  scripts/link-vmlinux.sh               |  3 ++-
>  7 files changed, 42 insertions(+), 4 deletions(-)
>  create mode 100644 scripts/build-id.lds.S
>  create mode 100755 scripts/gencomment
>
> --
> 2.16.2
>



-- 
Best Regards
Masahiro Yamada

Re: [RFC PATCH 0/3] Salted build ids via linker sections

[Mark Wielaard]

Hi,

On Mon, Mar 26, 2018 at 04:48:11PM +0900, Masahiro Yamada wrote:
> 2018-03-21 10:46 GMT+09:00 Laura Abbott <labbott@redhat.com>:
> > 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)
> 
> Is this a kernel-specific problem?
> 
> IIUC, the URL above is discussing packages in general.
> 
> Any executable from slightly different versions
> could result in identical ELF.
> If so, will you tweak link process of every package
> that needs parallel installation?

For almost any package it isn't necessary to tweak the link process
because the build-id can be tweaked if necessary at any stage of the
package process (the most convenient is during debuginfo splitting,
because the debuginfo contains almost all the information of the
build environment). The kernel vdso build is a bit special because it
embeds the executable also inside the kernel image (so it shows up at
runtime again not from the file system, but by being inserted into the
process by the kernel), where packaging tools cannot easily find it.

Cheers,

Mark