[PATCHv5 0/4] Salted build ids via ELF notes

Hi,

This is v5 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.
""

v5 uses the approach suggested by Masahiro Yamada which uses the
existing ELF note macro to more easily add the salt (vs previous
approaches which tried to adjust via linker section).

If arch maintainers are okay, I'd like acks for this so this can go
through the kbuild tree.

Thanks,
Laura

Laura Abbott (4):
  kbuild: Add build salt to the kernel and modules
  x86: Add build salt to the vDSO
  powerpc: Add build salt to the vDSO
  arm64: Add build salt to the vDSO

 arch/arm64/kernel/vdso/note.S     |  3 +++
 arch/powerpc/kernel/vdso32/note.S |  3 +++
 arch/x86/entry/vdso/vdso-note.S   |  3 +++
 arch/x86/entry/vdso/vdso32/note.S |  3 +++
 include/linux/build-salt.h        | 20 ++++++++++++++++++++
 init/Kconfig                      |  9 +++++++++
 init/version.c                    |  3 +++
 scripts/mod/modpost.c             |  3 +++
 8 files changed, 47 insertions(+)
 create mode 100644 include/linux/build-salt.h

-- 
2.17.1

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.

Add an ELF note to both the kernel and module which contains some data based
off of a config option.

Signed-off-by: Masahiro Yamada <redacted>
Signed-off-by: Laura Abbott <redacted>
---
v5: I used S-o-b here since the majority of the code was written
already. Please feel free to change the tag if you think it's not
appropriate. I also tweaked this to take an ascii string instead of just
a hex value since this makes things much easier on the distribution
side.
---
 include/linux/build-salt.h | 20 ++++++++++++++++++++
 init/Kconfig               |  9 +++++++++
 init/version.c             |  3 +++
 scripts/mod/modpost.c      |  3 +++
 4 files changed, 35 insertions(+)
 create mode 100644 include/linux/build-salt.h

diff --git a/include/linux/build-salt.h b/include/linux/build-salt.h
new file mode 100644
index 000000000000..bb007bd05e7a
--- /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, .asciz 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 041f3a022122..8de789f40db9 100644
--- a/init/Kconfig
+++ b/init/Kconfig

@@ -107,6 +107,15 @@ config LOCALVERSION_AUTO
 
 	  which is done within the script "scripts/setlocalversion".)
 
+config BUILD_SALT
+       string "Build ID Salt"
+       default "Linux"
+       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 bfb4e3f4955e..ef4012ec4375 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 1663fb19343a..dc6d714e4dcb 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");

-- 
2.17.1

The vDSO needs to have a unique build id in a similar manner
to the kernel and modules. Use the build salt macro.

Signed-off-by: Laura Abbott <redacted>
---
v5: Switched to using the single line BUILD_SALT macro
---
 arch/x86/entry/vdso/vdso-note.S   | 3 +++
 arch/x86/entry/vdso/vdso32/note.S | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/arch/x86/entry/vdso/vdso-note.S b/arch/x86/entry/vdso/vdso-note.S
index 79a071e4357e..79423170118f 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 9fd51f206314..e78047d119f6 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

-- 
2.17.1

The vDSO needs to have a unique build id in a similar manner
to the kernel and modules. Use the build salt macro.

Signed-off-by: Laura Abbott <redacted>
---
v5: New approach with the BUILD_SALT macro
---
 arch/powerpc/kernel/vdso32/note.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/kernel/vdso32/note.S b/arch/powerpc/kernel/vdso32/note.S
index d4b5be4f3d5f..ec1d05265c75 100644
--- a/arch/powerpc/kernel/vdso32/note.S
+++ b/arch/powerpc/kernel/vdso32/note.S

@@ -5,6 +5,7 @@
 
 #include <linux/uts.h>
 #include <linux/version.h>
+#include <linux/build-salt.h>
 
 #define ASM_ELF_NOTE_BEGIN(name, flags, vendor, type)			      \
 	.section name, flags;						      \

