All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFCv2 PATCH 0/3] Salted build ids via linker sections
@ 2018-03-29 18:01 Laura Abbott
  2018-03-29 18:01 ` [RFCv2 PATCH 1/3] kbuild: Introduce build-salt generated header Laura Abbott
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Laura Abbott @ 2018-03-29 18:01 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 v2 of my proposal to allow unique build-ids in the kernel. from
last time:

""
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 v2 cleans up the naming to be consistent and also switches to a
config option vs. an environment variable. I've seen some sporadic
failures about missing the generated header so I think I'm still missing
a dependency somewhere. I'm still mostly looking for feedback whether
this would be acceptable for merging or if we should just persue a
--build-id-salt in binutils.

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 +++
 init/Kconfig                          |  8 ++++++++
 scripts/.gitignore                    |  1 +
 scripts/Makefile                      |  2 +-
 scripts/build-salt.lds.S              |  5 +++++
 scripts/gensalt                       | 21 +++++++++++++++++++++
 scripts/link-vmlinux.sh               |  3 ++-
 8 files changed, 52 insertions(+), 4 deletions(-)
 create mode 100644 scripts/build-salt.lds.S
 create mode 100755 scripts/gensalt

-- 
2.16.2

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

* [RFCv2 PATCH 1/3] kbuild: Introduce build-salt generated header
  2018-03-29 18:01 [RFCv2 PATCH 0/3] Salted build ids via linker sections Laura Abbott
@ 2018-03-29 18:01 ` Laura Abbott
  2018-03-29 18:01 ` [RFCv2 PATCH 2/3] kbuild: Link with generated build-salt header Laura Abbott
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Laura Abbott @ 2018-03-29 18:01 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 header which is generated from a config setting. If this config is
set, an appropriate .comment section is generated. If the config isn't set,
the define is simply empty and there is no change to the build.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v2: Switched to Kconfig vs. environment variable per suggestion of Nick
Clifton. Changed names to be consistent.
---
 Makefile        |  9 ++++++++-
 init/Kconfig    |  8 ++++++++
 scripts/gensalt | 21 +++++++++++++++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100755 scripts/gensalt

diff --git a/Makefile b/Makefile
index 7ba478ab8c82..b80c2d6d0854 100644
--- a/Makefile
+++ b/Makefile
@@ -1096,7 +1096,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
@@ -1184,6 +1184,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/gensalt $(CONFIG_BUILD_ID_SALT))
+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/init/Kconfig b/init/Kconfig
index e37f4b2a6445..01e77aef3610 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1924,3 +1924,11 @@ source "kernel/Kconfig.locks"
 
 config ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
 	bool
+
+config BUILD_ID_SALT
+	string "Build ID Salt"
+	help
+	  The build ID is used to link binaries and their debug info. Setting
+          this option will use the value in the calculation of the build id.
+          This is mostly useful for distributions which want to ensure the
+          build is unique between builds. It's safe to leave this empty.
diff --git a/scripts/gensalt b/scripts/gensalt
new file mode 100755
index 000000000000..355a3e799550
--- /dev/null
+++ b/scripts/gensalt
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+if [[ $1 = "" ]]; then
+	echo "#define BUILD_ID_SALT"
+	exit 0
+fi
+
+BUILD_ID_SALT=$1
+
+echo "#define BUILD_ID_SALT \\"
+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 " } "
-- 
2.16.2

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

* [RFCv2 PATCH 2/3] kbuild: Link with generated build-salt header
  2018-03-29 18:01 [RFCv2 PATCH 0/3] Salted build ids via linker sections Laura Abbott
  2018-03-29 18:01 ` [RFCv2 PATCH 1/3] kbuild: Introduce build-salt generated header Laura Abbott
