Re: [PATCH 2/5 v16] ARM: Replace string mem* functions for KASan

[Naresh Kamboju <naresh.kamboju@linaro.org>]

On Mon, 19 Oct 2020 at 14:14, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> From: Andrey Ryabinin <aryabinin@virtuozzo.com>
>
> Functions like memset()/memmove()/memcpy() do a lot of memory
> accesses.
>
> If a bad pointer is passed to one of these functions it is important
> to catch this. Compiler instrumentation cannot do this since these
> functions are written in assembly.
>
> KASan replaces these memory functions with instrumented variants.
>
> The original functions are declared as weak symbols so that
> the strong definitions in mm/kasan/kasan.c can replace them.
>
> The original functions have aliases with a '__' prefix in their
> name, so we can call the non-instrumented variant if needed.
>
> We must use __memcpy()/__memset() in place of memcpy()/memset()
> when we copy .data to RAM and when we clear .bss, because
> kasan_early_init cannot be called before the initialization of
> .data and .bss.
>
> For the kernel compression and EFI libstub's custom string
> libraries we need a special quirk: even if these are built
> without KASan enabled, they rely on the global headers for their
> custom string libraries, which means that e.g. memcpy()
> will be defined to __memcpy() and we get link failures.
> Since these implementations are written i C rather than
> assembly we use e.g. __alias(memcpy) to redirected any
> users back to the local implementation.
>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: kasan-dev@googlegroups.com
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> Tested-by: Ard Biesheuvel <ardb@kernel.org> # QEMU/KVM/mach-virt/LPAE/8G
> Tested-by: Florian Fainelli <f.fainelli@gmail.com> # Brahma SoCs
> Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> # i.MX6Q
> Reported-by: Russell King - ARM Linux <linux@armlinux.org.uk>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Abbott Liu <liuwenliang@huawei.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v15->v16:
> - Fold in Ahmad Fatoum's fixup for fortify
> - Collect Florian's Tested-by
> - Resend with the other patches
> ChangeLog v14->v15:
> - Resend with the other patches
> ChangeLog v13->v14:
> - Resend with the other patches
> ChangeLog v12->v13:
> - Rebase on kernel v5.9-rc1
> ChangeLog v11->v12:
> - Resend with the other changes.
> ChangeLog v10->v11:
> - Resend with the other changes.
> ChangeLog v9->v10:
> - Rebase on v5.8-rc1
> ChangeLog v8->v9:
> - Collect Ard's tags.
> ChangeLog v7->v8:
> - Use the less invasive version of handling the global redefines
>   of the string functions in the decompressor: __alias() the
>   functions locally in the library.
> - Put in some more comments so readers of the code knows what
>   is going on.
> ChangeLog v6->v7:
> - Move the hacks around __SANITIZE_ADDRESS__ into this file
> - Edit the commit message
> - Rebase on the other v2 patches
> ---
>  arch/arm/boot/compressed/string.c | 19 +++++++++++++++++++
>  arch/arm/include/asm/string.h     | 26 ++++++++++++++++++++++++++
>  arch/arm/kernel/head-common.S     |  4 ++--
>  arch/arm/lib/memcpy.S             |  3 +++
>  arch/arm/lib/memmove.S            |  5 ++++-
>  arch/arm/lib/memset.S             |  3 +++
>  6 files changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/string.c b/arch/arm/boot/compressed/string.c
> index ade5079bebbf..8c0fa276d994 100644
> --- a/arch/arm/boot/compressed/string.c
> +++ b/arch/arm/boot/compressed/string.c
> @@ -7,6 +7,25 @@
>
>  #include <linux/string.h>
>
> +/*
> + * The decompressor is built without KASan but uses the same redirects as the
> + * rest of the kernel when CONFIG_KASAN is enabled, defining e.g. memcpy()
> + * to __memcpy() but since we are not linking with the main kernel string
> + * library in the decompressor, that will lead to link failures.
> + *
> + * Undefine KASan's versions, define the wrapped functions and alias them to
> + * the right names so that when e.g. __memcpy() appear in the code, it will
> + * still be linked to this local version of memcpy().
> + */
> +#ifdef CONFIG_KASAN
> +#undef memcpy
> +#undef memmove
> +#undef memset
> +void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias(memcpy);
> +void *__memmove(void *__dest, __const void *__src, size_t count) __alias(memmove);
> +void *__memset(void *s, int c, size_t count) __alias(memset);
> +#endif
> +
>  void *memcpy(void *__dest, __const void *__src, size_t __n)

arm KASAN build failure noticed on linux next 20201106 tag.
gcc: 9.x

Build error:
---------------
arch/arm/boot/compressed/string.c:24:1: error: attribute 'alias'
argument not a string
   24 | void *__memcpy(void *__dest, __const void *__src, size_t __n)
__alias(memcpy);
      | ^~~~
arch/arm/boot/compressed/string.c:25:1: error: attribute 'alias'
argument not a string
   25 | void *__memmove(void *__dest, __const void *__src, size_t
count) __alias(memmove);
      | ^~~~
arch/arm/boot/compressed/string.c:26:1: error: attribute 'alias'
argument not a string
   26 | void *__memset(void *s, int c, size_t count) __alias(memset);
      | ^~~~

Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>

Build details link,
https://builds.tuxbuild.com/1juBs4tXRA6Cwhd1Qnhh4vzCtDx/

-- 
Linaro LKFT
https://lkft.linaro.org

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel