Date:   Fri, 15 Oct 2021 18:11:41 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...belt.com>
To:     alex@...ti.fr
CC:     nathan@...nel.org, elver@...gle.com, akpm@...ux-foundation.org,
        ryabinin.a.a@...il.com, glider@...gle.com, andreyknvl@...il.com,
        dvyukov@...gle.com, ndesaulniers@...gle.com,
        Arnd Bergmann <arnd@...db.de>, kasan-dev@...glegroups.com,
        linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
        linux-riscv@...ts.infradead.org,
        Paul Walmsley <paul.walmsley@...ive.com>,
        aou@...s.berkeley.edu, linux-mm@...ck.org
Subject:     Re: [PATCH] kasan: Always respect CONFIG_KASAN_STACK

On Thu, 14 Oct 2021 11:31:00 PDT (-0700), alex@...ti.fr wrote:
> Hi Nathan,
>
> Le 14/10/2021 à 18:55, Nathan Chancellor a écrit :
>> On Fri, Oct 08, 2021 at 11:46:55AM -0700, Palmer Dabbelt wrote:
>>> On Thu, 23 Sep 2021 07:59:46 PDT (-0700), nathan@...nel.org wrote:
>>>> On Thu, Sep 23, 2021 at 12:07:17PM +0200, Marco Elver wrote:
>>>>> On Wed, 22 Sept 2021 at 22:55, Nathan Chancellor <nathan@...nel.org> wrote:
>>>>>> Currently, the asan-stack parameter is only passed along if
>>>>>> CFLAGS_KASAN_SHADOW is not empty, which requires KASAN_SHADOW_OFFSET to
>>>>>> be defined in Kconfig so that the value can be checked. In RISC-V's
>>>>>> case, KASAN_SHADOW_OFFSET is not defined in Kconfig, which means that
>>>>>> asan-stack does not get disabled with clang even when CONFIG_KASAN_STACK
>>>>>> is disabled, resulting in large stack warnings with allmodconfig:
>>>>>>
>>>>>> drivers/video/fbdev/omap2/omapfb/displays/panel-lgphilips-lb035q02.c:117:12:
>>>>>> error: stack frame size (14400) exceeds limit (2048) in function
>>>>>> 'lb035q02_connect' [-Werror,-Wframe-larger-than]
>>>>>> static int lb035q02_connect(struct omap_dss_device *dssdev)
>>>>>>             ^
>>>>>> 1 error generated.
>>>>>>
>>>>>> Ensure that the value of CONFIG_KASAN_STACK is always passed along to
>>>>>> the compiler so that these warnings do not happen when
>>>>>> CONFIG_KASAN_STACK is disabled.
>>>>>>
>>>>>> Link: https://github.com/ClangBuiltLinux/linux/issues/1453
>>>>>> References: 6baec880d7a5 ("kasan: turn off asan-stack for clang-8 and earlier")
>>>>>> Signed-off-by: Nathan Chancellor <nathan@...nel.org>
>>>>>
>>>>> Reviewed-by: Marco Elver <elver@...gle.com>
>>>>
>>>> Thanks!
>>>>
>>>>> [ Which tree are you planning to take it through? ]
>>>>
>>>> Gah, I was intending for it to go through -mm, then I cc'd neither
>>>> Andrew nor linux-mm... :/ Andrew, do you want me to resend or can you
>>>> grab it from LKML?
>>>
>>> Acked-by: Palmer Dabbelt <palmerdabbelt@...gle.com>
>>>
>>> (assuming you still want it through somewhere else)
>>
>> Thanks, it is now in mainline as commit 19532869feb9 ("kasan: always
>> respect CONFIG_KASAN_STACK").
>>
>>>>> Note, arch/riscv/include/asm/kasan.h mentions KASAN_SHADOW_OFFSET in
>>>>> comment (copied from arm64). Did RISC-V just forget to copy over the
>>>>> Kconfig option?
>>>>
>>>> I do see it defined in that file as well but you are right that they did
>>>> not copy the Kconfig logic, even though it was present in the tree when
>>>> RISC-V KASAN was implemented. Perhaps they should so that they get
>>>> access to the other flags in the "else" branch?
>>>
>>> Ya, looks like we just screwed this up.  I'm seeing some warnings like
>>>
>>>     cc1: warning: ‘-fsanitize=kernel-address’ with stack protection is not supported without ‘-fasan-shadow-offset=’ for this target
>>
>> Hmmm, I thought I did a GCC build with this change but I must not have
>> :/
>>
>>> which is how I ended up here, I'm assuming that's what you're talking about
>>> here?  LMK if you were planning on sending along a fix or if you want me to
>>> go figure it out.
>>
>> I took a look at moving the logic into Kconfig like arm64 before sending
>> this change and I did not really understand it well enough to do so. I
>> think it would be best if you were able to do that so that nothing gets
>> messed up.
>>
>
> I'll do it tomorrow, I'm the last one who touched kasan on riscv :)

Any luck?  I tried what I think is the simple way to do it last week, 
(merging with Linus' tree is turning these warnings into build 
failures) but it's hanging on boot.  Not sure what's going on

    diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
    index c3f3fd583e04..d3998b4a45f1 100644
    --- a/arch/riscv/Kconfig
    +++ b/arch/riscv/Kconfig
    @@ -212,6 +212,12 @@ config PGTABLE_LEVELS
     config LOCKDEP_SUPPORT
            def_bool y
    
    +config KASAN_SHADOW_OFFSET
    +       hex
    +       depends on KASAN_GENERIC
    +       default 0xdfffffc800000000  if 64BIT
    +       default 0xffffffff          if 32BIT
    +
     source "arch/riscv/Kconfig.socs"
     source "arch/riscv/Kconfig.erratas"
    
    diff --git a/arch/riscv/include/asm/kasan.h b/arch/riscv/include/asm/kasan.h
    index a2b3d9cdbc86..b00f503ec124 100644
    --- a/arch/riscv/include/asm/kasan.h
    +++ b/arch/riscv/include/asm/kasan.h
    @@ -30,8 +30,7 @@
     #define KASAN_SHADOW_SIZE      (UL(1) << ((CONFIG_VA_BITS - 1) - KASAN_SHADOW_SCALE_SHIFT))
     #define KASAN_SHADOW_START     KERN_VIRT_START
     #define KASAN_SHADOW_END       (KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
    -#define KASAN_SHADOW_OFFSET    (KASAN_SHADOW_END - (1ULL << \
    -                                       (64 - KASAN_SHADOW_SCALE_SHIFT)))
    +#define KASAN_SHADOW_OFFSET    _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
    
     void kasan_init(void);
     asmlinkage void kasan_early_init(void);

>
> Thanks,
>
> Alex
>
>> Cheers,
>> Nathan
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>

Powered by blists - more mailing lists