@ 2018-03-29 18:01 ` Laura Abbott
  2018-05-07  6:38   ` Masahiro Yamada
  2018-03-29 18:01 ` [RFCv2 PATCH 3/3] x86/vdso: Add build salt to the vDSO Laura Abbott
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Laura Abbott @ 2018-03-29 18:01 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

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

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

diff --git a/Makefile b/Makefile
index b80c2d6d0854..171f82295e5e 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-salt.lds
 GCC_PLUGINS_CFLAGS :=
 
 export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
@@ -1004,6 +1005,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..47f6ed5b0bcd 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-salt.lds
 
 # The following hostprogs-y programs are only build on demand
 hostprogs-y += unifdef
diff --git a/scripts/build-salt.lds.S b/scripts/build-salt.lds.S
new file mode 100644
index 000000000000..f85981f1187e
--- /dev/null
+++ b/scripts/build-salt.lds.S
@@ -0,0 +1,5 @@
+#include <generated/build-salt.h>
+
+SECTIONS {
+	BUILD_ID_SALT
+}
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

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

* [RFCv2 PATCH 3/3] x86/vdso: Add build salt to the vDSO
  2018-03-29 18:01 [RFCv2 PATCH 0/3] Salted build ids via linker sections Laura Abbott
  2018-03-29 18:01 ` [RFCv2 PATCH 1/3] kbuild: Introduce build-salt generated header Laura Abbott
  2018-03-29 18:01 ` [RFCv2 PATCH 2/3] kbuild: Link with generated build-salt header Laura Abbott
@ 2018-03-29 18:01 ` Laura Abbott
  2018-05-07  6:42   ` Masahiro Yamada
  2018-03-30 12:40 ` [RFCv2 PATCH 0/3] Salted build ids via linker sections Mark Wielaard
  2018-05-07  6:28 ` Masahiro Yamada
  4 siblings, 1 reply; 13+ messages in thread
From: Laura Abbott @ 2018-03-29 18:01 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>
---
v2: Updated name
---
 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..7c78dd39aae8 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
 
+	BUILD_ID_SALT
+
 	/DISCARD/ : {
 		*(.discard)
 		*(.discard.*)
-- 
2.16.2

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

* Re: [RFCv2 PATCH 0/3] Salted build ids via linker sections
  2018-03-29 18:01 [RFCv2 PATCH 0/3] Salted build ids via linker sections Laura Abbott
                   ` (2 preceding siblings ...)
  2018-03-29 18:01 ` [RFCv2 PATCH 3/3] x86/vdso: Add build salt to the vDSO Laura Abbott
@ 2018-03-30 12:40 ` Mark Wielaard
  2018-05-07  6:58   ` Masahiro Yamada
  2018-05-07  6:28 ` Masahiro Yamada
  4 siblings, 1 reply; 13+ messages in thread
From: Mark Wielaard @ 2018-03-30 12:40 UTC (permalink / raw)
  To: Laura Abbott, Andy Lutomirski, H . J . Lu, Masahiro Yamada
  Cc: Linus Torvalds, X86 ML, linux-kernel, Nick Clifton, Cary Coutant,
	linux-kbuild

On Thu, 2018-03-29 at 11:01 -0700, Laura Abbott wrote:
> I'm still mostly looking for feedback whether
> this would be acceptable for merging or if we should just persue a
> --build-id-salt in binutils.

Personally I would go with this approach. It seems simple and it might
take years before a new linker option is available everywhere.

To simplify things I think you could just always add the extra vdso
.comment initialized to something like KERNELRELEASE. Which distros
seem to update anyway to include their build number, so they wouldn't
need to do anything special to "update the build salt".

Cheers,

Mark

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

* Re: [RFCv2 PATCH 0/3] Salted build ids via linker sections
  2018-03-29 18:01 [RFCv2 PATCH 0/3] Salted build ids via linker sections Laura Abbott
                   ` (3 preceding siblings ...)
  2018-03-30 12:40 ` [RFCv2 PATCH 0/3] Salted build ids via linker sections Mark Wielaard
@ 2018-05-07  6:28 ` Masahiro Yamada
  2018-05-14 21:02   ` Laura Abbott
  4 siblings, 1 reply; 13+ messages in thread
