Re: [PATCH] lkdtm: Disable return thunks in rodata.c

[Kees Cook]

On Mon, Jul 18, 2022 at 07:50:25AM -0700, Josh Poimboeuf wrote:
> The following warning was seen:
> 
>   WARNING: CPU: 0 PID: 0 at arch/x86/kernel/alternative.c:557 apply_returns (arch/x86/kernel/alternative.c:557 (discriminator 1))
>   Modules linked in:
>   CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.19.0-rc4-00008-gee88d363d156 #1
>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-4 04/01/2014
>   RIP: 0010:apply_returns (arch/x86/kernel/alternative.c:557 (discriminator 1))
>   Code: ff ff 74 cb 48 83 c5 04 49 39 ee 0f 87 81 fe ff ff e9 22 ff ff ff 0f 0b 48 83 c5 04 49 39 ee 0f 87 6d fe ff ff e9 0e ff ff ff <0f> 0b 48 83 c5 04 49 39 ee 0f 87 59 fe ff ff e9 fa fe ff ff 48 89
> 
> The warning happened when apply_returns() failed to convert "JMP
> __x86_return_thunk" to RET.  It was instead a JMP to nowhere, due to the
> thunk relocation not getting resolved.
> 
> That rodata.o code is objcopy'd to .rodata, and later memcpy'd, so
> relocations don't work (and are apparently silently ignored).
> 
> LKDTM is only used for testing, so the naked RET should be fine.  So
> just disable return thunks for that file.
> 
> While at it, disable objtool and KCSAN for the file.
> 
> Fixes: 0b53c374b9ef ("x86/retpoline: Use -mfunction-return")
> Reported-by: kernel test robot <oliver.sang@xxxxxxxxx>
> Link: https://lore.kernel.org/lkml/Ys58BxHxoDZ7rfpr@xsang-OptiPlex-9020/
> Debugged-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>

Thanks!

Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>

-Kees

> ---
>  arch/x86/Makefile           | 1 +
>  drivers/misc/lkdtm/Makefile | 9 ++++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 1f40dad30d50..7854685c5f25 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -27,6 +27,7 @@ RETHUNK_CFLAGS		:= -mfunction-return=thunk-extern
>  RETPOLINE_CFLAGS	+= $(RETHUNK_CFLAGS)
>  endif
>  
> +export RETHUNK_CFLAGS
>  export RETPOLINE_CFLAGS
>  export RETPOLINE_VDSO_CFLAGS
>  
> diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> index 2e0aa74ac185..95ef971b5e1c 100644
> --- a/drivers/misc/lkdtm/Makefile
> +++ b/drivers/misc/lkdtm/Makefile
> @@ -13,10 +13,13 @@ lkdtm-$(CONFIG_LKDTM)		+= cfi.o
>  lkdtm-$(CONFIG_LKDTM)		+= fortify.o
>  lkdtm-$(CONFIG_PPC_64S_HASH_MMU)	+= powerpc.o
>  
> -KASAN_SANITIZE_rodata.o		:= n
>  KASAN_SANITIZE_stackleak.o	:= n
> -KCOV_INSTRUMENT_rodata.o	:= n
> -CFLAGS_REMOVE_rodata.o		+= $(CC_FLAGS_LTO)
> +
> +KASAN_SANITIZE_rodata.o			:= n
> +KCSAN_SANITIZE_rodata.o			:= n
> +KCOV_INSTRUMENT_rodata.o		:= n
> +OBJECT_FILES_NON_STANDARD_rodata.o	:= y
> +CFLAGS_REMOVE_rodata.o			+= $(CC_FLAGS_LTO) $(RETHUNK_CFLAGS)
>  
>  OBJCOPYFLAGS :=
>  OBJCOPYFLAGS_rodata_objcopy.o	:= \
> -- 
> 2.36.1
> 

-- 
Kees Cook