@@ -23,3 +24,5 @@
 	ASM_ELF_NOTE_BEGIN(".note.kernel-version", "a", UTS_SYSNAME, 0)
 	.long LINUX_VERSION_CODE
 	ASM_ELF_NOTE_END
+
+BUILD_SALT;

-- 
2.17.1

The vDSO needs to have a unique build id in a similar manner
to the kernel and modules. Use the build salt macro.

Signed-off-by: Laura Abbott <redacted>
---
v5: I was previously focused on x86 only but since powerpc gave a patch,
I figured I would do arm64 since the changes were also fairly simple.
---
 arch/arm64/kernel/vdso/note.S | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/kernel/vdso/note.S b/arch/arm64/kernel/vdso/note.S
index b82c85e5d972..2c429dfd3f45 100644
--- a/arch/arm64/kernel/vdso/note.S
+++ b/arch/arm64/kernel/vdso/note.S

@@ -22,7 +22,10 @@
 #include <linux/uts.h>
 #include <linux/version.h>
 #include <linux/elfnote.h>
+#include <linux/build-salt.h>
 
 ELFNOTE_START(Linux, 0, "a")
 	.long LINUX_VERSION_CODE
 ELFNOTE_END
+
+BUILD_SALT;

-- 
2.17.1

Hi.

2018-07-04 8:34 GMT+09:00 Laura Abbott [off-list ref]:

I think this works, but
I prefer no-semicolon in assembly files.

For coding consistency,
I want ';' as statement delimiter in .c files.
But, only new line after each statement in .S files.

For example, in arch/x86/xen/xen-head.S
I see no semicolon after ELFNOTE().

I found this:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473k/dom1359731141352.html
It says ';' starts a comment line
although it is not the case of GAS.


Same for 3/4.



-- 
Best Regards
Masahiro Yamada

Hi.

Thanks for the update.


2018-07-04 8:34 GMT+09:00 Laura Abbott [off-list ref]:

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.

Add an ELF note to both the kernel and module which contains some data based
off of a config option.

Signed-off-by: Masahiro Yamada <redacted>
Signed-off-by: Laura Abbott <redacted>
---
v5: I used S-o-b here since the majority of the code was written
already.

I think Suggested-by is good enough.
S-o-b is appended as a patch is passed from people to people.

Anyway, this looks good except one bike-shed.

Please feel free to change the tag if you think it's not
appropriate. I also tweaked this to take an ascii string instead of just
a hex value since this makes things much easier on the distribution
side.
---

How about empty string ""
for default?




-- 
Best Regards
Masahiro Yamada

On Tue, Jul 03, 2018 at 04:34:30PM -0700, Laura Abbott wrote:

The vDSO needs to have a unique build id in a similar manner
to the kernel and modules. Use the build salt macro.

Signed-off-by: Laura Abbott <redacted>
---
v5: I was previously focused on x86 only but since powerpc gave a patch,
I figured I would do arm64 since the changes were also fairly simple.
---
 arch/arm64/kernel/vdso/note.S | 3 +++
 1 file changed, 3 insertions(+)

If you drop the trailing semicolon, then:

Acked-by: Will Deacon <redacted>

Will

On Tue, Jul 3, 2018 at 4:34 PM, Laura Abbott [off-list ref] wrote:

The vDSO needs to have a unique build id in a similar manner
to the kernel and modules. Use the build salt macro.

Looks good to me.  I have no idea whose tree these would go through.

On 07/03/2018 08:55 PM, Masahiro Yamada wrote:

Hi.

2018-07-04 8:34 GMT+09:00 Laura Abbott [off-list ref]:

quoted

The vDSO needs to have a unique build id in a similar manner
to the kernel and modules. Use the build salt macro.

Signed-off-by: Laura Abbott <redacted>
---
v5: I was previously focused on x86 only but since powerpc gave a patch,
I figured I would do arm64 since the changes were also fairly simple.
---
  arch/arm64/kernel/vdso/note.S | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/arch/arm64/kernel/vdso/note.S b/arch/arm64/kernel/vdso/note.S
index b82c85e5d972..2c429dfd3f45 100644
--- a/arch/arm64/kernel/vdso/note.S
+++ b/arch/arm64/kernel/vdso/note.S