From: Masahiro Yamada @ 2018-05-07  6:28 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Andy Lutomirski, mjw, H . J . Lu, Linus Torvalds, X86 ML,
	Linux Kernel Mailing List, Nick Clifton, Cary Coutant,
	Linux Kbuild mailing list

2018-03-30 3:01 GMT+09:00 Laura Abbott <labbott@redhat.com>:
> Hi,
>
> This is v2 of my proposal to allow unique build-ids in the kernel. from
> last time:
>
> ""
> 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 think you already know '--build-id=uuid' linker option.

Doesn't this solve your problem?

The disadvantage of this option is,
we will lose reproducible building because  --build-id=uuid
adds every time random salt.

The advantage is, the implementation is even simpler,
and easier to migrate to --build-id-salt once it is supported
in the future.


> This v2 cleans up the naming to be consistent and also switches to a
> config option vs. an environment variable. I've seen some sporadic
> failures about missing the generated header so I think I'm still missing
> a dependency somewhere.

Right.

There is no dependency between 'prepare' and 'scripts'
in the top Makefile.
Therefore, Kbuild can run them simultaneously,
which would cause a race in parallel building.








 I'm still mostly looking for feedback whether
> this would be acceptable for merging or if we should just persue a
> --build-id-salt in binutils.
>
> 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 +++
>  init/Kconfig                          |  8 ++++++++
>  scripts/.gitignore                    |  1 +
>  scripts/Makefile                      |  2 +-
>  scripts/build-salt.lds.S              |  5 +++++
>  scripts/gensalt                       | 21 +++++++++++++++++++++
>  scripts/link-vmlinux.sh               |  3 ++-
>  8 files changed, 52 insertions(+), 4 deletions(-)
>  create mode 100644 scripts/build-salt.lds.S
>  create mode 100755 scripts/gensalt
>
> --
> 2.16.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

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

* Re: [RFCv2 PATCH 2/3] kbuild: Link with generated build-salt header
  2018-03-29 18:01 ` [RFCv2 PATCH 2/3] kbuild: Link with generated build-salt header Laura Abbott
@ 2018-05-07  6:38   ` Masahiro Yamada
  0 siblings, 0 replies; 13+ messages in thread
From: Masahiro Yamada @ 2018-05-07  6:38 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Andy Lutomirski, mjw, H . J . Lu, Linus Torvalds, X86 ML,
	Linux Kernel Mailing List, Nick Clifton, Cary Coutant,
	Linux Kbuild mailing list

2018-03-30 3:01 GMT+09:00 Laura Abbott <labbott@redhat.com>:

> diff --git a/scripts/build-salt.lds.S b/scripts/build-salt.lds.S
> new file mode 100644
> index 000000000000..f85981f1187e
> --- /dev/null
> +++ b/scripts/build-salt.lds.S
> @@ -0,0 +1,5 @@
> +#include <generated/build-salt.h>
> +
> +SECTIONS {
> +       BUILD_ID_SALT
> +}


Do you need this just for wrapping <generate/build-salt.h> ?


How about generating scripts/build-salt.lds
directly by scripts/gensalt ?

<generated/build-salt.h> is unneeded, then
"some sporadic failures about missing the generated header"
will be solved.



I think something like follows will be simpler and better.


always := $(hostprogs-y) $(hostprogs-m) build-salt.lds

define filechk_build-salt.lds
        ($(CONFIG_SHELL) $(srctree)/scripts/gensalt $(CONFIG_BUILD_ID_SALT))
endef

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




-- 
Best Regards
Masahiro Yamada

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

* Re: [RFCv2 PATCH 3/3] x86/vdso: Add build salt to the vDSO
  2018-03-29 18:01 ` [RFCv2 PATCH 3/3] x86/vdso: Add build salt to the vDSO Laura Abbott
@ 2018-05-07  6:42   ` Masahiro Yamada
  0 siblings, 0 replies; 13+ messages in thread
