On Fri, Jul 31, 2020 at 04:07:57PM -0700, Kees Cook wrote:
From: Nick Desaulniers ndesaulniers@google.com
Basically, consider .text.{hot|unlikely|unknown}.* part of .text, too.
When compiling with profiling information (collected via PGO instrumentations or AutoFDO sampling), Clang will separate code into .text.hot, .text.unlikely, or .text.unknown sections based on profiling information. After D79600 (clang-11), these sections will have a trailing `.` suffix, ie. .text.hot., .text.unlikely., .text.unknown..
When using -ffunction-sections together with profiling infomation, either explicitly (FGKASLR) or implicitly (LTO), code may be placed in sections following the convention: .text.hot.<foo>, .text.unlikely.<bar>, .text.unknown.<baz> where <foo>, <bar>, and <baz> are functions. (This produces one section per function; we generally try to merge these all back via linker script so that we don't have 50k sections).
For the above cases, we need to teach our linker scripts that such sections might exist and that we'd explicitly like them grouped together, otherwise we can wind up with code outside of the _stext/_etext boundaries that might not be mapped properly for some architectures, resulting in boot failures.
If the linker script is not told about possible input sections, then where the section is placed as output is a heuristic-laiden mess that's non-portable between linkers (ie. BFD and LLD), and has resulted in many hard to debug bugs. Kees Cook is working on cleaning this up by adding --orphan-handling=warn linker flag used in ARCH=powerpc to additional architectures. In the case of linker scripts, borrowing from the Zen of Python: explicit is better than implicit.
Also, ld.bfd's internal linker script considers .text.hot AND .text.hot.* to be part of .text, as well as .text.unlikely and .text.unlikely.*. I didn't see support for .text.unknown.*, and didn't see Clang producing such code in our kernel builds, but I see code in LLVM that can produce such section names if profiling information is missing. That may point to a larger issue with generating or collecting profiles, but I would much rather be safe and explicit than have to debug yet another issue related to orphan section placement.
Reported-by: Jian Cai jiancai@google.com Suggested-by: Fāng-ruì Sòng maskray@google.com Tested-by: Luis Lozano llozano@google.com Tested-by: Manoj Gupta manojgupta@google.com Acked-by: Kees Cook keescook@chromium.org Cc: stable@vger.kernel.org Link: https://sourceware.org/git/?p=binutils-gdb.git%3Ba=commitdiff%3Bh=add44f8d5c... Link: https://sourceware.org/git/?p=binutils-gdb.git%3Ba=commitdiff%3Bh=1de778ed23... Link: https://reviews.llvm.org/D79600 Link: https://bugs.chromium.org/p/chromium/issues/detail?id=1084760 Debugged-by: Luis Lozano llozano@google.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com Signed-off-by: Kees Cook keescook@chromium.org
include/asm-generic/vmlinux.lds.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 2593957f6e8b..af5211ca857c 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -561,7 +561,10 @@ */ #define TEXT_TEXT \ ALIGN_FUNCTION(); \
*(.text.hot TEXT_MAIN .text.fixup .text.unlikely) \
*(.text.hot .text.hot.*) \*(TEXT_MAIN .text.fixup) \*(.text.unlikely .text.unlikely.*) \*(.text.unknown .text.unknown.*) \-- 2.25.1
This also changes the ordering to place all hot resp unlikely sections separate from other text, while currently it places the hot/unlikely bits of each file together with the rest of the code in that file. That seems like a reasonable change and should be mentioned in the commit message.
However, the history of their being together comes from
9bebe9e5b0f3 ("kbuild: Fix .text.unlikely placement")
which seems to indicate there was some problem with having them separated out, although I don't quite understand what the issue was from the commit message.
Cc Andi and Michal to see if they remember.