Re: [PATCH] riscv: enable HAVE_ARCH_STACKLEAK

[Jisheng Zhang <jszhang@kernel.org>]

On Fri, Jun 21, 2024 at 03:10:42PM -0700, Charlie Jenkins wrote:
> On Mon, Jun 17, 2024 at 08:30:29PM +0800, Jisheng Zhang wrote:
> > Add support for the stackleak feature. Whenever the kernel returns to user
> > space the kernel stack is filled with a poison value.
> > 
> > At the same time, disables the plugin in EFI stub code because EFI stub
> > is out of scope for the protection.
> > 
> > Tested on qemu and milkv duo:
> > / # echo STACKLEAK_ERASING > /sys/kernel/debug/provoke-crash/DIRECT
> > [   38.675575] lkdtm: Performing direct entry STACKLEAK_ERASING
> > [   38.678448] lkdtm: stackleak stack usage:
> > [   38.678448]   high offset: 288 bytes
> > [   38.678448]   current:     496 bytes
> > [   38.678448]   lowest:      1328 bytes
> > [   38.678448]   tracked:     1328 bytes
> > [   38.678448]   untracked:   448 bytes
> > [   38.678448]   poisoned:    14312 bytes
> > [   38.678448]   low offset:  8 bytes
> > [   38.689887] lkdtm: OK: the rest of the thread stack is properly erased
> > 
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >  arch/riscv/Kconfig                    | 1 +
> >  arch/riscv/kernel/entry.S             | 4 ++++
> >  drivers/firmware/efi/libstub/Makefile | 3 ++-
> >  3 files changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 0525ee2d63c7..9cbfdffec96c 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -118,6 +118,7 @@ config RISCV
> >  	select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
> >  	select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
> >  	select HAVE_ARCH_SECCOMP_FILTER
> > +	select HAVE_ARCH_STACKLEAK
> 
> When this is selected, stackleak.h include
> arch/riscv/include/asm/thread_info.h without sizes.h and I hit:
> 
> ./arch/riscv/include/asm/thread_info.h:30:33: error: ‘SZ_4K’ undeclared here (not in a function)
>    30 | #define OVERFLOW_STACK_SIZE     SZ_4K
>       |                                 ^~~~~
> 
> Adding "#include <linux/sizes.h>" to thread_info.h resolves the issue.
> I am testing this based on 6.10-rc4. Did you encounter this?

I didn't meet this kind of compiler error when testing. Mind
to share your .config file? It looks strange.

> 
> - Charlie
> 
> >  	select HAVE_ARCH_THREAD_STRUCT_WHITELIST
> >  	select HAVE_ARCH_TRACEHOOK
> >  	select HAVE_ARCH_TRANSPARENT_HUGEPAGE if 64BIT && MMU
> > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> > index 68a24cf9481a..80ff55a26d13 100644
> > --- a/arch/riscv/kernel/entry.S
> > +++ b/arch/riscv/kernel/entry.S
> > @@ -130,6 +130,10 @@ SYM_CODE_START_NOALIGN(ret_from_exception)
> >  #endif
> >  	bnez s0, 1f
> >  
> > +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
> > +	call	stackleak_erase_on_task_stack
> > +#endif
> > +
> >  	/* Save unwound kernel stack pointer in thread_info */
> >  	addi s0, sp, PT_SIZE_ON_STACK
> >  	REG_S s0, TASK_TI_KERNEL_SP(tp)
> > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> > index 06f0428a723c..3a9521c57641 100644
> > --- a/drivers/firmware/efi/libstub/Makefile
> > +++ b/drivers/firmware/efi/libstub/Makefile
> > @@ -28,7 +28,8 @@ cflags-$(CONFIG_ARM)		+= -DEFI_HAVE_STRLEN -DEFI_HAVE_STRNLEN \
> >  				   -DEFI_HAVE_MEMCHR -DEFI_HAVE_STRRCHR \
> >  				   -DEFI_HAVE_STRCMP -fno-builtin -fpic \
> >  				   $(call cc-option,-mno-single-pic-base)
> > -cflags-$(CONFIG_RISCV)		+= -fpic -DNO_ALTERNATIVE -mno-relax
> > +cflags-$(CONFIG_RISCV)		+= -fpic -DNO_ALTERNATIVE -mno-relax \
> > +				   $(DISABLE_STACKLEAK_PLUGIN)
> >  cflags-$(CONFIG_LOONGARCH)	+= -fpie
> >  
> >  cflags-$(CONFIG_EFI_PARAMS_FROM_FDT)	+= -I$(srctree)/scripts/dtc/libfdt
> > -- 
> > 2.43.0
> > 
> > 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv