changed section flags for .note.Linux in arch/x86/entry/vdso/vdso-note.S

[tpimh]

arch/x86/entry/vdso/vdso-note.S:15:1: error: changed section flags for .note.Linux, expected: 0x2
.pushsection .note.Linux, "",@note ; .balign 4 ; .long 2f - 1f ; .long 4484f - 3f ; .long 0x100 ; 1:.asciz "Linux" ; 2:.balign 4 ; 3: .asciz "" ; 4484:.balign 4 ; .popsection ;
^

[nickdesaulniers]

That's not a great error message, llvm should print the flag rather than the hex constant. The "" likely needs a letter or two in there for the section flags.

[ihalip]

It seems that GNU as allows a second .pushsection with empty flags. Reproducer:

$ cat vdso-note.s
.pushsection .note.Linux, "a",@note ;
 .long 0x1234 ;
.popsection ;

.pushsection .note.Linux, "",@note ;
 .long 0x5678 ;
.popsection ;

$ as -o vdso-note.o vdso-note.s && objdump -D vdso-note.o
[...]
0000000000000000 <.note.Linux>:
   0:	34 12                	xor    $0x12,%al
   2:	00 00                	add    %al,(%rax)
   4:	78 56                	js     0x5c
	...

$ clang-11 -cc1as -triple x86_64-pc-linux-gnu -o vdso-note.o vdso-note.s
vdso-note.s:5:1: error: changed section flags for .note.Linux, expected: 0x2
.pushsection .note.Linux, "",@note ;

Looks like the fix should be easy enough on the LLVM side - just reuse the flags of the existing section. I'll try to fix this and send a review.

[MaskRay]

See https://lists.llvm.org/pipermail/llvm-dev/2020-February/139390.html

https://reviews.llvm.org/D73999 added the error. I agree that the error message is not great but there is some technical difficulty there. I have noted we should improve it.

.section .foo,"ax",@progbits; .byte 1
.section .foo,"",@progbits; .byte 2
.section .foo,"a",@progbits; .byte 3

So GNU as does not error for the empty flags case.

a.s:3: Error: changed section attributes for .foo

I asked https://sourceware.org/ml/binutils/2020-03/msg00053.html whether GNU as wants to emit an error.

I lean toward an error for consistency. Yes, we should fix the kernel assembly.

[nickdesaulniers]

I agree that we should error; we can likely fix this in the Linux kernel source code.

[ihalip]

The ELFNOTE macros are used with the "a" flags throughout the kernel. But I don't think changing the macro to make this hardcoded or implicit is a good idea.
On the other hand, the BUILD_SALT macro is there to add a piece of data to the binary (to tie a vDSO build to the kernel build) and doesn't seem needed at runtime, so changing it to use the same flag also doesn't sit well with me.

What about moving the build salt into it's own section? Such a change would look like this:

diff --git a/include/linux/build-salt.h b/include/linux/build-salt.h
index bb007bd05e7a..95292c7b7e91 100644
--- a/include/linux/build-salt.h
+++ b/include/linux/build-salt.h
@@ -8,12 +8,12 @@
 #ifdef __ASSEMBLER__
 
 #define BUILD_SALT \
-       ELFNOTE(Linux, LINUX_ELFNOTE_BUILD_SALT, .asciz CONFIG_BUILD_SALT)
+       ELFNOTE(buildsalt, LINUX_ELFNOTE_BUILD_SALT, .asciz CONFIG_BUILD_SALT)
 
 #else
 
 #define BUILD_SALT \
-       ELFNOTE32("Linux", LINUX_ELFNOTE_BUILD_SALT, CONFIG_BUILD_SALT)
+       ELFNOTE32("buildsalt", LINUX_ELFNOTE_BUILD_SALT, CONFIG_BUILD_SALT)
 
 #endif
 

cc @masahir0y

[nickdesaulniers]

If I preprocess the assembly, it looks like:

.pushsection .note.Linux, "a",@note ; .balign 4 ; .long 2f - 1f ; .long 4484f - 3f ; .long 0 ; 1:.asciz "Linux" ; 2:.balign 4 ; 3:
.long 329216
4484:.balign 4 ; .popsection ;
.pushsection .note.Linux, "",@note ; .balign 4 ; .long 2f - 1f ; .long 4484f - 3f ; .long 0x100 ; 1:.asciz "Linux" ; 2:.balign 4 ; 3: .asciz "" ; 4484:.balign 4 ; .popsection ;

So we have:

.pushsection .note.Linux, "a",@note ;
.pushsection .note.Linux, "",@note ;

[nickdesaulniers]

Ah, so it looks like all instances of ELFNOTE_START in the kernel set "a", except for ELFNOTE. I think this would be the right fix:

diff --git a/include/linux/elfnote.h b/include/linux/elfnote.h
index f236f5b931b2..7fdd7f355b52 100644
--- a/include/linux/elfnote.h
+++ b/include/linux/elfnote.h
@@ -54,7 +54,7 @@
 .popsection                            ;
 
 #define ELFNOTE(name, type, desc)              \
-       ELFNOTE_START(name, type, "")           \
+       ELFNOTE_START(name, type, "a")          \
                desc                    ;       \
        ELFNOTE_END

[nickdesaulniers]

(or better yet, I'd refactor the flag to be implicitly "a", and update all uses so they were no longer able to pass potentially different flags).

[ihalip]

(or better yet, I'd refactor the flag to be implicitly "a", and update all uses so they were no longer able to pass potentially different flags).

Agreed. It actually doesn't seem to matter what flags are set for .note.* sections, since the linker merges them all into a PT_NOTE section which is marked as SHF_ALLOC only. Let me spin up a patch.

[masahir0y]

On the other hand, the BUILD_SALT macro is there to add a piece of data to the binary (to tie a vDSO build to the kernel build) and doesn't seem needed at runtime, so changing it to use the same flag also doesn't sit well with me.

Right, we do not need BUILD_SALT at runtime, but
linker scripts puts (.note.*) into the same section.
So, ".note.buildsalt" would go to the same section after all.

The buildsalt is only a few bytes data, so this is not a big deal.

(or better yet, I'd refactor the flag to be implicitly "a", and update all uses so they were no longer able to pass potentially different flags).

I agree too.

[ihalip]

This turned into a series because I found another opportunity to do a minor change. See the last 2 commits: https://github.com/ihalip/linux/commits/elfnote
If you think these are OK, I can send them to LKML.

[nickdesaulniers]

First one LGTM, left comments on the second. Thank you for the fixes!

[masahir0y]

This turned into a series because I found another opportunity to do a minor change. See the last 2 commits: https://github.com/ihalip/linux/commits/elfnote
If you think these are OK, I can send them to LKML.

LGTM.
If there is no objection, please send them to ML.
Thanks.

[nickdesaulniers]

I sent https://lore.kernel.org/lkml/[email protected]/T/#u to the list. I'd like to rebase @ihalip changes and get those submitted, too, though treewide changes sometimes take longer to land.