[PATCHv4 0/3] Salted build ids via linker sections

[PATCHv4 0/3] Salted build ids via linker sections

[Laura Abbott]

Hi,

This is v4 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 to add in the salt while building. The
easiest proposal was to add an item in the linker script vs. linking in
an object since we need the salt to go in every module as well as the
kernel and vmlinux.
""

v4 takes Linus' suggestion of using linker fill to insert the build id.
This removes the need to use a generated header which makes things much
easier. One change is that because this section isn't .comment it won't
get stripped automatically. This is pretty small but I also know people
can be picky so I'm open to opinions or suggestions here.

Laura Abbott (3):
  scripts: Preprocess module-common.lds
  kbuild: Introduce build-salt linker section and config option
  x86: Add build salt to the vDSO and kernel linker scripts

 arch/x86/entry/vdso/vdso-layout.lds.S              | 3 ++-
 arch/x86/kernel/vmlinux.lds.S                      | 1 +
 include/asm-generic/vmlinux.lds.h                  | 6 ++++++
 init/Kconfig                                       | 9 +++++++++
 scripts/.gitignore                                 | 1 +
 scripts/Makefile                                   | 2 +-
 scripts/{module-common.lds => module-common.lds.S} | 4 ++++
 7 files changed, 24 insertions(+), 2 deletions(-)
 rename scripts/{module-common.lds => module-common.lds.S} (94%)

-- 
2.18.0.rc1

[PATCHv4 1/3] scripts: Preprocess module-common.lds

[Laura Abbott]

In preparation for some upcoming work, allow module-common.lds
to be run through the preprocessor.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 scripts/.gitignore                                 | 1 +
 scripts/Makefile                                   | 2 +-
 scripts/{module-common.lds => module-common.lds.S} | 0
 3 files changed, 2 insertions(+), 1 deletion(-)
 rename scripts/{module-common.lds => module-common.lds.S} (100%)

diff --git a/scripts/.gitignore b/scripts/.gitignore
index 0442c06eefcb..afd1de57d9c6 100644
--- a/scripts/.gitignore
+++ b/scripts/.gitignore
@@ -13,3 +13,4 @@ asn1_compiler
 extract-cert
 sign-file
 insert-sys-cert
+module-common.lds
diff --git a/scripts/Makefile b/scripts/Makefile
index 25ab143cbe14..631d9d1a71e4 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) module-common.lds
 
 # The following hostprogs-y programs are only build on demand
 hostprogs-y += unifdef
diff --git a/scripts/module-common.lds b/scripts/module-common.lds.S
similarity index 100%
rename from scripts/module-common.lds
rename to scripts/module-common.lds.S
-- 
2.18.0.rc1

[PATCHv4 2/3] kbuild: Introduce build-salt linker section and config option

