> Date: Mon, 20 Nov 2023 11:55:43 +0100 > From: Tobias Heider <tobias.hei...@stusta.de> > Cc: Jeremie Courreges-Anglas <j...@wxcvbn.org>, ports@openbsd.org, > kette...@openbsd.org, ajacou...@openbsd.org > Content-Type: text/plain; charset=us-ascii > Content-Disposition: inline > > On Mon, Nov 20, 2023 at 11:37:17AM +0100, Mark Kettenis wrote: > > > From: Jeremie Courreges-Anglas <j...@wxcvbn.org> > > > Date: Mon, 20 Nov 2023 08:22:13 +0100 > > > > > > On Sun, Nov 19 2023, Tobias Heider <tobias.hei...@stusta.de> wrote: > > > > Diff below fixes libgcrypt/gnupg on my m2. > > > > > > > > CFI_STARTPROC() seemed like a good place to add the bti instructions > > > > since it is called in all the right places. > > > > > > It's not something that makes sense for upstream IMHO, but for ports > > > that use it, it's an obvious place for us to tuck in "endbr64" or > > > "bti c". (eg I've been tempted to use such macros in lang/ocaml) > > > > > > Using a macro named CFI_STARTPROC() can be considered either appropriate > > > or misleading, since the .cfi_* directives are about "Call Frame > > > Information", and endbr64/bti are about "Control-flow Integrity"... > > > > > > > If this is too hacky I'm also fine with adding explicit instructions > > > > everywhere or even a new macro. > > > > > > I'd say it's fine for our port. > > > > > > I have one doubt: wouldn't it look more correct to have "bti c" *after* > > > .cfi_startproc: > > > > > > # define CFI_STARTPROC() .cfi_startproc; bti c; > > > > > > since "bti c" is intended to be part of the function? > > > > yes! > > > > Sure, ok like this? > > Index: Makefile > =================================================================== > RCS file: /cvs/ports/security/libgcrypt/Makefile,v > retrieving revision 1.92 > diff -u -p -r1.92 Makefile > --- Makefile 15 Nov 2023 08:00:14 -0000 1.92 > +++ Makefile 20 Nov 2023 10:54:58 -0000 > @@ -6,6 +6,7 @@ USE_NOEXECONLY= Yes > COMMENT= crypto library based on code used in GnuPG > > DISTNAME= libgcrypt-1.10.3 > +REVISION= 0 > > CATEGORIES= security > > Index: patches/patch-cipher_asm-common-aarch64_h > =================================================================== > RCS file: patches/patch-cipher_asm-common-aarch64_h > diff -N patches/patch-cipher_asm-common-aarch64_h > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ patches/patch-cipher_asm-common-aarch64_h 20 Nov 2023 10:54:58 -0000 > @@ -0,0 +1,21 @@ > +Index: cipher/asm-common-aarch64.h > +--- cipher/asm-common-aarch64.h.orig > ++++ cipher/asm-common-aarch64.h > +@@ -45,7 +45,7 @@ > + > + #ifdef HAVE_GCC_ASM_CFI_DIRECTIVES > + /* CFI directives to emit DWARF stack unwinding information. */ > +-# define CFI_STARTPROC() .cfi_startproc > ++# define CFI_STARTPROC() .cfi_startproc; bti c;
I think you don't need the semicolon at the end. > + # define CFI_ENDPROC() .cfi_endproc > + # define CFI_REMEMBER_STATE() .cfi_remember_state > + # define CFI_RESTORE_STATE() .cfi_restore_state > +@@ -87,7 +87,7 @@ > + DW_SLEB128_28BIT(rsp_offs) > + > + #else > +-# define CFI_STARTPROC() > ++# define CFI_STARTPROC() bti c; Same here > + # define CFI_ENDPROC() > + # define CFI_REMEMBER_STATE() > + # define CFI_RESTORE_STATE() >
