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

From: Laura Abbott
Date: Thu Jun 14 2018 - 17:39:19 EST


On 06/12/2018 11:06 PM, Masahiro Yamada wrote:
Hi.


2018-06-12 9:32 GMT+09:00 Laura Abbott <labbott@xxxxxxxxxx>:

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@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Laura Abbott <labbott@xxxxxxxxxx>
---
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");