[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 section with some data.

Introduce a macro to insert a linker section which will be filled
with a hex value. This will ensure the build id can be changed just via
a config option. Users who don't care about this can leave the
default value and strip the section.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 include/asm-generic/vmlinux.lds.h | 6 ++++++
 init/Kconfig                      | 9 +++++++++
 scripts/module-common.lds.S       | 4 ++++
 3 files changed, 19 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index e373e2e10f6a..4af7e683aad2 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -830,6 +830,12 @@
 #define PERCPU_DECRYPTED_SECTION
 #endif
 
+#define	BUILD_SALT							\
+	. = ALIGN(32);							\
+	.salt : AT(ADDR(.salt) - LOAD_OFFSET) {				\
+	  LONG(0xffaa5500);						\
+	  . = ALIGN(32);						\
+	} = CONFIG_BUILD_ID_SALT					\
 
 /*
  * Default discarded sections.
diff --git a/init/Kconfig b/init/Kconfig
index d2b8b2ea097e..eb92ccfe4ecb 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1967,3 +1967,12 @@ config ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
 # <asm/syscall_wrapper.h>.
 config ARCH_HAS_SYSCALL_WRAPPER
 	def_bool n
+
+config BUILD_ID_SALT
+	hex "Build ID Salt"
+	default 0x12345678
+	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 the default.
diff --git a/scripts/module-common.lds.S b/scripts/module-common.lds.S
index d61b9e8678e8..3c8410270ac1 100644
--- a/scripts/module-common.lds.S
+++ b/scripts/module-common.lds.S
@@ -3,6 +3,9 @@
  * Archs are free to supply their own linker scripts.  ld will
  * combine them automatically.
  */
+
+#include <asm-generic/vmlinux.lds.h>
+
 SECTIONS {
 	/DISCARD/ : {
 		*(.discard)
@@ -23,4 +26,5 @@ SECTIONS {
 	.init_array		0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) }
 
 	__jump_table		0 : ALIGN(8) { KEEP(*(__jump_table)) }
+	BUILD_SALT
 }
-- 
2.18.0.rc1

[PATCHv4 3/3] x86: Add build salt to the vDSO and kernel linker scripts

[Laura Abbott]

Both the kernel and the vDSO need to have unique build ids.
Insert the build salt section to make the build ids unique.

Signed-off-by: Laura Abbott <labbott@redhat.com>
---
 arch/x86/entry/vdso/vdso-layout.lds.S | 3 ++-
 arch/x86/kernel/vmlinux.lds.S         | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/vdso/vdso-layout.lds.S b/arch/x86/entry/vdso/vdso-layout.lds.S
index acfd5ba7d943..a331c1d41360 100644
--- a/arch/x86/entry/vdso/vdso-layout.lds.S
+++ b/arch/x86/entry/vdso/vdso-layout.lds.S
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #include <asm/vdso.h>
-
+#include <asm-generic/vmlinux.lds.h>
 /*
  * Linker script for vDSO.  This is an ELF shared object prelinked to
  * its virtual address, and with only one read-only segment.
@@ -74,6 +74,7 @@ SECTIONS
 	.fake_shstrtab	: { *(.fake_shstrtab) }		:text
 
 
+	BUILD_SALT
 	.note		: { *(.note.*) }		:text	:note
 
 	.eh_frame_hdr	: { *(.eh_frame_hdr) }		:text	:eh_frame_hdr
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 5e1458f609a1..b61c33fa2617 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -172,6 +172,7 @@ SECTIONS
 		_edata = .;
 	} :data
 
+	BUILD_SALT
 	BUG_TABLE
 
 	ORC_UNWIND_TABLE
-- 
2.18.0.rc1

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

[Linus Torvalds]

On Mon, Jun 11, 2018 at 5:32 PM Laura Abbott <labbott@redhat.com> wrote:
>
> v4 takes Linus' suggestion of using linker fill to insert the build id.

Heh. I was hoping somebody would come up with a way to avoid my crazy
"put a LONG into an aligned section to cause alignment filling" thing.

Because somebody will ask "what the heck is the meaning of that
LONG(0xffaa5500) thing"?

But since I couldn't come up with anything better myself, I guess I
can't complain.

> This is pretty small but I also know people can be picky so I'm open to opinions or suggestions here.

I'm not sure anybody will care about 32 bytes. Although maybe it could
be made to be just 8 bytes (four bytes to force padding, four bytes of
padding) and put somewhere where you'll have more padding anyway due
to being followed by a PAGE_ALIGNED section or something?

I don't think we care.

Anyway, I'll pull this if somebody sends it to me, it seems to be
fairly minimally invasive and apparently fixes an issue for distros.

                Linus

Re: [PATCHv4 1/3] scripts: Preprocess module-common.lds

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

Hi Laura,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Laura-Abbott/scripts-Preprocess-module-common-lds/20180612-083632
config: i386-randconfig-a1-201823 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> ld: cannot open linker script file scripts/module-common.lds: No such file or directory

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28549 bytes --]

Re: [PATCHv4 1/3] scripts: Preprocess module-common.lds

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

Hi Laura,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17 next-20180608]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Laura-Abbott/scripts-Preprocess-module-common-lds/20180612-083632
config: openrisc-or1ksim_defconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=openrisc 

All errors (new ones prefixed by >>):

>> or1k-linux-ld: cannot open linker script file scripts/module-common.lds: No such file or directory

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 7574 bytes --]

Re: [PATCHv4 1/3] scripts: Preprocess module-common.lds

[Michael Ellerman]

kbuild test robot <lkp@intel.com> writes:

> Hi Laura,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17 next-20180608]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Laura-Abbott/scripts-Preprocess-module-common-lds/20180612-083632
> config: i386-randconfig-a1-201823 (attached as .config)
> compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
>
> All errors (new ones prefixed by >>):
>
>>> ld: cannot open linker script file scripts/module-common.lds: No such file or directory

This seems to need the following.

cheers


diff --git a/Makefile b/Makefile
index 73f0bb2c7a98..55a5725b6606 100644
--- a/Makefile
+++ b/Makefile
@@ -425,7 +425,7 @@ 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 $(objtree)/scripts/module-common.lds
 LDFLAGS :=
 GCC_PLUGINS_CFLAGS :=
 

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

[Michael Ellerman]

Laura Abbott <labbott@redhat.com> writes:
> Hi,
>
> This is v4 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 to add in the salt while building. The
> easiest proposal was to add an item in the linker script vs. linking in
> an object since we need the salt to go in every module as well as the
> kernel and vmlinux.
> ""
>
> v4 takes Linus' suggestion of using linker fill to insert the build id.
> This removes the need to use a generated header which makes things much
> easier. One change is that because this section isn't .comment it won't
> get stripped automatically. This is pretty small but I also know people
> can be picky so I'm open to opinions or suggestions here.
>
> Laura Abbott (3):
>   scripts: Preprocess module-common.lds
>   kbuild: Introduce build-salt linker section and config option
>   x86: Add build salt to the vDSO and kernel linker scripts

Hi Laura,

Here's a patch to get it working on powerpc. Seems to work as expected.

cheers


From fc5e22e4873956f9328e401ee5dd2835f4884db9 Mon Sep 17 00:00:00 2001
From: Michael Ellerman <mpe@ellerman.id.au>
Date: Tue, 12 Jun 2018 14:52:34 +1000
Subject: [PATCH] powerpc: Add support for BUILD_SALT in kernel, modules & VDSO

This patch adds support for BUILD_SALT in the kernel, modules and
VDSO. See the commit that adds BUILD_SALT for more info.

Kernel:
  0:mon> d c000000001340840
  c000000001340840 0055aaff12345678 1234567812345678  |.U...4Vx.4Vx.4Vx|
  c000000001340850 1234567812345678 1234567812345678  |.4Vx.4Vx.4Vx.4Vx|

Module:

  $ cat/sys/module/kvm/sections/.salt
  0xd0000000064385e8
  ...
  0:mon> d d0000000064385e8
  d0000000064385e8 0055aaff12345678 1234567812345678  |.U...4Vx.4Vx.4Vx|
  d0000000064385f8 1234567812345678 1234567812345678  |.4Vx.4Vx.4Vx.4Vx|

vdso:
  (gdb) x/4xw (0x7ffff7f80000 + 0x4a0)
  0x7ffff7f804a0:	0xffaa5500	0x78563412	0x78563412	0x78563412

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/vdso32/vdso32.lds.S | 2 ++
 arch/powerpc/kernel/vdso64/vdso64.lds.S | 2 ++
 arch/powerpc/kernel/vmlinux.lds.S       | 1 +
 3 files changed, 5 insertions(+)

diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 099a6db14e67..c06a12607777 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -4,6 +4,7 @@
  * library
  */
 #include <asm/vdso.h>
+#include <asm-generic/vmlinux.lds.h>
 
 #ifdef __LITTLE_ENDIAN__
 OUTPUT_FORMAT("elf32-powerpcle", "elf32-powerpcle", "elf32-powerpcle")
@@ -25,6 +26,7 @@ SECTIONS
 	.gnu.version_d 	: { *(.gnu.version_d) }
 	.gnu.version_r 	: { *(.gnu.version_r) }
 
+	BUILD_SALT
 	.note		: { *(.note.*) }		:text	:note
 
 	. = ALIGN(16);
diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
index 256fb9720298..ace69258446a 100644
--- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
+++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
@@ -4,6 +4,7 @@
  * library
  */
 #include <asm/vdso.h>
+#include <asm-generic/vmlinux.lds.h>
 
 #ifdef __LITTLE_ENDIAN__
 OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle")
@@ -25,6 +26,7 @@ SECTIONS
 	.gnu.version_d	: { *(.gnu.version_d) }
 	.gnu.version_r	: { *(.gnu.version_r) }
 
+	BUILD_SALT
 	.note		: { *(.note.*) }		:text	:note
 
 	. = ALIGN(16);
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 5baac79df97e..59635369ceea 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -348,6 +348,7 @@ SECTIONS
 	}
 
 	BUG_TABLE