From: Masahiro Yamada @ 2018-05-07  6:42 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Andy Lutomirski, mjw, H . J . Lu, Linus Torvalds, X86 ML,
	Linux Kernel Mailing List, Nick Clifton, Cary Coutant,
	Linux Kbuild mailing list

2018-03-30 3:01 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>
> ---
> v2: Updated name
> ---
>  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..7c78dd39aae8 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
>
> +       BUILD_ID_SALT
> +
>         /DISCARD/ : {
>                 *(.discard)
>                 *(.discard.*)
> --
> 2.16.2
>


For consistency, I think '-T scripts/build-salt.lds'
should be passed to cmd_vdso
in arch/x86/entry/vdso/Makefile.




-- 
Best Regards
Masahiro Yamada

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

* Re: [RFCv2 PATCH 0/3] Salted build ids via linker sections
  2018-03-30 12:40 ` [RFCv2 PATCH 0/3] Salted build ids via linker sections Mark Wielaard
@ 2018-05-07  6:58   ` Masahiro Yamada
  2018-05-08  2:49     ` Andy Lutomirski
  0 siblings, 1 reply; 13+ messages in thread
From: Masahiro Yamada @ 2018-05-07  6:58 UTC (permalink / raw)
  To: Mark Wielaard
  Cc: Laura Abbott, Andy Lutomirski, H . J . Lu, Linus Torvalds,
	X86 ML, Linux Kernel Mailing List, Nick Clifton, Cary Coutant,
	Linux Kbuild mailing list

2018-03-30 21:40 GMT+09:00 Mark Wielaard <mjw@fedoraproject.org>:
> On Thu, 2018-03-29 at 11:01 -0700, Laura Abbott wrote:
>> I'm still mostly looking for feedback whether
>> this would be acceptable for merging or if we should just persue a
>> --build-id-salt in binutils.
>
> Personally I would go with this approach. It seems simple and it might
> take years before a new linker option is available everywhere.


Indeed.  This series is easier than --build-id-salt.

If you do not see any better solution, I can accept this.


BTW, when I read
https://fedoraproject.org/wiki/Changes/ParallelInstallableDebuginfo
I thought "we could reverse the symlink direction from debug file to
build-id file)"
sensible (but I understand it is not easy to change this way).


If two packages share an identical image,
one package can borrow the image from the other,
then the storage space will be saved.

So, having identical ID should be advantage,
but we actually see only disadvantage...




> To simplify things I think you could just always add the extra vdso
> .comment initialized to something like KERNELRELEASE. Which distros
> seem to update anyway to include their build number, so they wouldn't
> need to do anything special to "update the build salt".
>






-- 
Best Regards
Masahiro Yamada

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

* Re: [RFCv2 PATCH 0/3] Salted build ids via linker sections
  2018-05-07  6:58   ` Masahiro Yamada
@ 2018-05-08  2:49     ` Andy Lutomirski
  2018-05-14 20:51       ` Laura Abbott
  0 siblings, 1 reply; 13+ messages in thread
From: Andy Lutomirski @ 2018-05-08  2:49 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Mark Wielaard, Laura Abbott, Andrew Lutomirski, H. J. Lu,
	Linus Torvalds, X86 ML, LKML, Nick Clifton, Cary Coutant,
	linux-kbuild

On Sun, May 6, 2018 at 11:59 PM Masahiro Yamada <
yamada.masahiro@socionext.com> wrote:

> 2018-03-30 21:40 GMT+09:00 Mark Wielaard <mjw@fedoraproject.org>:
> > On Thu, 2018-03-29 at 11:01 -0700, Laura Abbott wrote:
> >> I'm still mostly looking for feedback whether
> >> this would be acceptable for merging or if we should just persue a
> >> --build-id-salt in binutils.
> >
> > Personally I would go with this approach. It seems simple and it might
> > take years before a new linker option is available everywhere.


> Indeed.  This series is easier than --build-id-salt.

> If you do not see any better solution, I can accept this.


> BTW, when I read
> https://fedoraproject.org/wiki/Changes/ParallelInstallableDebuginfo
> I thought "we could reverse the symlink direction from debug file to
> build-id file)"
> sensible (but I understand it is not easy to change this way).


