[PATCH] arm64: hide __efistub_ aliases from kallsyms

[mark.rutland@arm.com (Mark Rutland)]

On Fri, Jan 15, 2016 at 01:28:57PM +0100, Ard Biesheuvel wrote:
> Commit e8f3010f7326 ("arm64/efi: isolate EFI stub from the kernel
> proper") isolated the EFI stub code from the kernel proper by prefixing
> all of its symbols with __efistub_, and selectively allowing access to
> core kernel symbols from the stub by emitting __efistub_ aliases for
> functions and variables that the stub can access legally.
> 
> As an unintended side effect, these aliases are emitted into the
> kallsyms symbol table, which means they may turn up in backtraces,
> e.g.,
> 
>   ...
>   PC is at __efistub_memset+0x108/0x200
>   LR is at fixup_init+0x3c/0x48
>   ...
>   [<ffffff8008328608>] __efistub_memset+0x108/0x200
>   [<ffffff8008094dcc>] free_initmem+0x2c/0x40
>   [<ffffff8008645198>] kernel_init+0x20/0xe0
>   [<ffffff8008085cd0>] ret_from_fork+0x10/0x40
> 
> The backtrace in question has nothing to do with the EFI stub, but
> simply returns one of the several aliases of memset() that have been
> recorded in the kallsyms table. This is undesirable, since it may
> suggest to people who are not aware of this that the issue they are
> seeing is somehow EFI related.
> 
> So hide the __efistub_ aliases from kallsyms, by emitting them as
> absolute linker symbols explicitly. The distinction between those
> and section relative symbols is completely irrelevant to these
> definitions, and to the final link we are performing when these
> definitions are being taken into account (the distinction is only
> relevant to symbols defined inside a section definition when performing
> a partial link), and so the resulting values are identical to the
> original ones. Since absolute symbols are ignored by kallsyms, this
> will result in these values to be omitted from its symbol table.
> 
> After this patch, the backtrace generated from the same address looks
> like this:
>   ...
>   PC is at __memset+0x108/0x200
>   LR is at fixup_init+0x3c/0x48
>   ...
>   [<ffffff8008328608>] __memset+0x108/0x200
>   [<ffffff8008094dcc>] free_initmem+0x2c/0x40
>   [<ffffff8008645198>] kernel_init+0x20/0xe0
>   [<ffffff8008085cd0>] ret_from_fork+0x10/0x40
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/kernel/image.h | 40 +++++++++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
> index ddd61e2d9e3e..ce6f02c56b3d 100644
> --- a/arch/arm64/kernel/image.h
> +++ b/arch/arm64/kernel/image.h
> @@ -81,6 +81,16 @@
>  #ifdef CONFIG_EFI
>  
>  /*
> + * Prevent the symbol aliases below from being emitted into the kallsyms
> + * table, by forcing them to be absolute symbols (which are conveniently
> + * ignored by scripts/kallsyms) rather than section relative symbols.
> + * The distinction is only relevant for partial linking, and only for symbols
> + * that are defined within a section declaration (which is not the case for
> + * the definitions below) so the resulting values will be identical.
> + */
> +#define KALLSYMS_HIDE(sym)	ABSOLUTE(sym)

>From looking at the GNU LD manual recently, I discovered
PROVIDE_HIDDEN(sym), which sounds closer to what we want semantically,
even if ABSOLUTE(sym) acheives the same thing.

If that works, that might be preferable.

Either way this looks sensible to me, so:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> +
> +/*
>   * The EFI stub has its own symbol namespace prefixed by __efistub_, to
>   * isolate it from the kernel proper. The following symbols are legally
>   * accessed by the stub, so provide some aliases to make them accessible.
> @@ -89,25 +99,25 @@
>   * linked at. The routines below are all implemented in assembler in a
>   * position independent manner
>   */
> -__efistub_memcmp		= __pi_memcmp;
> -__efistub_memchr		= __pi_memchr;
> -__efistub_memcpy		= __pi_memcpy;
> -__efistub_memmove		= __pi_memmove;
> -__efistub_memset		= __pi_memset;
> -__efistub_strlen		= __pi_strlen;
> -__efistub_strcmp		= __pi_strcmp;
> -__efistub_strncmp		= __pi_strncmp;
> -__efistub___flush_dcache_area	= __pi___flush_dcache_area;
> +__efistub_memcmp		= KALLSYMS_HIDE(__pi_memcmp);
> +__efistub_memchr		= KALLSYMS_HIDE(__pi_memchr);
> +__efistub_memcpy		= KALLSYMS_HIDE(__pi_memcpy);
> +__efistub_memmove		= KALLSYMS_HIDE(__pi_memmove);
> +__efistub_memset		= KALLSYMS_HIDE(__pi_memset);
> +__efistub_strlen		= KALLSYMS_HIDE(__pi_strlen);
> +__efistub_strcmp		= KALLSYMS_HIDE(__pi_strcmp);
> +__efistub_strncmp		= KALLSYMS_HIDE(__pi_strncmp);
> +__efistub___flush_dcache_area	= KALLSYMS_HIDE(__pi___flush_dcache_area);
>  
>  #ifdef CONFIG_KASAN
> -__efistub___memcpy		= __pi_memcpy;
> -__efistub___memmove		= __pi_memmove;
> -__efistub___memset		= __pi_memset;
> +__efistub___memcpy		= KALLSYMS_HIDE(__pi_memcpy);
> +__efistub___memmove		= KALLSYMS_HIDE(__pi_memmove);
> +__efistub___memset		= KALLSYMS_HIDE(__pi_memset);
>  #endif
>  
> -__efistub__text			= _text;
> -__efistub__end			= _end;
> -__efistub__edata		= _edata;
> +__efistub__text			= KALLSYMS_HIDE(_text);
> +__efistub__end			= KALLSYMS_HIDE(_end);
> +__efistub__edata		= KALLSYMS_HIDE(_edata);
>  
>  #endif
>  
> -- 
> 2.5.0
>