+	BUILD_SALT
 
 	. = ALIGN(PAGE_SIZE);
 	_edata  =  .;
-- 
2.14.1

Re: [PATCHv4 1/3] scripts: Preprocess module-common.lds

[Laura Abbott]

On 06/11/2018 11:03 PM, Michael Ellerman wrote:
> kbuild test robot <lkp@intel.com> writes:
> 
>> Hi Laura,
>>
>> I love your patch! Yet something to improve:
>>
>> [auto build test ERROR on linus/master]
>> [also build test ERROR on v4.17 next-20180608]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>
>> url:    https://github.com/0day-ci/linux/commits/Laura-Abbott/scripts-Preprocess-module-common-lds/20180612-083632
>> config: i386-randconfig-a1-201823 (attached as .config)
>> compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
>> reproduce:
>>          # save the attached .config to linux build tree
>>          make ARCH=i386
>>
>> All errors (new ones prefixed by >>):
>>
>>>> ld: cannot open linker script file scripts/module-common.lds: No such file or directory
> 
> This seems to need the following.
> 
> cheers
> 
> 
> diff --git a/Makefile b/Makefile
> index 73f0bb2c7a98..55a5725b6606 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -425,7 +425,7 @@ 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 $(objtree)/scripts/module-common.lds
>   LDFLAGS :=
>   GCC_PLUGINS_CFLAGS :=
>   
> 