> If two packages share an identical image,
> one package can borrow the image from the other,
> then the storage space will be saved.

> So, having identical ID should be advantage,
> but we actually see only disadvantage...




> > To simplify things I think you could just always add the extra vdso
> > .comment initialized to something like KERNELRELEASE. Which distros
> > seem to update anyway to include their build number, so they wouldn't
> > need to do anything special to "update the build salt".
> >

That's what I was thinking, too.  Would that solve Fedora's problem?

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

* Re: [RFCv2 PATCH 0/3] Salted build ids via linker sections
  2018-05-08  2:49     ` Andy Lutomirski
@ 2018-05-14 20:51       ` Laura Abbott
  0 siblings, 0 replies; 13+ messages in thread
From: Laura Abbott @ 2018-05-14 20:51 UTC (permalink / raw)
  To: Andy Lutomirski, Masahiro Yamada
  Cc: Mark Wielaard, H. J. Lu, Linus Torvalds, X86 ML, LKML,
	Nick Clifton, Cary Coutant, linux-kbuild

On 05/07/2018 07:49 PM, Andy Lutomirski wrote:
> On Sun, May 6, 2018 at 11:59 PM Masahiro Yamada <
> yamada.masahiro@socionext.com> wrote:
> 
>> 2018-03-30 21:40 GMT+09:00 Mark Wielaard <mjw@fedoraproject.org>:
>>> On Thu, 2018-03-29 at 11:01 -0700, Laura Abbott wrote:
>>>> I'm still mostly looking for feedback whether
>>>> this would be acceptable for merging or if we should just persue a
>>>> --build-id-salt in binutils.
>>>
>>> Personally I would go with this approach. It seems simple and it might
>>> take years before a new linker option is available everywhere.
> 
> 
>> Indeed.  This series is easier than --build-id-salt.
> 
>> If you do not see any better solution, I can accept this.
> 
> 
>> BTW, when I read
>> https://fedoraproject.org/wiki/Changes/ParallelInstallableDebuginfo
>> I thought "we could reverse the symlink direction from debug file to
>> build-id file)"
>> sensible (but I understand it is not easy to change this way).
> 
> 
>> If two packages share an identical image,
>> one package can borrow the image from the other,
>> then the storage space will be saved.
> 
>> So, having identical ID should be advantage,
>> but we actually see only disadvantage...
> 
> 
> 
> 
>>> To simplify things I think you could just always add the extra vdso
>>> .comment initialized to something like KERNELRELEASE. Which distros
>>> seem to update anyway to include their build number, so they wouldn't
>>> need to do anything special to "update the build salt".
>>>
> 
> That's what I was thinking, too.  Would that solve Fedora's problem?
> 

Yes, that seems reasonable.

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

