linux-kernel - Re: [PATCH v6 15/18] khwasan, arm64: add brk handler for inline instrumentation

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]

Message-ID: <CAAeHK+wVgmp2G63c5w4sMH1i97Ju-YW7RUQgOQjr5d8aMgh1EQ@mail.gmail.com>
Date:   Mon, 17 Sep 2018 21:12:00 +0200
From:   Andrey Konovalov <andreyknvl@...gle.com>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Alexander Potapenko <glider@...gle.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Christoph Lameter <cl@...ux.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Mark Rutland <mark.rutland@....com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Marc Zyngier <marc.zyngier@....com>,
        Dave Martin <dave.martin@....com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        Ingo Molnar <mingo@...nel.org>,
        Paul Lawrence <paullawrence@...gle.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Arnd Bergmann <arnd@...db.de>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kate Stewart <kstewart@...uxfoundation.org>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        kasan-dev <kasan-dev@...glegroups.com>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linux-sparse@...r.kernel.org, Linux-MM <linux-mm@...ck.org>,
        "open list:KERNEL BUILD + fi..." <linux-kbuild@...r.kernel.org>,
        Kostya Serebryany <kcc@...gle.com>,
        Evgeniy Stepanov <eugenis@...gle.com>,
        Lee Smith <Lee.Smith@....com>,
        Ramana Radhakrishnan <Ramana.Radhakrishnan@....com>,
        Jacob Bramley <Jacob.Bramley@....com>,
        Ruben Ayrapetyan <Ruben.Ayrapetyan@....com>,
        Jann Horn <jannh@...gle.com>,
        Mark Brand <markbrand@...gle.com>,
        Chintan Pandya <cpandya@...eaurora.org>,
        Vishwath Mohan <vishwath@...gle.com>
Subject: Re: [PATCH v6 15/18] khwasan, arm64: add brk handler for inline instrumentation

On Wed, Sep 12, 2018 at 7:13 PM, Dmitry Vyukov <dvyukov@...gle.com> wrote:
> On Wed, Aug 29, 2018 at 1:35 PM, Andrey Konovalov <andreyknvl@...gle.com> wrote:

>> +static int khwasan_handler(struct pt_regs *regs, unsigned int esr)
>> +{
>> +       bool recover = esr & KHWASAN_ESR_RECOVER;
>> +       bool write = esr & KHWASAN_ESR_WRITE;
>> +       size_t size = KHWASAN_ESR_SIZE(esr);
>> +       u64 addr = regs->regs[0];
>> +       u64 pc = regs->pc;
>> +
>> +       if (user_mode(regs))
>> +               return DBG_HOOK_ERROR;
>> +
>> +       kasan_report(addr, size, write, pc);
>> +
>> +       /*
>> +        * The instrumentation allows to control whether we can proceed after
>> +        * a crash was detected. This is done by passing the -recover flag to
>> +        * the compiler. Disabling recovery allows to generate more compact
>> +        * code.
>> +        *
>> +        * Unfortunately disabling recovery doesn't work for the kernel right
>> +        * now. KHWASAN reporting is disabled in some contexts (for example when
>> +        * the allocator accesses slab object metadata; same is true for KASAN;
>> +        * this is controlled by current->kasan_depth). All these accesses are
>> +        * detected by the tool, even though the reports for them are not
>> +        * printed.
>
>
> I am not following this part.
> Slab accesses metadata. OK.
> This is detected as bad access. OK.
> Report is not printed. OK.
> We skip BRK and resume execution.
> What is the problem?

When the kernel is compiled with -fsanitize=kernel-hwaddress without
any additional flags (like it's done now with KASAN_HW) everything
works as you described and there's no problem. However if one were to
recompile the kernel with hwasan recovery disabled, KHWASAN wouldn't
work due to the reasons described in the comment. Should I make it
more clear?

>
>
>
>> +        *
>> +        * This is something that might be fixed at some point in the future.
>> +        */
>> +       if (!recover)
>> +               die("Oops - KHWASAN", regs, 0);