@@ -22,7 +22,10 @@
  #include <linux/uts.h>
  #include <linux/version.h>
  #include <linux/elfnote.h>
+#include <linux/build-salt.h>

  ELFNOTE_START(Linux, 0, "a")
         .long LINUX_VERSION_CODE
  ELFNOTE_END
+
+BUILD_SALT;

I think this works, but
I prefer no-semicolon in assembly files.

For coding consistency,
I want ';' as statement delimiter in .c files.
But, only new line after each statement in .S files.

For example, in arch/x86/xen/xen-head.S
I see no semicolon after ELFNOTE().

I found this:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0473k/dom1359731141352.html
It says ';' starts a comment line
although it is not the case of GAS.


Same for 3/4.

Yes, that was a typo out of habit. Will fix.

On 07/05/2018 08:58 AM, Andy Lutomirski wrote:

On Tue, Jul 3, 2018 at 4:34 PM, Laura Abbott [off-list ref] wrote:

quoted

The vDSO needs to have a unique build id in a similar manner
to the kernel and modules. Use the build salt macro.

Looks good to me.  I have no idea whose tree these would go through.

I was intending this to go through kbuild tree. Can I take this
as an Ack?

quoted

Signed-off-by: Laura Abbott <redacted>
---
v5: Switched to using the single line BUILD_SALT macro
---
  arch/x86/entry/vdso/vdso-note.S   | 3 +++
  arch/x86/entry/vdso/vdso32/note.S | 3 +++
  2 files changed, 6 insertions(+)

diff --git a/arch/x86/entry/vdso/vdso-note.S b/arch/x86/entry/vdso/vdso-note.S
index 79a071e4357e..79423170118f 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 9fd51f206314..e78047d119f6 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
--

2.17.1

On 07/03/2018 08:59 PM, Masahiro Yamada wrote:

Hi.

Thanks for the update.


2018-07-04 8:34 GMT+09:00 Laura Abbott [off-list ref]:

quoted

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.

Add an ELF note to both the kernel and module which contains some data based
off of a config option.

Signed-off-by: Masahiro Yamada <redacted>
Signed-off-by: Laura Abbott <redacted>
---
v5: I used S-o-b here since the majority of the code was written
already.

I think Suggested-by is good enough.
S-o-b is appended as a patch is passed from people to people.

Anyway, this looks good except one bike-shed.

quoted

Please feel free to change the tag if you think it's not
appropriate. I also tweaked this to take an ascii string instead of just
a hex value since this makes things much easier on the distribution
side.
---

quoted

diff --git a/init/Kconfig b/init/Kconfig
index 041f3a022122..8de789f40db9 100644
--- a/init/Kconfig
+++ b/init/Kconfig

@@ -107,6 +107,15 @@ config LOCALVERSION_AUTO

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

+config BUILD_SALT
+       string "Build ID Salt"
+       default "Linux"

How about empty string ""
for default?

Sure, seems to work fine.

Thanks,
Laura

Sure.

On Thu, Jul 5, 2018 at 12:08 PM, Laura Abbott [off-list ref] wrote:

On 07/05/2018 08:58 AM, Andy Lutomirski wrote:

quoted

On Tue, Jul 3, 2018 at 4:34 PM, Laura Abbott [off-list ref] wrote:

quoted

The vDSO needs to have a unique build id in a similar manner
to the kernel and modules. Use the build salt macro.

Looks good to me.  I have no idea whose tree these would go through.

I was intending this to go through kbuild tree. Can I take this
as an Ack?

quoted

quoted

Signed-off-by: Laura Abbott <redacted>
---
v5: Switched to using the single line BUILD_SALT macro
---
  arch/x86/entry/vdso/vdso-note.S   | 3 +++
  arch/x86/entry/vdso/vdso32/note.S | 3 +++
  2 files changed, 6 insertions(+)

diff --git a/arch/x86/entry/vdso/vdso-note.S

b/arch/x86/entry/vdso/vdso-note.S
index 79a071e4357e..79423170118f 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 9fd51f206314..e78047d119f6 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
--

2.17.1