linux-kernel - Re: [PATCH v7 3/9] arm64, kfence: enable KFENCE for ARM64

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: <20201104145601.GB7577@C02TD0UTHF1T.local>
Date:   Wed, 4 Nov 2020 14:56:01 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Marco Elver <elver@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Potapenko <glider@...gle.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Andy Lutomirski <luto@...nel.org>,
        Borislav Petkov <bp@...en8.de>,
        Catalin Marinas <catalin.marinas@....com>,
        Christoph Lameter <cl@...ux.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hillf Danton <hdanton@...a.com>,
        Ingo Molnar <mingo@...hat.com>, Jann Horn <jannh@...gle.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Jonathan Corbet <corbet@....net>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Jörn Engel <joern@...estorage.com>,
        Kees Cook <keescook@...omium.org>,
        Pekka Enberg <penberg@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        SeongJae Park <sjpark@...zon.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vlastimil Babka <vbabka@...e.cz>,
        Will Deacon <will@...nel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        kasan-dev <kasan-dev@...glegroups.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Memory Management List <linux-mm@...ck.org>
Subject: Re: [PATCH v7 3/9] arm64, kfence: enable KFENCE for ARM64

On Wed, Nov 04, 2020 at 03:23:48PM +0100, Marco Elver wrote:
> On Wed, 4 Nov 2020 at 14:06, Mark Rutland <mark.rutland@....com> wrote:
> > On Tue, Nov 03, 2020 at 06:58:35PM +0100, Marco Elver wrote:
> > There is one thing that I thing we should improve as a subsequent
> > cleanup, but I don't think that should block this as-is.
> >
> > > +#define KFENCE_SKIP_ARCH_FAULT_HANDLER "el1_sync"
> >
> > IIUC, the core kfence code is using this to figure out where to trace
> > from when there's a fault taken on an access to a protected page.
> 
> Correct.
> 
> > It would be better if the arch code passed the exception's pt_regs into
> > the kfence fault handler, and the kfence began the trace began from
> > there. That would also allow for dumping the exception registers which
> > can help with debugging (e.g. figuring out how the address was derived
> > when it's calculated from multiple source registers). That would also be
> > a bit more robust to changes in an architectures' exception handling
> > code.
> 
> Good idea, thanks. I guess there's no reason to not want to always
> skip to instruction_pointer(regs)?

I don't think we need the exception handling gunk in the trace, but note
that you'd need to use stack_trace_save_regs(regs, ...) directly, rather
than using stack_trace_save() and skipping based on
instruction_pointer(regs). Otherwise, if the fault was somewhere in an
exception handler, and we invoked the same function on the path to the
kfence fault handler we might cut the trace at the wrong point.

> In which case I can prepare a patch to make this change. If this
> should go into a v8, please let me know. But it'd be easier as a
> subsequent patch as you say, given it'll be easier to review and these
> patches are in -mm now.

I think it'd make more sense as a subsequent change, since it's liable
to need a cycle or two of review, and I don't think it should block the
rest of the series.

Thanks,
Mark.