Re: security/libgcrypt: Sprinkle some ENDBR64 instructions
No need for an apology. You understand that stuff much better than I do anyway :-) Clueless OK for me. — Antoine > On Feb 22, 2024, at 09:57, Mark Kettenis wrote: > > >> >> From: Renato Aguiar >> Date: Wed, 21 Feb 2024 17:31:41 -0800 > > Apologies to Antoine, I forgot to CC you the first time I sent this > out. Anyway, here is a new version. Thinking about this a bit more > changing CFI_STARTPROC like we did on arm64 will make maintenance a > lot easier. This will over-BTI, but I'm also looking at having the > linker remove unnecessary ENDBR64 instructions at the start of a > function. > > ok? > >> On Tue, Feb 20 2024, Mark Kettenis wrote: >> >> I probably could have done this by changing CFI_STARTPROC, like >> on >> arm64. But that would "over-BTI" and there is a benefit in >> trying to >> avoid that on amd64. >> >> Let me know what you think. > > Index: security/libgcrypt/Makefile > === > RCS file: /cvs/ports/security/libgcrypt/Makefile,v > retrieving revision 1.93 > diff -u -p -r1.93 Makefile > --- security/libgcrypt/Makefile20 Nov 2023 16:53:17 -1.93 > +++ security/libgcrypt/Makefile22 Feb 2024 13:57:00 - > @@ -6,7 +6,7 @@ USE_NOEXECONLY=Yes > COMMENT=crypto library based on code used in GnuPG > > DISTNAME=libgcrypt-1.10.3 > -REVISION=0 > +REVISION=1 > > CATEGORIES=security > > Index: security/libgcrypt/patches/patch-cipher_asm-common-amd64_h > === > RCS file: security/libgcrypt/patches/patch-cipher_asm-common-amd64_h > diff -N security/libgcrypt/patches/patch-cipher_asm-common-amd64_h > --- /dev/null1 Jan 1970 00:00:00 - > +++ security/libgcrypt/patches/patch-cipher_asm-common-amd64_h22 Feb 2024 > 13:57:00 - > @@ -0,0 +1,21 @@ > +Index: cipher/asm-common-amd64.h > +--- cipher/asm-common-amd64.h.orig > cipher/asm-common-amd64.h > +@@ -68,7 +68,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; endbr64 > + # define CFI_ENDPROC() .cfi_endproc > + # define CFI_REMEMBER_STATE() .cfi_remember_state > + # define CFI_RESTORE_STATE().cfi_restore_state > +@@ -136,7 +136,7 @@ > +DW_SLEB128_28BIT(rsp_offs) > + > + #else > +-# define CFI_STARTPROC() > ++# define CFI_STARTPROC() endbr64 > + # define CFI_ENDPROC() > + # define CFI_REMEMBER_STATE() > + # define CFI_RESTORE_STATE()
Re: security/libgcrypt: Sprinkle some ENDBR64 instructions
On February 22, 2024 7:56:45 PM GMT+01:00, Theo Buehler wrote: >On Thu, Feb 22, 2024 at 02:57:39PM +0100, Mark Kettenis wrote: >> > From: Renato Aguiar >> > Date: Wed, 21 Feb 2024 17:31:41 -0800 >> >> Apologies to Antoine, I forgot to CC you the first time I sent this >> out. Anyway, here is a new version. Thinking about this a bit more >> changing CFI_STARTPROC like we did on arm64 will make maintenance a >> lot easier. This will over-BTI, but I'm also looking at having the >> linker remove unnecessary ENDBR64 instructions at the start of a >> function. >> >> ok? > >I think this is preferable, especially if you are planning to amortize >the cost in lld. > >ok tb > oh I missed that there was a second version. ok with me too, we arrived at the same conclusion for arm64 after all.
Re: security/libgcrypt: Sprinkle some ENDBR64 instructions
On Thu, Feb 22, 2024 at 02:57:39PM +0100, Mark Kettenis wrote: > > From: Renato Aguiar > > Date: Wed, 21 Feb 2024 17:31:41 -0800 > > Apologies to Antoine, I forgot to CC you the first time I sent this > out. Anyway, here is a new version. Thinking about this a bit more > changing CFI_STARTPROC like we did on arm64 will make maintenance a > lot easier. This will over-BTI, but I'm also looking at having the > linker remove unnecessary ENDBR64 instructions at the start of a > function. > > ok? I think this is preferable, especially if you are planning to amortize the cost in lld. ok tb
Re: security/libgcrypt: Sprinkle some ENDBR64 instructions
On Tue, Feb 20, 2024 at 01:11:35PM +0100, Mark Kettenis wrote: > I probably could have done this by changing CFI_STARTPROC, like on > arm64. But that would "over-BTI" and there is a benefit in trying to > avoid that on amd64. > > Let me know what you think. Looks ok to me. Hard to tell if that covers everything but I think that's fine. If anything is missing people will complain and we fix it. > > > Index: security/libgcrypt/Makefile > === > RCS file: /cvs/ports/security/libgcrypt/Makefile,v > retrieving revision 1.93 > diff -u -p -r1.93 Makefile > --- security/libgcrypt/Makefile 20 Nov 2023 16:53:17 - 1.93 > +++ security/libgcrypt/Makefile 20 Feb 2024 11:27:18 - > @@ -6,7 +6,7 @@ USE_NOEXECONLY= Yes > COMMENT= crypto library based on code used in GnuPG > > DISTNAME=libgcrypt-1.10.3 > -REVISION=0 > +REVISION=1 > > CATEGORIES= security > > Index: security/libgcrypt/patches/patch-cipher_arcfour-amd64_S > === > RCS file: security/libgcrypt/patches/patch-cipher_arcfour-amd64_S > diff -N security/libgcrypt/patches/patch-cipher_arcfour-amd64_S > --- /dev/null 1 Jan 1970 00:00:00 - > +++ security/libgcrypt/patches/patch-cipher_arcfour-amd64_S 20 Feb 2024 > 11:27:18 - > @@ -0,0 +1,11 @@ > +Index: cipher/arcfour-amd64.S > +--- cipher/arcfour-amd64.S.orig > cipher/arcfour-amd64.S > +@@ -26,6 +26,7 @@ > + ELF(.type _gcry_arcfour_amd64,@function) > + _gcry_arcfour_amd64: > + CFI_STARTPROC() > ++endbr64 > + ENTER_SYSV_FUNC_PARAMS_0_4 > + push%rbp > + CFI_PUSH(%rbp) > Index: security/libgcrypt/patches/patch-cipher_blake2b-amd64-avx2_S > === > RCS file: > /cvs/ports/security/libgcrypt/patches/patch-cipher_blake2b-amd64-avx2_S,v > retrieving revision 1.1 > diff -u -p -r1.1 patch-cipher_blake2b-amd64-avx2_S > --- security/libgcrypt/patches/patch-cipher_blake2b-amd64-avx2_S 19 Jan > 2023 17:11:02 - 1.1 > +++ security/libgcrypt/patches/patch-cipher_blake2b-amd64-avx2_S 20 Feb > 2024 11:27:18 - > @@ -17,3 +17,11 @@ Index: cipher/blake2b-amd64-avx2.S > .align 64 > .globl _gcry_blake2b_transform_amd64_avx2 > ELF(.type _gcry_blake2b_transform_amd64_avx2,@function;) > +@@ -208,6 +210,7 @@ _gcry_blake2b_transform_amd64_avx2: > + * %rdx: num_blks > + */ > + CFI_STARTPROC(); > ++endbr64; > + > + vzeroupper; > + > Index: security/libgcrypt/patches/patch-cipher_blake2s-amd64-avx_S > === > RCS file: > /cvs/ports/security/libgcrypt/patches/patch-cipher_blake2s-amd64-avx_S,v > retrieving revision 1.1 > diff -u -p -r1.1 patch-cipher_blake2s-amd64-avx_S > --- security/libgcrypt/patches/patch-cipher_blake2s-amd64-avx_S 19 Jan > 2023 17:11:02 - 1.1 > +++ security/libgcrypt/patches/patch-cipher_blake2s-amd64-avx_S 20 Feb > 2024 11:27:18 - > @@ -18,3 +18,11 @@ Index: cipher/blake2s-amd64-avx.S > .align 64 > .globl _gcry_blake2s_transform_amd64_avx > ELF(.type _gcry_blake2s_transform_amd64_avx,@function;) > +@@ -192,6 +193,7 @@ _gcry_blake2s_transform_amd64_avx: > + * %rdx: num_blks > + */ > + CFI_STARTPROC(); > ++endbr64; > + > + vzeroupper; > + > Index: security/libgcrypt/patches/patch-cipher_blowfish-amd64_S > === > RCS file: security/libgcrypt/patches/patch-cipher_blowfish-amd64_S > diff -N security/libgcrypt/patches/patch-cipher_blowfish-amd64_S > --- /dev/null 1 Jan 1970 00:00:00 - > +++ security/libgcrypt/patches/patch-cipher_blowfish-amd64_S 20 Feb 2024 > 11:27:18 - > @@ -0,0 +1,51 @@ > +Index: cipher/blowfish-amd64.S > +--- cipher/blowfish-amd64.S.orig > cipher/blowfish-amd64.S > +@@ -166,6 +166,7 @@ _gcry_blowfish_amd64_do_encrypt: > + * %rdx: u32 *ret_xr > + */ > + CFI_STARTPROC(); > ++endbr64; > + ENTER_SYSV_FUNC_PARAMS_0_4 > + > + movl (%rdx), RX0d; > +@@ -197,6 +198,7 @@ _gcry_blowfish_amd64_encrypt_block: > + * %rdx: src > + */ > + CFI_STARTPROC(); > ++endbr64; > + ENTER_SYSV_FUNC_PARAMS_0_4 > + > + movq %rsi, %r10; > +@@ -225,6 +227,7 @@ _gcry_blowfish_amd64_decrypt_block: > + * %rdx: src > + */ > + CFI_STARTPROC(); > ++endbr64; > + ENTER_SYSV_FUNC_PARAMS_0_4 > + > + movq %rbp, %r11; > +@@ -413,6 +416,7 @@ _gcry_blowfish_amd64_ctr_enc: > + * %rcx: iv (big endian, 64bit) > + */ > + CFI_STARTPROC(); > ++endbr64; > + ENTER_SYSV_FUNC_PARAMS_0_4 > + > + pushq %rbp; > +@@ -483,6 +487,7 @@ _gcry_blowfish_amd64_cbc_dec: > + * %rcx: iv (64bit) > + */ > +
Re: security/libgcrypt: Sprinkle some ENDBR64 instructions
> From: Renato Aguiar > Date: Wed, 21 Feb 2024 17:31:41 -0800 Apologies to Antoine, I forgot to CC you the first time I sent this out. Anyway, here is a new version. Thinking about this a bit more changing CFI_STARTPROC like we did on arm64 will make maintenance a lot easier. This will over-BTI, but I'm also looking at having the linker remove unnecessary ENDBR64 instructions at the start of a function. ok? On Tue, Feb 20 2024, Mark Kettenis wrote: > I probably could have done this by changing CFI_STARTPROC, like > on > arm64. But that would "over-BTI" and there is a benefit in > trying to > avoid that on amd64. > > Let me know what you think. Index: security/libgcrypt/Makefile === RCS file: /cvs/ports/security/libgcrypt/Makefile,v retrieving revision 1.93 diff -u -p -r1.93 Makefile --- security/libgcrypt/Makefile 20 Nov 2023 16:53:17 - 1.93 +++ security/libgcrypt/Makefile 22 Feb 2024 13:57:00 - @@ -6,7 +6,7 @@ USE_NOEXECONLY= Yes COMMENT= crypto library based on code used in GnuPG DISTNAME= libgcrypt-1.10.3 -REVISION= 0 +REVISION= 1 CATEGORIES=security Index: security/libgcrypt/patches/patch-cipher_asm-common-amd64_h === RCS file: security/libgcrypt/patches/patch-cipher_asm-common-amd64_h diff -N security/libgcrypt/patches/patch-cipher_asm-common-amd64_h --- /dev/null 1 Jan 1970 00:00:00 - +++ security/libgcrypt/patches/patch-cipher_asm-common-amd64_h 22 Feb 2024 13:57:00 - @@ -0,0 +1,21 @@ +Index: cipher/asm-common-amd64.h +--- cipher/asm-common-amd64.h.orig cipher/asm-common-amd64.h +@@ -68,7 +68,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; endbr64 + # define CFI_ENDPROC() .cfi_endproc + # define CFI_REMEMBER_STATE() .cfi_remember_state + # define CFI_RESTORE_STATE().cfi_restore_state +@@ -136,7 +136,7 @@ + DW_SLEB128_28BIT(rsp_offs) + + #else +-# define CFI_STARTPROC() ++# define CFI_STARTPROC() endbr64 + # define CFI_ENDPROC() + # define CFI_REMEMBER_STATE() + # define CFI_RESTORE_STATE()
security/libgcrypt: Sprinkle some ENDBR64 instructions
I probably could have done this by changing CFI_STARTPROC, like on arm64. But that would "over-BTI" and there is a benefit in trying to avoid that on amd64. Let me know what you think. Index: security/libgcrypt/Makefile === RCS file: /cvs/ports/security/libgcrypt/Makefile,v retrieving revision 1.93 diff -u -p -r1.93 Makefile --- security/libgcrypt/Makefile 20 Nov 2023 16:53:17 - 1.93 +++ security/libgcrypt/Makefile 20 Feb 2024 11:27:18 - @@ -6,7 +6,7 @@ USE_NOEXECONLY= Yes COMMENT= crypto library based on code used in GnuPG DISTNAME= libgcrypt-1.10.3 -REVISION= 0 +REVISION= 1 CATEGORIES=security Index: security/libgcrypt/patches/patch-cipher_arcfour-amd64_S === RCS file: security/libgcrypt/patches/patch-cipher_arcfour-amd64_S diff -N security/libgcrypt/patches/patch-cipher_arcfour-amd64_S --- /dev/null 1 Jan 1970 00:00:00 - +++ security/libgcrypt/patches/patch-cipher_arcfour-amd64_S 20 Feb 2024 11:27:18 - @@ -0,0 +1,11 @@ +Index: cipher/arcfour-amd64.S +--- cipher/arcfour-amd64.S.orig cipher/arcfour-amd64.S +@@ -26,6 +26,7 @@ + ELF(.type _gcry_arcfour_amd64,@function) + _gcry_arcfour_amd64: + CFI_STARTPROC() ++ endbr64 + ENTER_SYSV_FUNC_PARAMS_0_4 + push%rbp + CFI_PUSH(%rbp) Index: security/libgcrypt/patches/patch-cipher_blake2b-amd64-avx2_S === RCS file: /cvs/ports/security/libgcrypt/patches/patch-cipher_blake2b-amd64-avx2_S,v retrieving revision 1.1 diff -u -p -r1.1 patch-cipher_blake2b-amd64-avx2_S --- security/libgcrypt/patches/patch-cipher_blake2b-amd64-avx2_S19 Jan 2023 17:11:02 - 1.1 +++ security/libgcrypt/patches/patch-cipher_blake2b-amd64-avx2_S20 Feb 2024 11:27:18 - @@ -17,3 +17,11 @@ Index: cipher/blake2b-amd64-avx2.S .align 64 .globl _gcry_blake2b_transform_amd64_avx2 ELF(.type _gcry_blake2b_transform_amd64_avx2,@function;) +@@ -208,6 +210,7 @@ _gcry_blake2b_transform_amd64_avx2: + *%rdx: num_blks + */ + CFI_STARTPROC(); ++endbr64; + + vzeroupper; + Index: security/libgcrypt/patches/patch-cipher_blake2s-amd64-avx_S === RCS file: /cvs/ports/security/libgcrypt/patches/patch-cipher_blake2s-amd64-avx_S,v retrieving revision 1.1 diff -u -p -r1.1 patch-cipher_blake2s-amd64-avx_S --- security/libgcrypt/patches/patch-cipher_blake2s-amd64-avx_S 19 Jan 2023 17:11:02 - 1.1 +++ security/libgcrypt/patches/patch-cipher_blake2s-amd64-avx_S 20 Feb 2024 11:27:18 - @@ -18,3 +18,11 @@ Index: cipher/blake2s-amd64-avx.S .align 64 .globl _gcry_blake2s_transform_amd64_avx ELF(.type _gcry_blake2s_transform_amd64_avx,@function;) +@@ -192,6 +193,7 @@ _gcry_blake2s_transform_amd64_avx: + *%rdx: num_blks + */ + CFI_STARTPROC(); ++endbr64; + + vzeroupper; + Index: security/libgcrypt/patches/patch-cipher_blowfish-amd64_S === RCS file: security/libgcrypt/patches/patch-cipher_blowfish-amd64_S diff -N security/libgcrypt/patches/patch-cipher_blowfish-amd64_S --- /dev/null 1 Jan 1970 00:00:00 - +++ security/libgcrypt/patches/patch-cipher_blowfish-amd64_S20 Feb 2024 11:27:18 - @@ -0,0 +1,51 @@ +Index: cipher/blowfish-amd64.S +--- cipher/blowfish-amd64.S.orig cipher/blowfish-amd64.S +@@ -166,6 +166,7 @@ _gcry_blowfish_amd64_do_encrypt: +* %rdx: u32 *ret_xr +*/ + CFI_STARTPROC(); ++ endbr64; + ENTER_SYSV_FUNC_PARAMS_0_4 + + movl (%rdx), RX0d; +@@ -197,6 +198,7 @@ _gcry_blowfish_amd64_encrypt_block: +* %rdx: src +*/ + CFI_STARTPROC(); ++ endbr64; + ENTER_SYSV_FUNC_PARAMS_0_4 + + movq %rsi, %r10; +@@ -225,6 +227,7 @@ _gcry_blowfish_amd64_decrypt_block: +* %rdx: src +*/ + CFI_STARTPROC(); ++ endbr64; + ENTER_SYSV_FUNC_PARAMS_0_4 + + movq %rbp, %r11; +@@ -413,6 +416,7 @@ _gcry_blowfish_amd64_ctr_enc: +* %rcx: iv (big endian, 64bit) +*/ + CFI_STARTPROC(); ++ endbr64; + ENTER_SYSV_FUNC_PARAMS_0_4 + + pushq %rbp; +@@ -483,6 +487,7 @@ _gcry_blowfish_amd64_cbc_dec: +* %rcx: iv (64bit) +*/ + CFI_STARTPROC(); ++ endbr64; + ENTER_SYSV_FUNC_PARAMS_0_4 + + pushq %rbp; +@@ -544,6 +549,7 @@ _gcry_blowfish_amd64_cfb_dec: +* %rcx: iv (64bit) +*/ + CFI_STARTPROC(); ++ endbr64; + ENTER_SYSV_FUNC_PARAMS_0_4 + + pushq %rbp; Index: security/libgcrypt/patches/patch-cipher_camellia-aesni-avx-amd64_S
