Salted build ids via linker sections

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 >>):

Quoted 1 line

Quoted 1 line

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

Quoted 8 lines · kbuild test robot

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]

Quoted 10 lines

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 >>):

Quoted 1 line

Quoted 1 line

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

This seems to need the following.

cheers

Makefile +0 -0

index 73f0bb2c7a98..55a5725b6606 100644

Quoted 1 line · Michael Ellerman

kbuild test robot <[email protected]> writes:

Quoted 6 lines

Quoted 3 lines

All errors (new ones prefixed by >>):

Quoted 1 line

Quoted 1 line

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

This seems to need the following.

Makefile KBUILD_AFLAGS_KERNEL := · line 430

Quoted 12 lines

Makefile +1 -1

index 73f0bb2c7a98..55a5725b6606 100644
@@ -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.

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

Signed-off-by Laura Abbott <[email protected]>

---
 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%)

scripts/.gitignore +1 -0

index 0442c06eefcb..afd1de57d9c6 100644
@@ -13,3 +13,4 @@ asn1_compiler
 extract-cert
 sign-file
 insert-sys-cert
+module-common.lds

scripts/Makefile +0 -0

index 25ab143cbe14..631d9d1a71e4 100644

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 <[email protected]>

Signed-off-by Laura Abbott <[email protected]>

---
 include/asm-generic/vmlinux.lds.h | 6 ++++++
 init/Kconfig                      | 9 +++++++++
 scripts/module-common.lds.S       | 4 ++++
 3 files changed, 19 insertions(+)

include/asm-generic/vmlinux.lds.h +0 -0

index e373e2e10f6a..4af7e683aad2 100644

include/asm-generic/vmlinux.lds.h +0 -0

 /*
  * Default discarded sections.

init/Kconfig +0 -0

index d2b8b2ea097e..eb92ccfe4ecb 100644
@@ -1967,3 +1967,12 @@ config ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
 # <asm/syscall_wrapper.h>.
 config ARCH_HAS_SYSCALL_WRAPPER

scripts/module-common.lds.S +1 -1

 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

Hi.

2018-06-12 9:32 GMT+09:00 Laura Abbott <[email protected]>:

Quoted 25 lines

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 <[email protected]>

Signed-off-by Laura Abbott <[email protected]>

---
  include/asm-generic/vmlinux.lds.h | 6 ++++++
  init/Kconfig                      | 9 +++++++++
  scripts/module-common.lds.S       | 4 ++++
  3 files changed, 19 insertions(+)

include/asm-generic/vmlinux.lds.h +0 -0

index e373e2e10f6a..4af7e683aad2 100644

include/asm-generic/vmlinux.lds.h line 838

Quoted 9 lines

include/asm-generic/vmlinux.lds.h +6 -0

@@ -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?

Quoted 9 lines

include/asm-generic/vmlinux.lds.h +0 -0

  /*
   * Default discarded sections.

init/Kconfig +0 -0

index d2b8b2ea097e..eb92ccfe4ecb 100644
@@ -1967,3 +1967,12 @@ config ARCH_HAS_SYNC_CORE_BEFORE_USERMODE
  # <asm/syscall_wrapper.h>.
  config ARCH_HAS_SYSCALL_WRAPPER

init/Kconfig config ARCH_HAS_SYNC_CORE_BEFORE_USERMODE · line 1978

Quoted 10 lines

init/Kconfig +9 -0

         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.

scripts/module-common.lds.S line 8

Quoted 11 lines

scripts/module-common.lds.S +3 -0

index d61b9e8678e8..3c8410270ac1 100644
@@ -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.

Quoted 11 lines

scripts/module-common.lds.S +1 -1

  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

arch/x86/entry/vdso/vdso-note.S +3 -0

index 79a071e..7942317 100644
@@ -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

arch/x86/entry/vdso/vdso32/note.S +3 -0

index 9fd51f2..e78047d 100644
@@ -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

include/linux/build-salt.h new file +20 -0

new file mode 100644
index 0000000..66e87c9
@@ -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 */

init/Kconfig +10 -0

index d2b8b2e..54b5828 100644
@@ -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
 

init/version.c +3 -0

index bfb4e3f..ef4012e 100644
@@ -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;

scripts/mod/modpost.c +3 -0

index 1663fb1..dc6d714 100644
@@ -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");

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 <[email protected]>

---
 arch/x86/entry/vdso/vdso-layout.lds.S | 3 ++-
 arch/x86/kernel/vmlinux.lds.S         | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

Quoted 9 lines · 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 <[email protected]>

---
  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 <[email protected]>

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

Thanks,

Ingo

Quoted 21 lines · Laura Abbott

Quoted 10 lines

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 <[email protected]>

---
  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 <[email protected]>

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

Thanks,

Quoted 2 lines

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.

v4 takes Linus' suggestion of using linker fill to insert the build id.

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

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