Thanks for pointing that out. I think I missed that when refactoring.
I'll fix that in the next version plus adding your powerpc patch.

Thanks,
Laura

Re: [PATCHv4 1/3] scripts: Preprocess module-common.lds

[Masahiro Yamada]

2018-06-12 9:32 GMT+09:00 Laura Abbott <labbott@redhat.com>:
>
> In preparation for some upcoming work, allow module-common.lds
> to be run through the preprocessor.
>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
>  scripts/.gitignore                                 | 1 +
>  scripts/Makefile                                   | 2 +-
>  scripts/{module-common.lds => module-common.lds.S} | 0
>  3 files changed, 2 insertions(+), 1 deletion(-)
>  rename scripts/{module-common.lds => module-common.lds.S} (100%)
>
> diff --git a/scripts/.gitignore b/scripts/.gitignore
> index 0442c06eefcb..afd1de57d9c6 100644
> --- a/scripts/.gitignore
> +++ b/scripts/.gitignore
> @@ -13,3 +13,4 @@ asn1_compiler
>  extract-cert
>  sign-file
>  insert-sys-cert
> +module-common.lds
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 25ab143cbe14..631d9d1a71e4 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) module-common.lds


You do not need to generate module-common.lds all the time.
It is necessary only when the module feature is enabled.

You can do like this:

extra-$(CONFIG_MODULES)  +=  module-common.lds






-- 
Best Regards
Masahiro Yamada

Re: [PATCHv4 2/3] kbuild: Introduce build-salt linker section and config option

[Masahiro Yamada]

Hi.


2018-06-12 9:32 GMT+09:00 Laura Abbott <labbott@redhat.com>:
>
> 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 section with some data.
>
> Introduce a macro to insert a linker section which will be filled
> with a hex value. This will ensure the build id can be changed just via
> a config option. Users who don't care about this can leave the
> default value and strip the section.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
>  include/asm-generic/vmlinux.lds.h | 6 ++++++
>  init/Kconfig                      | 9 +++++++++
>  scripts/module-common.lds.S       | 4 ++++
>  3 files changed, 19 insertions(+)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index e373e2e10f6a..4af7e683aad2 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -830,6 +830,12 @@
>  #define PERCPU_DECRYPTED_SECTION
>  #endif
>
> +#define        BUILD_SALT                                                      \
> +       . = ALIGN(32);                                                  \
> +       .salt : AT(ADDR(.salt) - LOAD_OFFSET) {                         \
> +         LONG(0xffaa5500);                                             \
> +         . = ALIGN(32);                                                \
> +       } = CONFIG_BUILD_ID_SALT                                        \


What is 0xffaa5500 ?

Is it another salt in addition to CONFIG_BUILD_ID_SALT ?

Or, does it have a special meaning?



>  /*
>   * Default discarded sections.
> diff --git a/init/Kconfig b/init/Kconfig
> index d2b8b2ea097e..eb92ccfe4ecb 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1967,3 +1967,12 @@ config ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
>  # <asm/syscall_wrapper.h>.
>  config ARCH_HAS_SYSCALL_WRAPPER
>         def_bool n
> +
> +config BUILD_ID_SALT
> +       hex "Build ID Salt"
> +       default 0x12345678
> +       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 the default.


If you run "make menuconfig",
you will notice this looks so strange;
"Build ID Salt" is displayed in the top level menu.






> diff --git a/scripts/module-common.lds.S b/scripts/module-common.lds.S
> index d61b9e8678e8..3c8410270ac1 100644
> --- a/scripts/module-common.lds.S
> +++ b/scripts/module-common.lds.S
> @@ -3,6 +3,9 @@
>   * Archs are free to supply their own linker scripts.  ld will
>   * combine them automatically.
>   */
> +
> +#include <asm-generic/vmlinux.lds.h>
> +

You are pulling many macros in <asm-generic/vmlinux.lds.h>
into modules, VDSO, etc.


You also need to touch
arch/*/kernel/vmlinux.lds.S
of all architectures.

This is not so nice.




>  SECTIONS {
>         /DISCARD/ : {
>                 *(.discard)
> @@ -23,4 +26,5 @@ SECTIONS {
>         .init_array             0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) }
>
>         __jump_table            0 : ALIGN(8) { KEEP(*(__jump_table)) }
> +       BUILD_SALT
>  }
> --
> 2.18.0.rc1



I was playing with a different approach.

Instead of touching linker scripts around,
how about putting the salt into an elfnote?


The compiler puts the build ID
into the '.note.gnu.build-id' section.

So, I guess it is sensible to put
the salt into '.note.*' section as well.


I attached my trial below:


 - I moved 'config BUILD_SALT' to a better location
   so that it will be displayed in the "General setup" menu.

 - I added 'range 0 0xffffffff' to avoid warnings.

 - I renamed the config symbol to CONFIG_BUILD_SALT
   and change the default to 0x0.
   Of course, this is just bike-shed things, though..

 - It looks like 'owner=Linux, type_id=0'
   is already used in VDSO.
   https://github.com/torvalds/linux/blob/v4.17/arch/arm64/kernel/vdso/note.S#L26

   I chose 0x100 for the type id, but it could be a different value


diff --git a/arch/x86/entry/vdso/vdso-note.S b/arch/x86/entry/vdso/vdso-note.S
index 79a071e..7942317 100644
--- a/arch/x86/entry/vdso/vdso-note.S
+++ b/arch/x86/entry/vdso/vdso-note.S
@@ -3,6 +3,7 @@
  * Here we can supply some information useful to userland.
  */

+#include <linux/build-salt.h>
 #include <linux/uts.h>
 #include <linux/version.h>
 #include <linux/elfnote.h>
@@ -10,3 +11,5 @@
 ELFNOTE_START(Linux, 0, "a")
        .long LINUX_VERSION_CODE
 ELFNOTE_END
+
+BUILD_SALT
diff --git a/arch/x86/entry/vdso/vdso32/note.S
b/arch/x86/entry/vdso/vdso32/note.S
index 9fd51f2..e78047d 100644
--- a/arch/x86/entry/vdso/vdso32/note.S
+++ b/arch/x86/entry/vdso/vdso32/note.S
@@ -4,6 +4,7 @@
  * Here we can supply some information useful to userland.
  */

+#include <linux/build-salt.h>
 #include <linux/version.h>
 #include <linux/elfnote.h>

@@ -14,6 +15,8 @@ ELFNOTE_START(Linux, 0, "a")
        .long LINUX_VERSION_CODE
 ELFNOTE_END

+BUILD_SALT
+
 #ifdef CONFIG_XEN
 /*
  * Add a special note telling glibc's dynamic linker a fake hardware
diff --git a/include/linux/build-salt.h b/include/linux/build-salt.h
new file mode 100644
index 0000000..66e87c9
--- /dev/null
+++ b/include/linux/build-salt.h
@@ -0,0 +1,20 @@
+#ifndef __BUILD_SALT_H
+#define __BUILD_SALT_H
+
+#include <linux/elfnote.h>
+
+#define LINUX_ELFNOTE_BUILD_SALT       0x100
+
+#ifdef __ASSEMBLER__
+
+#define BUILD_SALT \
+       ELFNOTE(Linux, LINUX_ELFNOTE_BUILD_SALT, .long CONFIG_BUILD_SALT)
+
+#else
+
+#define BUILD_SALT \
+       ELFNOTE32("Linux", LINUX_ELFNOTE_BUILD_SALT, CONFIG_BUILD_SALT)
+
+#endif
+
+#endif /* __BUILD_SALT_H */
diff --git a/init/Kconfig b/init/Kconfig
index d2b8b2e..54b5828 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -92,6 +92,16 @@ config LOCALVERSION_AUTO

          which is done within the script "scripts/setlocalversion".)

+config BUILD_SALT
+       hex "Build ID Salt"
+       default 0x0
+       range 0 0xffffffff
+       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 the default.
+
 config HAVE_KERNEL_GZIP
        bool

diff --git a/init/version.c b/init/version.c
index bfb4e3f..ef4012e 100644
--- a/init/version.c
+++ b/init/version.c
@@ -7,6 +7,7 @@
  */

 #include <generated/compile.h>
+#include <linux/build-salt.h>
 #include <linux/export.h>
 #include <linux/uts.h>
 #include <linux/utsname.h>
@@ -49,3 +50,5 @@ const char linux_proc_banner[] =
        "%s version %s"
        " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
        " (" LINUX_COMPILER ") %s\n";
