On Thu, Mar 11, 2021 at 6:39 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > On Thu, Mar 11, 2021 at 04:49:06PM -0800, Sami Tolvanen wrote: > > CONFIG_CFI_CLANG_SHADOW assumes the __cfi_check() function is page > > aligned and at the beginning of the .text section. While Clang would > > normally align the function correctly, it fails to do so for modules > > with no executable code. > > > > This change ensures the correct __cfi_check() location and > > alignment. It also discards the .eh_frame section, which Clang can > > generate with certain sanitizers, such as CFI. > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=46293 > > Signed-off-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx> > > --- > > scripts/module.lds.S | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/module.lds.S b/scripts/module.lds.S > > index 168cd27e6122..552ddb084f76 100644 > > --- a/scripts/module.lds.S > > +++ b/scripts/module.lds.S > > @@ -3,10 +3,13 @@ > > * Archs are free to supply their own linker scripts. ld will > > * combine them automatically. > > */ > > +#include <asm/page.h> > > + > > SECTIONS { > > /DISCARD/ : { > > *(.discard) > > *(.discard.*) > > + *(.eh_frame) > > } > > > > __ksymtab 0 : { *(SORT(___ksymtab+*)) } > > @@ -40,7 +43,16 @@ SECTIONS { > > *(.rodata..L*) > > } > > > > - .text : { *(.text .text.[0-9a-zA-Z_]*) } > > +#ifdef CONFIG_CFI_CLANG > > + /* > > + * With CFI_CLANG, ensure __cfi_check is at the beginning of the > > + * .text section, and that the section is aligned to page size. > > + */ > > + .text : ALIGN(PAGE_SIZE) { > > + *(.text.__cfi_check) > > + *(.text .text.[0-9a-zA-Z_]* .text..L.cfi*) > > + } > > +#endif > > Whoops, I think this reverts to the default .text declaration when > CONFIG_CFI_CLANG is unset. Oops, thanks for pointing this out. I'll fix it in v2. > I think the only thing that needs the ifdef is the ALIGN, yes? Perhaps > something like this? > > #ifdef CONFIG_CFI_CLANG > # define ALIGN_CFI ALIGN(PAGE_SIZE) > #else > # define ALIGN_CFI > #endif > > .text : ALIGN_CFI { > *(.text.__cfi_check) > *(.text .text.[0-9a-zA-Z_]* .text..L.cfi*) > } Agreed, that looks better. Sami
