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
| ||
|
Message-ID: <20210921162804.GD35846@C02TD0UTHF1T.local> Date: Tue, 21 Sep 2021 17:28:04 +0100 From: Mark Rutland <mark.rutland@....com> To: Ard Biesheuvel <ardb@...nel.org> Cc: Peter Zijlstra <peterz@...radead.org>, Frederic Weisbecker <frederic@...nel.org>, Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, LKML <linux-kernel@...r.kernel.org>, James Morse <james.morse@....com>, Quentin Perret <qperret@...gle.com>, Christophe Leroy <christophe.leroy@...roup.eu> Subject: Re: [PATCH 2/4] arm64: implement support for static call trampolines On Tue, Sep 21, 2021 at 05:55:11PM +0200, Ard Biesheuvel wrote: > On Tue, 21 Sept 2021 at 17:33, Mark Rutland <mark.rutland@....com> wrote: > > > > On Tue, Sep 21, 2021 at 04:44:56PM +0200, Ard Biesheuvel wrote: > > > On Tue, 21 Sept 2021 at 09:10, Peter Zijlstra <peterz@...radead.org> wrote: > ... > > > > > > > > So I like what Christophe did for PPC32: > > > > > > > > https://lkml.kernel.org/r/6ec2a7865ed6a5ec54ab46d026785bafe1d837ea.1630484892.git.christophe.leroy@csgroup.eu > > > > > > > > Where he starts with an unconditional jmp and uses that IFF the offset > > > > fits and only does the data load when it doesn't. Ard, woulnd't that > > > > also make sense on ARM64? I'm thinking most in-kernel function pointers > > > > would actually fit, it's just the module muck that gets to have too > > > > large pointers, no? > > > > > > > > > > Yeah, I'd have to page that back in. But it seems like the following > > > > > > bti c > > > <branch> > > > adrp x16, <literal> > > > ldr x16, [x16, ...] > > > br x16 > > > > > > with <branch> either set to 'b target' for the near targets, 'ret' for > > > the NULL target, and 'nop' for the far targets should work, and the > > > architecture permits patching branches into NOPs and vice versa > > > without special synchronization. > > > > I think so, yes. We can do sligntly better with an inline literal pool > > and a PC-relative LDR to fold the ADRP+LDR, e.g. > > > > .align 3 > > tramp: > > BTI C > > {B <func> | RET | NOP} > > LDR X16, 1f > > BR X16 > > 1: .quad <literal> > > > > Since that's in the .text, it's RO for regular accesses anyway. > > > > I tried to keep the literal in .rodata to avoid inadvertent gadgets > and/or anticipate exec-only mappings of .text, but that may be a bit > overzealous. I think that in practice the risk of gadgetisation is minimal, and having it inline means we only need to record a single address per trampoline, so there's less risk that we get the patching wrong. > > > But I must be missing something here, or why did we have that long > > > discussion before? > > > > I think the long discussion was because v2 had some more complex options > > (mostly due to trying to use ADRP+ADD) and atomicity/preemption issues > > meant we could only transition between some of those one-way, and it was > > subtle/complex: > > > > https://lore.kernel.org/linux-arm-kernel/20201028184114.6834-1-ardb@kernel.org/ > > > > Ah yes, I was trying to use ADRP/ADD to avoid the load, and this is > what created all the complexity. > > > For v3, that was all gone, but we didn't have a user. > > > > Since the common case *should* be handled by {B <func> | RET | NOP }, I > > reckon it's fine to have just that and the literal pool fallback (which > > I'll definitely need for the sorts of kernel I run when fuzzing, where > > the kernel Image itself can be 100s of MiBs). > > Ack. So I'll respin this along these lines. Sounds good! > Do we care deeply about the branch and the literal being transiently > out of sync? I don't think we care about the tranisent window, since even if we just patched a branch, a thread could be preempted immediately after the branch and sit around blocked for a while. So it's always necessary to either handle such threads taking stale branches, or to flip the branch such that this doesn't matter (e.g. done once at boot time). That said, I'd suggest that we always patch the literal, then patch the {B| RET | NOP}, so that outside of patch times those are consistent with one another and we can't accidentally get into a state were we use a stale/bogus target after multiple patches. We can align the trampoline such that we know it falls within a single page, so that we only need to map/unmap it once (and the cost of the extra STR will be far smaller than the map/unmap anyhow). Thanks, Mark.
Powered by blists - more mailing lists