linux-kernel - Re: [PATCH] mm: kasan: mark file report so ftrace doesn't trace it

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] [day] [month] [year] [list]

Message-ID: <CACT4Y+Yes1Fxk24qemvB6b7NWzSD24ciqZsm0UN61jph46EdOQ@mail.gmail.com>
Date:   Mon, 3 Jun 2019 12:50:56 +0200
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Anders Roxell <anders.roxell@...aro.org>
Cc:     Andrey Ryabinin <aryabinin@...tuozzo.com>,
        kasan-dev <kasan-dev@...glegroups.com>,
        Linux-MM <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH] mm: kasan: mark file report so ftrace doesn't trace it

On Mon, Jun 3, 2019 at 11:11 AM Anders Roxell <anders.roxell@...aro.org> wrote:
>
> __kasan_report() triggers ftrace and the preempt_count() in ftrace
> causes a call to __asan_load4(), breaking the circular dependency by
> making report as no trace for ftrace.
>
> Signed-off-by: Anders Roxell <anders.roxell@...aro.org>
> ---
>  mm/kasan/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/kasan/Makefile b/mm/kasan/Makefile
> index 08b43de2383b..2b2da731483c 100644
> --- a/mm/kasan/Makefile
> +++ b/mm/kasan/Makefile
> @@ -3,12 +3,14 @@ KASAN_SANITIZE := n
>  UBSAN_SANITIZE_common.o := n
>  UBSAN_SANITIZE_generic.o := n
>  UBSAN_SANITIZE_generic_report.o := n
> +UBSAN_SANITIZE_report.o := n
>  UBSAN_SANITIZE_tags.o := n
>  KCOV_INSTRUMENT := n
>
>  CFLAGS_REMOVE_common.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_generic.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_generic_report.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_report.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_tags.o = $(CC_FLAGS_FTRACE)
>
>  # Function splitter causes unnecessary splits in __asan_load1/__asan_store1
> @@ -17,6 +19,7 @@ CFLAGS_REMOVE_tags.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_common.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector)
>  CFLAGS_generic.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector)
>  CFLAGS_generic_report.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector)
> +CFLAGS_report.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector)
>  CFLAGS_tags.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector)
>
>  obj-$(CONFIG_KASAN) := common.o init.o report.o


Acked-by: Dmitry Vyukov <dvyukov@...gle.com>

Is it needed in all section? Or you just followed the pattern?
Different flag changes were initially done on very specific files for
specific reasons. E.g. -fno-conserve-stack is only for performance
reasons, so report* should not be there. But I see Peter already added
generic_report.o there. Perhaps we need to give up on selective
per-file changes, because this causes constant flow of new bugs in the
absence of testing and just do something like:

KASAN_SANITIZE := n
KCOV_INSTRUMENT := n
UBSAN_SANITIZE := n
CFLAGS_REMOVE = $(CC_FLAGS_FTRACE)
CFLAGS := $(call cc-option, -fno-conserve-stack -fno-stack-protector)