linux-kernel - Re: [PATCH] ARM: use choice for kernel unwinders

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]

Date:   Wed, 22 Aug 2018 16:38:09 +0200
From:   Stefan Agner <stefan@...er.ch>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Russell King - ARM Linux <linux@...linux.org.uk>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARM: use choice for kernel unwinders

On 22.08.2018 12:02, Arnd Bergmann wrote:
> On Wed, Aug 22, 2018 at 12:24 AM Stefan Agner <stefan@...er.ch> wrote:
>>
>> While in theory multiple unwinders could be compiled in, it does
>> not make sense in practise. Use a choice to make the unwinder
>> selection mutually exclusive and mandatory.
>>
>> Already before this commit it has not been possible to deselect
>> FRAME_POINTER. Remove the obsolete comment.
>>
>> Furthermore, to produce a meaningful backtrace with FRAME_POINTER
>> enabled the kernel needs a specific function prologue:
>>     mov    ip, sp
>>     stmfd    sp!, {fp, ip, lr, pc}
>>     sub    fp, ip, #4
>>
>> To get to the required prologue gcc uses apcs and no-sched-prolog.
>> This compiler options are not available on clang, and clang is not
>> able to generate the required prologue. Make the FRAME_POINTER
>> config symbol depending on !clang.
>>
>> Suggested-by: Arnd Bergmann <arnd@...db.de>
>> Signed-off-by: Stefan Agner <stefan@...er.ch>
> 
> Looks ok to me. I've added it to my randconfig test environment, you
> will hear from me within a day if I run into build regressions.
> 
> We may still want to clean up these three lines:
> 
> lib/Kconfig.debug:  select FRAME_POINTER if !MIPS && !PPC &&
> !ARM_UNWIND && !S390 && !MICROBLAZE && !ARC && !X86
> lib/Kconfig.debug:  select FRAME_POINTER if !MIPS && !PPC && !S390 &&
> !MICROBLAZE && !ARM_UNWIND && !ARC && !X86
> lib/Kconfig.debug:  select FRAME_POINTER if !MIPS && !PPC && !S390 &&
> !MICROBLAZE && !ARM_UNWIND && !ARC && !X86
> 
> in which ARM is the odd case that currently depends on an architecture
> specific rather than the architecture itself.

I guess we would just follow X86 lead by saying ARM is guaranteed to
have unwinding support, and hence we can add !ARM.

> We could introduce a 'config ARCH_HAS_UNWINDER' symbol that gets
> selected by mips, ppc, s390, microblaze, arm and x86 unconditionally,
> and then simplify the 'select' statements here.

Yeah I was thinking about something like that too.

It seems to be a bit weird to me that lib/Kconfig.debug selects a
specific stack unwinding technique...

Ideally other config symbol should just ask arch to make sure a
unwinding technique is available (NEED_STACK_UNWINDING?) and arch then
makes sure to provide a reasonable default.

This then also would make it possible to select no stack unwinding in
case arch supports that and all the users of stack unwinding are
disabled too. Not sure how that exactly would look like in Kconfig, I
was thinking like:

choice
    prompt "Choose kernel unwinder"
    optional if !NEED_STACK_UNWINDING
    default UNWINDER_ARM if AEABI && !FUNCTION_GRAPH_TRACER
    default UNWINDER_FRAME_POINTER if !AEABI || FUNCTION_GRAPH_TRACER

But "optional if" does not exist yet :-)

Reading the comments in arch/arm/Kconfig.debug seems to suggest that
once upon a time it was possible to disable stack unwinding on ARM.

But then, maybe we don't really want to go there? Might be interesting
for tinification efforts.

--
Stefan