+
+BUILD_SALT;
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 1663fb1..dc6d714 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2125,10 +2125,13 @@ static int check_modname_len(struct module *mod)
  **/
 static void add_header(struct buffer *b, struct module *mod)
 {
+       buf_printf(b, "#include <linux/build-salt.h>\n");
        buf_printf(b, "#include <linux/module.h>\n");
        buf_printf(b, "#include <linux/vermagic.h>\n");
        buf_printf(b, "#include <linux/compiler.h>\n");
        buf_printf(b, "\n");
+       buf_printf(b, "BUILD_SALT;\n");
+       buf_printf(b, "\n");
        buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
        buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
        buf_printf(b, "\n");




-- 
Best Regards
Masahiro Yamada

Re: [PATCHv4 1/3] scripts: Preprocess module-common.lds

[Michael Ellerman]

Laura Abbott <labbott@redhat.com> writes:
> On 06/11/2018 11:03 PM, Michael Ellerman wrote:
>> kbuild test robot <lkp@intel.com> writes:
...
>>> All errors (new ones prefixed by >>):
>>>
>>>>> ld: cannot open linker script file scripts/module-common.lds: No such file or directory
>> 
>> This seems to need the following.
>> 
>> diff --git a/Makefile b/Makefile
>> index 73f0bb2c7a98..55a5725b6606 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -425,7 +425,7 @@ 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 $(objtree)/scripts/module-common.lds
>>   LDFLAGS :=
>>   GCC_PLUGINS_CFLAGS :=
>
> Thanks for pointing that out. I think I missed that when refactoring.
> I'll fix that in the next version plus adding your powerpc patch.

Thanks.

cheers

Re: [PATCHv4 2/3] kbuild: Introduce build-salt linker section and config option

[Laura Abbott]

On 06/12/2018 11:06 PM, Masahiro Yamada wrote:
> Hi.
> 
> 
> 2018-06-12 9:32 GMT+09:00 Laura Abbott <labbott@redhat.com>:
>>
>> 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 section with some data.
>>
>> Introduce a macro to insert a linker section which will be filled
>> with a hex value. This will ensure the build id can be changed just via
>> a config option. Users who don't care about this can leave the
>> default value and strip the section.
>>
>> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>>   include/asm-generic/vmlinux.lds.h | 6 ++++++
>>   init/Kconfig                      | 9 +++++++++
>>   scripts/module-common.lds.S       | 4 ++++
>>   3 files changed, 19 insertions(+)
>>
>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>> index e373e2e10f6a..4af7e683aad2 100644
>> --- a/include/asm-generic/vmlinux.lds.h
>> +++ b/include/asm-generic/vmlinux.lds.h
>> @@ -830,6 +830,12 @@
>>   #define PERCPU_DECRYPTED_SECTION
>>   #endif
>>
>> +#define        BUILD_SALT                                                      \
>> +       . = ALIGN(32);                                                  \
>> +       .salt : AT(ADDR(.salt) - LOAD_OFFSET) {                         \
>> +         LONG(0xffaa5500);                                             \
>> +         . = ALIGN(32);                                                \
>> +       } = CONFIG_BUILD_ID_SALT                                        \
> 
> 
> What is 0xffaa5500 ?
> 
> Is it another salt in addition to CONFIG_BUILD_ID_SALT ?
> 
> Or, does it have a special meaning?
> 
> 
> 
>>   /*
>>    * Default discarded sections.
>> diff --git a/init/Kconfig b/init/Kconfig
>> index d2b8b2ea097e..eb92ccfe4ecb 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1967,3 +1967,12 @@ config ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
>>   # <asm/syscall_wrapper.h>.
>>   config ARCH_HAS_SYSCALL_WRAPPER
>>          def_bool n
>> +
>> +config BUILD_ID_SALT
>> +       hex "Build ID Salt"
>> +       default 0x12345678
>> +       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 the default.
> 
> 
> If you run "make menuconfig",
> you will notice this looks so strange;
> "Build ID Salt" is displayed in the top level menu.
> 
> 
> 
> 
> 
> 
>> diff --git a/scripts/module-common.lds.S b/scripts/module-common.lds.S
>> index d61b9e8678e8..3c8410270ac1 100644
>> --- a/scripts/module-common.lds.S
>> +++ b/scripts/module-common.lds.S
>> @@ -3,6 +3,9 @@
>>    * Archs are free to supply their own linker scripts.  ld will
>>    * combine them automatically.
>>    */
>> +
>> +#include <asm-generic/vmlinux.lds.h>
>> +
> 
> You are pulling many macros in <asm-generic/vmlinux.lds.h>
> into modules, VDSO, etc.
> 
> 
> You also need to touch
> arch/*/kernel/vmlinux.lds.S
> of all architectures.
> 
> This is not so nice.
> 
> 
> 
> 
>>   SECTIONS {
>>          /DISCARD/ : {
>>                  *(.discard)
>> @@ -23,4 +26,5 @@ SECTIONS {
>>          .init_array             0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) }
>>
>>          __jump_table            0 : ALIGN(8) { KEEP(*(__jump_table)) }
>> +       BUILD_SALT
>>   }
>> --
>> 2.18.0.rc1
> 
> 
> 
> I was playing with a different approach.
> 
> Instead of touching linker scripts around,
> how about putting the salt into an elfnote?
> 
> 
> The compiler puts the build ID
> into the '.note.gnu.build-id' section.
> 
> So, I guess it is sensible to put
> the salt into '.note.*' section as well.
> 
>
> I attached my trial below:
> 
> 
>   - I moved 'config BUILD_SALT' to a better location
>     so that it will be displayed in the "General setup" menu.
> 
>   - I added 'range 0 0xffffffff' to avoid warnings.
> 
>   - I renamed the config symbol to CONFIG_BUILD_SALT
>     and change the default to 0x0.
>     Of course, this is just bike-shed things, though..
> 
>   - It looks like 'owner=Linux, type_id=0'
>     is already used in VDSO.
>     https://github.com/torvalds/linux/blob/v4.17/arch/arm64/kernel/vdso/note.S#L26
> 
>     I chose 0x100 for the type id, but it could be a different value
>


I like this approach. It covers what was suggested without having
to add extra infrastructure. With this approach, we could go
back to the original suggestion of making the salt a string which
is easier to work with from a distro perspective (can just use the
unique version string for each package vs. having to calculate a hash)

Thanks,
Laura
  
> 
> diff --git a/arch/x86/entry/vdso/vdso-note.S b/arch/x86/entry/vdso/vdso-note.S
> index 79a071e..7942317 100644
> --- a/arch/x86/entry/vdso/vdso-note.S
> +++ b/arch/x86/entry/vdso/vdso-note.S
> @@ -3,6 +3,7 @@
>    * Here we can supply some information useful to userland.
>    */
> 
> +#include <linux/build-salt.h>
>   #include <linux/uts.h>
>   #include <linux/version.h>
>   #include <linux/elfnote.h>
> @@ -10,3 +11,5 @@
>   ELFNOTE_START(Linux, 0, "a")
>          .long LINUX_VERSION_CODE
>   ELFNOTE_END
> +
> +BUILD_SALT
> diff --git a/arch/x86/entry/vdso/vdso32/note.S
> b/arch/x86/entry/vdso/vdso32/note.S
> index 9fd51f2..e78047d 100644
> --- a/arch/x86/entry/vdso/vdso32/note.S
> +++ b/arch/x86/entry/vdso/vdso32/note.S
> @@ -4,6 +4,7 @@
>    * Here we can supply some information useful to userland.
>    */
> 
> +#include <linux/build-salt.h>
>   #include <linux/version.h>
>   #include <linux/elfnote.h>
> 
> @@ -14,6 +15,8 @@ ELFNOTE_START(Linux, 0, "a")
>          .long LINUX_VERSION_CODE
>   ELFNOTE_END
> 
> +BUILD_SALT
> +
>   #ifdef CONFIG_XEN
>   /*
>    * Add a special note telling glibc's dynamic linker a fake hardware
> diff --git a/include/linux/build-salt.h b/include/linux/build-salt.h
> new file mode 100644
> index 0000000..66e87c9
> --- /dev/null
> +++ b/include/linux/build-salt.h
> @@ -0,0 +1,20 @@
> +#ifndef __BUILD_SALT_H
> +#define __BUILD_SALT_H
> +
> +#include <linux/elfnote.h>
> +
> +#define LINUX_ELFNOTE_BUILD_SALT       0x100
> +
> +#ifdef __ASSEMBLER__
> +
> +#define BUILD_SALT \
> +       ELFNOTE(Linux, LINUX_ELFNOTE_BUILD_SALT, .long CONFIG_BUILD_SALT)
> +
> +#else
> +
> +#define BUILD_SALT \
> +       ELFNOTE32("Linux", LINUX_ELFNOTE_BUILD_SALT, CONFIG_BUILD_SALT)
> +
> +#endif
> +
> +#endif /* __BUILD_SALT_H */
> diff --git a/init/Kconfig b/init/Kconfig
> index d2b8b2e..54b5828 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -92,6 +92,16 @@ config LOCALVERSION_AUTO
> 
>            which is done within the script "scripts/setlocalversion".)
> 
> +config BUILD_SALT
> +       hex "Build ID Salt"
> +       default 0x0
> +       range 0 0xffffffff
> +       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 the default.
> +
>   config HAVE_KERNEL_GZIP
>          bool
> 
> diff --git a/init/version.c b/init/version.c
> index bfb4e3f..ef4012e 100644
> --- a/init/version.c
> +++ b/init/version.c
> @@ -7,6 +7,7 @@
>    */
> 
>   #include <generated/compile.h>
> +#include <linux/build-salt.h>
>   #include <linux/export.h>
>   #include <linux/uts.h>
>   #include <linux/utsname.h>
> @@ -49,3 +50,5 @@ const char linux_proc_banner[] =
>          "%s version %s"
>          " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
>          " (" LINUX_COMPILER ") %s\n";
> +
> +BUILD_SALT;
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 1663fb1..dc6d714 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2125,10 +2125,13 @@ static int check_modname_len(struct module *mod)
>    **/
>   static void add_header(struct buffer *b, struct module *mod)
>   {
> +       buf_printf(b, "#include <linux/build-salt.h>\n");
>          buf_printf(b, "#include <linux/module.h>\n");
>          buf_printf(b, "#include <linux/vermagic.h>\n");
>          buf_printf(b, "#include <linux/compiler.h>\n");
>          buf_printf(b, "\n");
> +       buf_printf(b, "BUILD_SALT;\n");
> +       buf_printf(b, "\n");
>          buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
>          buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
>          buf_printf(b, "\n");
> 
> 
> 
> 

Re: [PATCHv4 3/3] x86: Add build salt to the vDSO and kernel linker scripts

[Ingo Molnar]

* Laura Abbott <labbott@redhat.com> wrote:

> 
> Both the kernel and the vDSO need to have unique build ids.
> Insert the build salt section to make the build ids unique.
> 
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
>  arch/x86/entry/vdso/vdso-layout.lds.S | 3 ++-
>  arch/x86/kernel/vmlinux.lds.S         | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)

