Date:   Thu, 12 Mar 2020 18:42:11 +0000
From:   Catalin Marinas <catalin.marinas@....com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Will Deacon <will@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Paul Elliott <paul.elliott@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Yu-cheng Yu <yu-cheng.yu@...el.com>,
        Amit Kachhap <amit.kachhap@....com>,
        Vincenzo Frascino <vincenzo.frascino@....com>,
        Marc Zyngier <maz@...nel.org>,
        Eugene Syromiatnikov <esyr@...hat.com>,
        Szabolcs Nagy <szabolcs.nagy@....com>,
        "H . J . Lu " <hjl.tools@...il.com>,
        Andrew Jones <drjones@...hat.com>,
        Kees Cook <keescook@...omium.org>,
        Arnd Bergmann <arnd@...db.de>, Jann Horn <jannh@...gle.com>,
        Richard Henderson <richard.henderson@...aro.org>,
        Kristina Martšenko <kristina.martsenko@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Florian Weimer <fweimer@...hat.com>,
        Sudakshina Das <sudi.das@....com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-arch@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v8 00/11] arm64: Branch Target Identification support

On Wed, Mar 11, 2020 at 05:25:56PM +0000, Mark Brown wrote:
> On Wed, Mar 11, 2020 at 04:28:58PM +0000, Catalin Marinas wrote:
> > On Tue, Mar 10, 2020 at 12:42:26PM +0000, Mark Brown wrote:
> > > Sorry, I realized thanks to Amit's off-list prompting that I was testing
> > > that I was verifying with the wrong kernel binary here (user error since
> > > it took me a while to sort out uprobes) so this isn't quite right - you
> > > can probe the landing pads with or without this series.
> 
> > Can we not change aarch64_insn_is_nop() to actually return true only for
> > NOP and ignore everything else in the hint space? We tend to re-use the
> > hint instructions for new things in the architecture, so I'd rather
> > white-list what we know we can safely probe than black-listing only some
> > of the hint instructions.
[...]
> > I haven't assessed the effort of doing the above (probably not a lot)
> > but as a short-term workaround we could add the BTI and PAC hint
> > instructions to the aarch64_insn_is_nop() (though my preferred option is
> > the white-list one).
> 
> The only thing I've seen in testing with just NOPs whitelisted is an
> inability to probe the PAC instructions which isn't the best user
> experience, especially since the effect is that the probes get silently
> ignored. This isn't extensive userspace testing though.  Adding
> whitelisting of the BTI and PAC hints would definitely be a safer as a
> first step though.  I can post either version?

I thought BTI and PAC are already whitelisted in mainline as they fall
into the hint space (by whitelisting I mean you can probe them).

I'm trying to understand how the BTI patches affect the current uprobes
support and what is needed. Executing BTI or PCI?SP out of line should
be fine as they don't generate a BTI exception (the BRK doesn't either,
just the normal debug exception).

I think (it needs checking) that BRK preserves the PSTATE.BTYPE in SPSR.
If we probe an instruction in a guarded page and then we single-step it
in a non-guarded page, we'll miss a potential BTI fault. Is this an
issue?

If we are to keep the BTI faulting behaviour, we'd need an additional
xol page, guarded, and to find a way to report the original probed
address of the fault rather than the xol page.

So, IIUC, we don't have an issue with the actual BTI or PACI?SP
instructions but rather the other instructions that would not fault with
the BTI support. While we should try to address this, I think the
important bit now is not to break the existing uprobes support when
running a binary with BTI enabled.

Have I missed anything?

-- 
Catalin

Powered by blists - more mailing lists