From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.2 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, T_SCC_BODY_TEXT_LINE,URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 0E76D2924C for ; Wed, 14 Feb 2024 04:19:29 +0100 (CET) Received: (qmail 23579 invoked by uid 550); 14 Feb 2024 03:16:25 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 22513 invoked from network); 14 Feb 2024 03:16:25 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1707880754; x=1708485554; darn=lists.openwall.com; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=2I6ey/aIvEFPYlq2N+jNDX0hgIi1+GsU6rUtAtauHoQ=; b=WGMehZZKrnAd6n09ixqKSW1K3mC8LucHm8jWCwjNWCZ0uK+Pcc2Ckql30mJA6pvAit K9ZH6Bv1v1s2H1pwNvwNjxVCpFirJMuY332NwMDh/14HHWX0+EzDG96JiSUmkxyZFlbD iiDGT8O3BZBD3JFRGorQhPk1wlAqeU2BO+Zogp9bHGo08krJHUIz/ZbHRdnGsEd2xOIe GbnyNwVrG/YHMg+6sss8RokzLj9lX5mb2jy+aUDbS3dCbPK7XsDpN6LqgGPWYuP61zAt Q4UZd7fPYoMqMl2smudtdWGdNezzsOLk6/eJWuxKv3faXzzV43feeLSmRRiLGP0KA2gF QaJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707880754; x=1708485554; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=2I6ey/aIvEFPYlq2N+jNDX0hgIi1+GsU6rUtAtauHoQ=; b=oZuk5lKj7EwNEseK8ie7H3cmz5yl+/2NzxM/ulJLgo9DR3SUtcgjKoh8T/w23TrDag xL97BJf5yoYj/G/ggBefjjlfBEFRk23D+dIiTVdDjuOgQhUKPzo559prsTdRfcltPiYu K2ImM4x1AXXOTrDf7QpLiSphmdQBVIbhjyHghDb3cNbtS3pZTPAML3LQzJC6zYF+grMN CLTBKD5ingZfgbOw6Zc/sfcFAX01DeZBJ9aOWALBC2IJ4bMURiCyT1uZlJkcKDFvo8eO UYlBMXZL9mr6FdoXnzAbuZ1vPTuhjlvETy88/YhxtHyXhyU0BebuMP2qaeopQUlXck0E UYLg== X-Gm-Message-State: AOJu0Yx+N1MwDwY9vMIvx6JI5yDb3xQ69SCHKdUqD83yrrjIE/hdMITy 5NUi7r86koICx8nwf1g2NC0ynhJ2sbAqGLeUIuAuBDdY2ouZewsk0lFnoOYgYc49159MwUdu6r8 NW7mUq5v3hM0pKAT+FNyCtwEwMiLCm1ahUdB58A== X-Google-Smtp-Source: AGHT+IHIGTVp/k6IISK9wEFD1qYKAvNUngrT+5cmG6mFxdz7uYhRG6mNfD7iROpaAFK3Bf7NcEPM9Lp61sY9E2kWX18= X-Received: by 2002:a05:6512:53a:b0:511:5994:2c92 with SMTP id o26-20020a056512053a00b0051159942c92mr927010lfc.7.1707880754113; Tue, 13 Feb 2024 19:19:14 -0800 (PST) MIME-Version: 1.0 References: <20240212184236.GZ4163@brightrain.aerifal.cx> <20240212224657.GA4163@brightrain.aerifal.cx> <20240213020834.GB4163@brightrain.aerifal.cx> <20240214021925.GC4163@brightrain.aerifal.cx> In-Reply-To: <20240214021925.GC4163@brightrain.aerifal.cx> From: William Roberts Date: Tue, 13 Feb 2024 21:19:02 -0600 Message-ID: To: musl@lists.openwall.com Cc: Markus Wichmann , enh Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [musl] PAC/BTI Support on aarch64 On Tue, Feb 13, 2024 at 8:19=E2=80=AFPM Rich Felker wrote= : > > On Tue, Feb 13, 2024 at 06:51:47PM +0100, Markus Wichmann wrote: > > Am Tue, Feb 13, 2024 at 08:47:42AM -0600 schrieb William Roberts: > > > It should. Is there a known minimal tool chain requirement and I can = test? > > > > Typically the first C99 compiler or the first aarch64 compiler, > > whichever is younger. > > I think binutils is the relevant component, and that'd be whichever > version of binutils added aarch64. AFAICT 2.24 tagged Dec 2013 > > > > No, anywhere branches are allowed, a BTI instruction must be the firs= t > > > instruction. BTI is just a way for software to say, hey this is a > > > valid jump/branch > > > target, allow it. This reduces the amount of gadgets available to an > > > attacker, which > > > is why libc is such a juicy target, as it's in everything. A lot of > > > things static link it, > > > which effectively turns it off for the whole process. > > > > > > > So this means there must be a BTI instruction following every single BL > > instruction. > > I don't think so, you wouldn't want call sites to effectively become gadget locations. You want entry points marked, returns are handled with PAC, which goes hand in hand with BTI. As the PAC instruction can also be a landing pad. Looking at some generated ASM, I don't see BL's being marked= . Here's a decent doc BTW: - https://www.google.com/search?q=3Darm+introduction+to+bti&rlz=3D1C5GCEM= _enUS1088US1089&oq=3Darm+introduction+to+bti&gs_lcrp=3DEgZjaHJvbWUyBggAEEUY= OTIHCAEQIRigATIHCAIQIRigATIHCAMQIRifBTIHCAQQIRifBTIHCAUQIRifBdIBCDM2NTZqMGo= 3qAIAsAIA&sourceid=3Dchrome&ie=3DUTF-8#:~:text=3DArm%20Instruction%20Set,Ar= m%20Compiler%206 Essentially call points need a valid pac/bti instruction, if using pac, then there must be a validate before ret. landing pad with pointer auth (with the A key): pacisasp or hint #25 validate with autiasp or int #29 landing pad with pointer auth (with the B key): pacisasp or hint #hint #27 validate with autibsp or hint #31 landing pad BTI only: bti c or hint #34 The compilers set some defines so you know which key to use, but some projects just support the A key. To support other keys, you would need to go the route of conditional asm, but almost everyone just uses the A key, it's what's turned on by -mbranch-protection=3Dstandard (I think), easy to catch in the arm header and balk if someone sets the B key. > > But in the end this isn't that much different from endbr64 on the PC. > > Whatever happened to those patches, BTW? > > What is the situation on x86? Does it use the same kind of per-page > enforcement mode, or is it only global, requiring disabling it if any > DSO lacks support? Is the endbr64 opcode a guaranteed-safe nop on > older ISA levels, or does it need to be conditional? > > > > Yes, so the kernel will manage the EL1 register flag for this, and th= en > > > mprotect sets the PROT_BTI flag during dlopen(). > > > > Well, this is a novelty. This is the first time there will be an > > arch-specific flag in mmap()/mprotect() for the musl dynlinker. So far > > that code has been entirely portable. > > Can the flag be used at mmap time, or only in mprotect? It would be a > lot more efficient to do it as part of the mmap, but getting > visibility to the note to know you need it at mmap time seems > difficult and more costly than doing the mprotect later... > > I assume we would either add the code conditional on the existence of > a PROT_BTI macro (#ifdef) and define that to the corresponding thing > on other archs in the future, or abstract it with a new name in > arch/$ARCH/reloc.h defined in terms of whatever the arch provides so > as to be a little bit more naming-agnostic. > > It should not be #ifdef __aarch64__ or similar. > > > > It's important to note, that even when enabling the assembly code fil= es, if the > > > C level source is not built with -mbranch-protection=3Dstandard, the = feature will > > > remain off for the library. > > > > > > > Arch-specific compiler flags are not a problem; configure.sh can add > > those as needed. > > Yep, that's fine. Possibly a question of whether it should be on by > default or configurable, but if there's essentially no cost, > on-by-default seems fine. > > > > I can't think of anything like this offhand, but aarches may want to = add prot > > > flags to mprotect calls. > > > > That hasn't happened yet. Of course, this may be as simple as adding a > > static inline function. The fact that the important information is in a > > note section is yet another novelty, of course. So far, the important > > information (even arch-specific) has been contained in the dynamic > > section. > > Yes, that's gratuitously annoying. Ideally it would have been > somewhere easily accessible from the Ehdr so it's available at initial > mmap time... :/ > > > > it usually > > > #ifdef aarch64 > > > if (gnu_notes_bti_set && (prot & PROT_EXEC)) { > > > prot |=3D PROT_BTI; > > > else { > > > prot &=3D ~PROT_BTI; > > > } > > > #endif > > > > > > mprotect(..., prot); > > > > > > > So far we have managed to steer clear of conditional inclusion, and I > > think we should try to keep it that way. > > Yes. I think reloc.h should define a predicate macro (which may call a > static inline function if the predicate is complex) to check if a DSO > needs branch protection on its PROT_EXEC segments. > src/internal/dynlink.h could provide a default always-false one if > it's not defined. Then dynlink.c can just, when that predicate > evaluates true, loop thru the segments and mprotect any PROT_EXEC ones > to also have PROT_BTI or whatever. I was tinkering today, arch/generic has a bunch of empty files, just add an empty file and let arches add to it. This code is far from useful, but just clarifying that approach. diff --git a/arch/aarch64/mprot_arch.h b/arch/aarch64/mprot_arch.h new file mode 100644 index 00000000..32f7afc6 --- /dev/null +++ b/arch/aarch64/mprot_arch.h @@ -0,0 +1,13 @@ +#ifndef _MPROT_ARCH_H +#define _MPROT_ARCH_H + +static inline int do_mprot(int prot) { + if (prot & PROT_EXEC) + +#define MPROT_ARCH(prot) \ +do { + if (prot & PROT_EXEC) { \ + prot |=3D PROT_BTI; \ + } \ +} while(0) +#endif diff --git a/arch/generic/mprot_arch.h b/arch/generic/mprot_arch.h new file mode 100644 index 00000000..e69de29b diff --git a/ldso/dynlink.c b/ldso/dynlink.c index 324aa859..a9b2278a 100644 --- a/ldso/dynlink.c +++ b/ldso/dynlink.c @@ -22,6 +22,7 @@ #include "pthread_impl.h" #include "fork_impl.h" #include "dynlink.h" +#include "mprot_arch.h" static size_t ldso_page_size; #ifndef PAGE_SIZE @@ -851,7 +852,9 @@ static void *map_library(int fd, struct dso *dso) } for (i=3D0; ((size_t *)(base+dyn))[i]; i+=3D2) if (((size_t *)(base+dyn))[i]=3D=3DDT_TEXTREL) { - if (mprotect(map, map_len, PROT_READ|PROT_WRITE|PROT_EXEC) + int prot =3D PROT_READ|PROT_WRITE|PROT_EXEC; + MPROT_ARCH(prot); + if (mprotect(map, map_len, prot) && errno !=3D ENOSYS) goto error; break; > This remains very arch-agnostic and the code should be either directly > usable on other archs, or admit easy generalization if needed. > > Rich