Acked-by: Ingo Molnar <mingo@kernel.org>

What is the upstream merge plan for this series? kbuild tree?

Thanks,

	Ingo

Re: [PATCHv4 3/3] x86: Add build salt to the vDSO and kernel linker scripts

[Laura Abbott]

On 06/21/2018 05:43 AM, Ingo Molnar wrote:
> 
> * Laura Abbott <labbott@redhat.com> wrote:
> 
>>
>> Both the kernel and the vDSO need to have unique build ids.
>> Insert the build salt section to make the build ids unique.
>>
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>>   arch/x86/entry/vdso/vdso-layout.lds.S | 3 ++-
>>   arch/x86/kernel/vmlinux.lds.S         | 1 +
>>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> Acked-by: Ingo Molnar <mingo@kernel.org>
> 
> What is the upstream merge plan for this series? kbuild tree?
> 
> Thanks,
> 
> 	Ingo
> 

There was an alternate proposal that requires slightly fewer
changes that needs to be cleaned up and submitted. I do
think the plan is for the series to eventually go through
the kbuild tree.

Thanks,
Laura

Re: [PATCHv4 3/3] x86: Add build salt to the vDSO and kernel linker scripts

[Masahiro Yamada]

Hi.


2018-06-22 0:58 GMT+09:00 Laura Abbott <labbott@redhat.com>:
> On 06/21/2018 05:43 AM, Ingo Molnar wrote:
>>
>>
>> * Laura Abbott <labbott@redhat.com> wrote:
>>
>>>
>>> Both the kernel and the vDSO need to have unique build ids.
>>> Insert the build salt section to make the build ids unique.
>>>
>>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>>> ---
>>>   arch/x86/entry/vdso/vdso-layout.lds.S | 3 ++-
>>>   arch/x86/kernel/vmlinux.lds.S         | 1 +
>>>   2 files changed, 3 insertions(+), 1 deletion(-)
>>
>>
>> Acked-by: Ingo Molnar <mingo@kernel.org>
>>
>> What is the upstream merge plan for this series? kbuild tree?
>>
>> Thanks,
>>
>>         Ingo
>>
>
> There was an alternate proposal that requires slightly fewer
> changes that needs to be cleaned up and submitted. I do
> think the plan is for the series to eventually go through
> the kbuild tree.


Yes.  V5 is welcome.

Thanks.





-- 
Best Regards
Masahiro Yamada