* Re: [RFCv2 PATCH 0/3] Salted build ids via linker sections
  2018-05-07  6:28 ` Masahiro Yamada
@ 2018-05-14 21:02   ` Laura Abbott
  2018-05-17  8:37     ` Masahiro Yamada
  0 siblings, 1 reply; 13+ messages in thread
From: Laura Abbott @ 2018-05-14 21:02 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Andy Lutomirski, mjw, H . J . Lu, Linus Torvalds, X86 ML,
	Linux Kernel Mailing List, Nick Clifton, Cary Coutant,
	Linux Kbuild mailing list

On 05/06/2018 11:28 PM, Masahiro Yamada wrote:
> 2018-03-30 3:01 GMT+09:00 Laura Abbott <labbott@redhat.com>:
>> Hi,
>>
>> This is v2 of my proposal to allow unique build-ids in the kernel. from
>> last time:
>>
>> ""
>> 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 think you already know '--build-id=uuid' linker option.
> 
> Doesn't this solve your problem?
> 
> The disadvantage of this option is,
> we will lose reproducible building because  --build-id=uuid
> adds every time random salt.
> 
> The advantage is, the implementation is even simpler,
> and easier to migrate to --build-id-salt once it is supported
> in the future.
> 
> 

It could, theoretically. The reproducibility is nice though and
I'd like to keep the kernel close to matching what other packages
do though.

Thanks,
Laura

>> This v2 cleans up the naming to be consistent and also switches to a
>> config option vs. an environment variable. I've seen some sporadic
>> failures about missing the generated header so I think I'm still missing
>> a dependency somewhere.
> 
> Right.
> 
> There is no dependency between 'prepare' and 'scripts'
> in the top Makefile.
> Therefore, Kbuild can run them simultaneously,
> which would cause a race in parallel building.
> 
> 
> 
> 
> 
> 
> 
> 
>   I'm still mostly looking for feedback whether
>> this would be acceptable for merging or if we should just persue a
>> --build-id-salt in binutils.
>>
>> 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 +++
>>   init/Kconfig                          |  8 ++++++++
>>   scripts/.gitignore                    |  1 +
>>   scripts/Makefile                      |  2 +-
>>   scripts/build-salt.lds.S              |  5 +++++
>>   scripts/gensalt                       | 21 +++++++++++++++++++++
>>   scripts/link-vmlinux.sh               |  3 ++-
>>   8 files changed, 52 insertions(+), 4 deletions(-)
>>   create mode 100644 scripts/build-salt.lds.S
>>   create mode 100755 scripts/gensalt
>>
>> --
>> 2.16.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

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

* Re: [RFCv2 PATCH 0/3] Salted build ids via linker sections
  2018-05-14 21:02   ` Laura Abbott
@ 2018-05-17  8:37     ` Masahiro Yamada
  0 siblings, 0 replies; 13+ messages in thread
From: Masahiro Yamada @ 2018-05-17  8:37 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-15 6:02 GMT+09:00 Laura Abbott <labbott@redhat.com>:
> On 05/06/2018 11:28 PM, Masahiro Yamada wrote:
>>
>> 2018-03-30 3:01 GMT+09:00 Laura Abbott <labbott@redhat.com>:
>>>
>>> Hi,
>>>
>>> This is v2 of my proposal to allow unique build-ids in the kernel. from
>>> last time:
>>>
>>> ""
>>> 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 think you already know '--build-id=uuid' linker option.
>>
>> Doesn't this solve your problem?
>>
>> The disadvantage of this option is,
>> we will lose reproducible building because  --build-id=uuid
>> adds every time random salt.
>>
>> The advantage is, the implementation is even simpler,
>> and easier to migrate to --build-id-salt once it is supported
>> in the future.
>>
>>
>
> It could, theoretically. The reproducibility is nice though and
> I'd like to keep the kernel close to matching what other packages
> do though.

Okay.
I left some comments in v2
about improvement, and cause of
the sporadic build failures.

Will wait for v3.

-- 
Best Regards
Masahiro Yamada

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

end of thread, other threads:[~2018-05-17  8:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-29 18:01 [RFCv2 PATCH 0/3] Salted build ids via linker sections Laura Abbott
2018-03-29 18:01 ` [RFCv2 PATCH 1/3] kbuild: Introduce build-salt generated header Laura Abbott
2018-03-29 18:01 ` [RFCv2 PATCH 2/3] kbuild: Link with generated build-salt header Laura Abbott
2018-05-07  6:38   ` Masahiro Yamada
2018-03-29 18:01 ` [RFCv2 PATCH 3/3] x86/vdso: Add build salt to the vDSO Laura Abbott
2018-05-07  6:42   ` Masahiro Yamada
2018-03-30 12:40 ` [RFCv2 PATCH 0/3] Salted build ids via linker sections Mark Wielaard
2018-05-07  6:58   ` Masahiro Yamada
2018-05-08  2:49     ` Andy Lutomirski
2018-05-14 20:51       ` Laura Abbott
2018-05-07  6:28 ` Masahiro Yamada
2018-05-14 21:02   ` Laura Abbott
2018-05-17  8:37     ` Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.