linux-kernel - Re: [PATCH v6 3/6] x86, efi, kasan: #undef memset/memcpy/memmove per arch.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150929083814.GA32127@gmail.com>
Date:	Tue, 29 Sep 2015 10:38:14 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Andrey Ryabinin <ryabinin.a.a@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Will Deacon <will.deacon@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	linux-arm-kernel@...ts.infradead.org,
	Matt Fleming <matt.fleming@...el.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	linux-efi@...r.kernel.org, fengguang.wu@...el.com,
	Linus Walleij <linus.walleij@...aro.org>,
	Alexander Potapenko <glider@...gle.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org,
	David Keitel <dkeitel@...eaurora.org>, linux-mm@...ck.org,
	Alexey Klimov <klimov.linux@...il.com>,
	Yury <yury.norov@...il.com>,
	Andrey Konovalov <andreyknvl@...gle.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH v6 3/6] x86, efi, kasan: #undef memset/memcpy/memmove per
 arch.


* Andrey Ryabinin <ryabinin.a.a@...il.com> wrote:

> In not-instrumented code KASAN replaces instrumented
> memset/memcpy/memmove with not-instrumented analogues
> __memset/__memcpy/__memove.
> However, on x86 the EFI stub is not linked with the kernel.
> It uses not-instrumented mem*() functions from
> arch/x86/boot/compressed/string.c
> So we don't replace them with __mem*() variants in EFI stub.
> 
> On ARM64 the EFI stub is linked with the kernel, so we should
> replace mem*() functions with __mem*(), because the EFI stub
> runs before KASAN sets up early shadow.
> 
> So let's move these #undef mem* into arch's asm/efi.h which is
> also included by the EFI stub.
> 
> Also, this will fix the warning in 32-bit build reported by
> kbuild test robot <fengguang.wu@...el.com>:
> 	efi-stub-helper.c:599:2: warning: implicit declaration of function 'memcpy'
> 
> Signed-off-by: Andrey Ryabinin <ryabinin.a.a@...il.com>
> ---
>  arch/x86/include/asm/efi.h             | 12 ++++++++++++
>  drivers/firmware/efi/libstub/efistub.h |  4 ----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index 155162e..6db2742 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -86,6 +86,18 @@ extern u64 asmlinkage efi_call(void *fp, ...);
>  extern void __iomem *__init efi_ioremap(unsigned long addr, unsigned long size,
>  					u32 type, u64 attribute);
>  
> +/*
> + * CONFIG_KASAN may redefine memset to __memset.
> + * __memset function is present only in kernel binary.
> + * Since the EFI stub linked into a separate binary it
> + * doesn't have __memset(). So we should use standard
> + * memset from arch/x86/boot/compressed/string.c
> + * The same applies to memcpy and memmove.
> + */
> +#undef memcpy
> +#undef memset
> +#undef memmove

Hm, so this hack got upstream via -mm, and it breaks the 64-bit x86 build with 
some configs:

 arch/x86/platform/efi/efi.c:673:3: error: implicit declaration of function ‘memcpy’ [-Werror=implicit-function-declaration]
 arch/x86/platform/efi/efi_64.c:139:2: error: implicit declaration of function ‘memcpy’ [-Werror=implicit-function-declaration]
 ./arch/x86/include/asm/desc.h:121:2: error: implicit declaration of function ‘memcpy’ [-Werror=implicit-function-declaration]

I guess it's about EFI=y but KASAN=n. Config attached.

beyond fixing the build bug ... could we also engineer this in a better fashion 
than spreading random #undefs across various KASAN unrelated headers?

Thanks,

	Ingo

View attachment "config" of type "text/plain" (118647 bytes)