[PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Hi Jason,

this mini series enhances compatibility of the wireguard-linux-compat
repo with grsecurity. I noticed some build breakage on our 4.14 kernel
which the following patches take care of.

Please apply!

Thanks,
Mathias


Mathias Krause (2):
  compat: better grsecurity compatibility
  curve25519-x86_64: solve register constraints with reserved registers

 src/compat/compat-asm.h                        | 4 ++--
 src/compat/compat.h                            | 4 ++--
 src/crypto/zinc/curve25519/curve25519-x86_64.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.20.1

[PATCH 1/2] compat: better grsecurity compatibility

[Mathias Krause]

grsecurity kernels tend to carry additional backports and changes, like
commit b60b87fc2996 ("netlink: add ethernet address policy types") or
the SYM_FUNC_* changes. RAP nowadays hooks the latter, therefore no
diversion to RAP_ENTRY is needed any more.

Instead of relying on the kernel version test, also test for the macros
we're about to define to not already be defined to account for these
additional changes in the grsecurity patch without breaking
compatibility to the older public ones.

Also test for CONFIG_PAX instead of RAP_PLUGIN for the timer API related
changes as these don't depend on the RAP plugin to be enabled but just a
PaX/grsecurity patch to be applied. While there is no preprocessor knob
for the latter, use CONFIG_PAX as this will likely be enabled in every
kernel that uses the patch.

Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
 src/compat/compat-asm.h | 4 ++--
 src/compat/compat.h     | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/compat/compat-asm.h b/src/compat/compat-asm.h
index fde21dabba4f..5bfdb9410933 100644
--- a/src/compat/compat-asm.h
+++ b/src/compat/compat-asm.h
@@ -22,7 +22,7 @@
 #endif
 
 /* PaX compatibility */
-#if defined(RAP_PLUGIN)
+#if defined(RAP_PLUGIN) && defined(RAP_ENTRY)
 #undef ENTRY
 #define ENTRY RAP_ENTRY
 #endif
@@ -51,7 +51,7 @@
 #undef pull
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(5, 4, 76) && !defined(ISCENTOS8S)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(5, 4, 76) && !defined(ISCENTOS8S) && !defined(SYM_FUNC_START)
 #define SYM_FUNC_START ENTRY
 #define SYM_FUNC_END ENDPROC
 #endif
diff --git a/src/compat/compat.h b/src/compat/compat.h
index b2041327d85c..da6912d871fa 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -830,7 +830,7 @@ static inline void skb_mark_not_on_list(struct sk_buff *skb)
 }
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 20, 0) && !defined(ISRHEL8)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 20, 0) && !defined(ISRHEL8) && !defined(NLA_POLICY_EXACT_LEN)
 #define NLA_POLICY_EXACT_LEN(_len) { .type = NLA_UNSPEC, .len = _len }
 #endif
 #if LINUX_VERSION_CODE < KERNEL_VERSION(5, 2, 0) && !defined(ISRHEL8)
@@ -1127,7 +1127,7 @@ static const struct header_ops ip_tunnel_header_ops = { .parse_protocol = ip_tun
 #undef __read_mostly
 #define __read_mostly
 #endif
-#if (defined(RAP_PLUGIN) || defined(CONFIG_CFI_CLANG)) && LINUX_VERSION_CODE < KERNEL_VERSION(4, 15, 0)
+#if (defined(CONFIG_PAX) || defined(CONFIG_CFI_CLANG)) && LINUX_VERSION_CODE < KERNEL_VERSION(4, 15, 0)
 #include <linux/timer.h>
 #define wg_expired_retransmit_handshake(a) wg_expired_retransmit_handshake(unsigned long timer)
 #define wg_expired_send_keepalive(a) wg_expired_send_keepalive(unsigned long timer)
-- 
2.20.1

[PATCH 2/2] curve25519-x86_64: solve register constraints with reserved registers

[Mathias Krause]

The register constraints for the inline assembly in fsqr() and fsqr2()
are pretty tight on what the compiler may assign to the remaining three
register variables. The clobber list only allows the following to be
used: RDI, RSI, RBP and R12. With RAP reserving R12 and a kernel having
CONFIG_FRAME_POINTER=y, claiming RBP, there are only two registers left
so the compiler rightfully complains about impossible constraints.

Provide alternatives that'll allow a memory reference for 'out' to solve
the allocation constraint dilemma for this configuration.

Signed-off-by: Mathias Krause <minipli@grsecurity.net>

---

Yes, the '+' constraint prefix doesn't need to be repeated for the
alternatives. In fact, it's invalid syntax to do so (see [1]). Also
"+rm" won't do the trick either, as in this case gcc still insists to
have a free register -- even if it would choose a memory operand in the
end.

[1] https://gcc.gnu.org/onlinedocs/gcc/Multi-Alternative.html#Multi-Alternative
---
 src/crypto/zinc/curve25519/curve25519-x86_64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/crypto/zinc/curve25519/curve25519-x86_64.c b/src/crypto/zinc/curve25519/curve25519-x86_64.c
index 79716c425b0c..67f55affcf88 100644
--- a/src/crypto/zinc/curve25519/curve25519-x86_64.c
+++ b/src/crypto/zinc/curve25519/curve25519-x86_64.c
@@ -581,7 +581,7 @@ static inline void fsqr(u64 *out, const u64 *f, u64 *tmp)
 		"  cmovc %%rdx, %%rax;"
 		"  add %%rax, %%r8;"
 		"  movq %%r8, 0(%0);"
-	: "+&r" (tmp), "+&r" (f), "+&r" (out)
+	: "+&r,&r" (tmp), "+&r,&r" (f), "+&r,m" (out)
 	:
 	: "%rax", "%rcx", "%rdx", "%r8", "%r9", "%r10", "%r11", "%rbx", "%r13", "%r14", "%r15", "memory", "cc"
 	);
@@ -743,7 +743,7 @@ static inline void fsqr2(u64 *out, const u64 *f, u64 *tmp)
 		"  cmovc %%rdx, %%rax;"
 		"  add %%rax, %%r8;"
 		"  movq %%r8, 32(%0);"
-	: "+&r" (tmp), "+&r" (f), "+&r" (out)
+	: "+&r,&r" (tmp), "+&r,&r" (f), "+&r,m" (out)
 	:
 	: "%rax", "%rcx", "%rdx", "%r8", "%r9", "%r10", "%r11", "%rbx", "%r13", "%r14", "%r15", "memory", "cc"
 	);
-- 
2.20.1

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

Hi Mathias,

Sorry for the delay in reviewing these. Thanks for that. I've merged
them with a trivial change. The constraint one is interesting; it
looks like with your change and those extra constraints it winds up
spilling rdi to the stack?

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Hi Jason,

Am 08.08.21 um 22:53 schrieb Jason A. Donenfeld:
> Hi Mathias,
> 
> Sorry for the delay in reviewing these. Thanks for that. I've merged
> them with a trivial change.

thanks!

>                             The constraint one is interesting; it
> looks like with your change and those extra constraints it winds up
> spilling rdi to the stack?

Indeed, looking, for example, at fsquare_times() (!RAP, FRAMEPOINTER=y),
it looks like gcc prefers to choose the constraint with a memory output
argument even if it could fulfill the register allocation request, i.e.
it could allocate (all remaining) 3 registers instead of only 2 but
chooses to do the latter nonetheless. However, for me it's %rsi that's
put on the stack. Also it's not spilled, but used as a variable -- the
stack frame increases by 8 bytes and the stack slot where %rsi is
written to is used as-is for operations in the end. But, again, this
differs from the original code that used to allocate three registers for
the inline assembly. The old code has two memory loads at the end and
one compare while the new one has only one -- directly encode into the
compare instruction. So, memory-wise, a net win?

Below is the diff of the disassembly of fsquare_times(), spare of
addresses to reduce clutter with comments below each hunk:

--- old.dis	2021-08-09 11:46:53.050680456 +0200
+++ new.dis	2021-08-09 11:48:18.816851059 +0200
@@ -6,10 +6,10 @@
 	push   %r13
 	push   %r12
 	push   %rbx
-	sub    $0x18,%rsp
-	mov    %rdi,-0x38(%rbp)
-	mov    %rdx,-0x40(%rbp)
-	mov    %ecx,-0x2c(%rbp)
+	sub    $0x20,%rsp
+	mov    %rdi,-0x40(%rbp)
+	mov    %rdx,-0x48(%rbp)
+	mov    %ecx,-0x30(%rbp)
 	mov    %rdx,%r12
 	mov    (%rsi),%rdx
 	mulx   0x8(%rsi),%r8,%r14

This first hunk is really only the increased stack frame and resulting
offset changes.

@@ -92,15 +92,14 @@
 	cmovb  %rdx,%rax
 	add    %rax,%r8
 	mov    %r8,(%r12)
-	mov    -0x2c(%rbp),%eax
+	mov    -0x30(%rbp),%eax
 	sub    $0x1,%eax
-	mov    %eax,-0x30(%rbp)
-	je     1f42 <fsquare_times+0x3a2>
+	mov    %eax,-0x34(%rbp)
+	je     1f3c <fsquare_times+0x39c>
 	xor    %r12d,%r12d
-	mov    %r12d,-0x2c(%rbp)     <-- no longer needed in new code
-	mov    -0x38(%rbp),%rsi
-	mov    -0x40(%rbp),%rdi
-	mov    %rsi,%r12             <-- see below
+	mov    -0x40(%rbp),%rsi
+	mov    -0x48(%rbp),%rdi
+	mov    %rsi,-0x30(%rbp)      <-- reg -> mem operand constraint
 	mov    (%rsi),%rdx
 	mulx   0x8(%rsi),%r8,%r14
 	xor    %r15,%r15

Offset changes again. Beside the one dropped instruction there's only
one real change ("mov %rsi,%r12" -> "mov %rsi,-0x30(%rbp)"), induced by
the memory constraint of the inline assembly, switching a register
operand to a memory location.

@@ -154,7 +153,7 @@
 	adcx   %rcx,%r14
 	mov    %r14,0x38(%rdi)
 	mov    %rdi,%rsi
-	mov    %r12,%rdi
+	mov    -0x30(%rbp),%rdi
 	mov    $0x26,%rdx
 	mulx   0x20(%rsi),%r8,%r13
 	xor    %rcx,%rcx

Load of the stack slot which used to be a register operation before.

@@ -182,12 +181,10 @@
 	cmovb  %rdx,%rax
 	add    %rax,%r8
 	mov    %r8,(%rdi)
-	addl   $0x1,-0x2c(%rbp)            <-- mem operation
-	mov    -0x30(%rbp),%ebx            <-- mem load
-	mov    -0x2c(%rbp),%eax            <-- mem load
-	cmp    %ebx,%eax                   <-- reg compare
-	jne    1d83 <fsquare_times+0x1e3>
-	add    $0x18,%rsp
+	add    $0x1,%r12d                  <-- reg operation
+	cmp    -0x34(%rbp),%r12d           <-- mem compare
+	jne    1d7f <fsquare_times+0x1df>
+	add    $0x20,%rsp
 	pop    %rbx
 	pop    %r12
 	pop    %r13

We switch one memory operation (addl) to a register one and spare the
memory loads for the compare by directly encoding one of the memory
operands into the compare instruction. IMHO a net win, as gcc now can
use a register for the loop condition variable, making the conditional
jump depend only on one memory operand, not two. But only a benchmark
can tell if it's really faster or even slower.


Thanks,
Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

Hey Mathias,

This resulted in kind of an interesting regression with old compilers
on old kernel versions when I backported this to
wireguard-linux-compat:
https://git.zx2c4.com/wireguard-linux-compat/commit/?id=8118c247a75ae95169f0a9a539dfc661ffda8bc5

The 25519 tests fail for 4.8.17, 4.7.10, 4.6.7, 4.5.7 with gcc 6:

https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.8.17-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.7.10-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.6.7-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.5.7-x86_64.log

But then they crash for 4.0.9, 3.19.8, 3.17.8 with gcc 5:

https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.0.9-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.19.8-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.17.8-x86_64.log

And also crash with 3.16.85, 3.15.10, 3.14.79, 3.12.74, 3.11.10 with gcc 4:

https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.16.85-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.15.10-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.14.79-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.13.11-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.12.74-x86_64.log
https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.11.10-x86_64.log

Any intuition about what might have happened?

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

On Fri, Dec 3, 2021 at 11:20 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> on old kernel versions when I backported this to

Er, nix the "when I backported this" part: you sent it originally for
compat and never for mainline.

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Hi Jason,

Am 03.12.21 um 23:20 schrieb Jason A. Donenfeld:
> This resulted in kind of an interesting regression with old compilers
> on old kernel versions when I backported this to
> wireguard-linux-compat:
> https://git.zx2c4.com/wireguard-linux-compat/commit/?id=8118c247a75ae95169f0a9a539dfc661ffda8bc5
> 
> The 25519 tests fail for 4.8.17, 4.7.10, 4.6.7, 4.5.7 with gcc 6:
> 
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.8.17-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.7.10-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.6.7-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.5.7-x86_64.log
> 
> But then they crash for 4.0.9, 3.19.8, 3.17.8 with gcc 5:
> 
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/4.0.9-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.19.8-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.17.8-x86_64.log
> 
> And also crash with 3.16.85, 3.15.10, 3.14.79, 3.12.74, 3.11.10 with gcc 4:
> 
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.16.85-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.15.10-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.14.79-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.13.11-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.12.74-x86_64.log
> https://build.wireguard.com/wireguard-linux-compat/e8db181d62467da6c476cf4ac21e13dd477612c8/3.11.10-x86_64.log
> 
> Any intuition about what might have happened?

Sorry to hear that. I didn't ran into such issues when doing the
backport and, in fact, trying to reproduce the selftest errors / crashes
failed so far on v4.8.17 with gcc 6.3 and 4.6.3:

[    0.137871] wireguard: chacha20 self-tests: pass
[    0.141106] wireguard: poly1305 self-tests: pass
[    0.141604] wireguard: chacha20poly1305 self-tests: pass
[    0.142309] wireguard: blake2s self-tests: pass
[    0.157012] wireguard: curve25519 self-tests: pass
[    0.157430] wireguard: allowedips self-tests: pass
[    0.158354] wireguard: nonce counter self-tests: pass
[    0.388426] wireguard: ratelimiter self-tests: pass
[    0.389045] wireguard: WireGuard 1.0.20210606 loaded. See
www.wireguard.com for information.
[    0.389874] wireguard: Copyright (C) 2015-2019 Jason A. Donenfeld
<Jason@zx2c4.com>. All Rights Reserved.

I'll try older kernels and see if they trigger. In case not, can you
send me the object files of a failing kernel?

Thanks,
Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

Hey Mathias,

I couldn't repro with a new kernel either. Only these old ones.

Here are some object files for the logs in the earlier message:
https://data.zx2c4.com/curve25519-miscompile-a87440f8-7d1e-4179-be83-c82b4636a448.tar.zst

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Am 06.12.21 um 15:48 schrieb Jason A. Donenfeld:
> I couldn't repro with a new kernel either. Only these old ones.
> 
> Here are some object files for the logs in the earlier message:
> https://data.zx2c4.com/curve25519-miscompile-a87440f8-7d1e-4179-be83-c82b4636a448.tar.zst

Just a heads-up, some of these seem to have been compiled with a recent
gcc which might not be what was intended?:

$ strings -fa curve25519-*.o | grep -i gcc
curve25519-3.11.10.o: GCC: (GNU) 4.9.4
curve25519-3.12.74.o: GCC: (Gentoo 11.2.0 p1) 11.2.0
curve25519-3.14.79.o: GCC: (Gentoo 11.2.0 p1) 11.2.0
curve25519-3.15.10.o: GCC: (GNU) 4.9.4
curve25519-3.16.85.o: GCC: (Gentoo 11.2.0 p1) 11.2.0
curve25519-3.17.8.o: GCC: (GNU) 5.5.0
curve25519-3.19.8.o: GCC: (GNU) 5.5.0
curve25519-4.0.9.o: GCC: (GNU) 5.5.0
curve25519-4.5.7.o: GCC: (GNU) 6.5.0
curve25519-4.6.7.o: GCC: (GNU) 6.5.0
curve25519-4.7.10.o: GCC: (GNU) 6.5.0
curve25519-4.8.17.o: GCC: (GNU) 6.5.0

Anyhow, now I'm able to reproduce it myself. Was a PEBCAK config failure.

I'll look into it and work on a fix!

Thanks,
Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

Hi Mathias,

Oh, you're right about recent gcc. That actually _is_ intended, yet
they still fail. It would seem, then, that the problem is not so much
gcc version as it is some kernel patch that never made it to these
ancient kernels.

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Hi Jason,

Am 06.12.21 um 17:27 schrieb Jason A. Donenfeld:
> Oh, you're right about recent gcc. That actually _is_ intended, yet
> they still fail. It would seem, then, that the problem is not so much
> gcc version as it is some kernel patch that never made it to these
> ancient kernels.

actually, it's the i/o constraints, they're wrong. 'out' is an input
operand but we specify it as an output one. Now this works when gcc
respects the "+" constraint, as in marking this operand as being read
and written, thereby implicitly requiring it to be initialized. But
looks like older gcc ignore that (at least when using alternatives) and
make the asm work on a stale 'out' operand, resulting in the selftest
failures and crashes you've seen.

The following change fixes it by putting 'out' to the input operand
list, where it really belongs to:

diff --git a/src/crypto/zinc/curve25519/curve25519-x86_64.c
b/src/crypto/zinc/curve25519/curve25519-x86_64.c
index 67f55affcf88..f26ed5d897ac 100644
--- a/src/crypto/zinc/curve25519/curve25519-x86_64.c
+++ b/src/crypto/zinc/curve25519/curve25519-x86_64.c
@@ -581,8 +581,8 @@ static inline void fsqr(u64 *out, const u64 *f, u64
*tmp)
                "  cmovc %%rdx, %%rax;"
                "  add %%rax, %%r8;"
                "  movq %%r8, 0(%0);"
-       : "+&r,&r" (tmp), "+&r,&r" (f), "+&r,m" (out)
-       :
+       : "+&r,&r" (tmp), "+&r,&r" (f)
+       : "r,m" (out)
        : "%rax", "%rcx", "%rdx", "%r8", "%r9", "%r10", "%r11", "%rbx",
"%r13", "%r14", "%r15", "memory", "cc"
        );
 }
@@ -743,8 +743,8 @@ static inline void fsqr2(u64 *out, const u64 *f, u64
*tmp)
                "  cmovc %%rdx, %%rax;"
                "  add %%rax, %%r8;"
                "  movq %%r8, 32(%0);"
-       : "+&r,&r" (tmp), "+&r,&r" (f), "+&r,m" (out)
-       :
+       : "+&r,&r" (tmp), "+&r,&r" (f)
+       : "r,m" (out)
        : "%rax", "%rcx", "%rdx", "%r8", "%r9", "%r10", "%r11", "%rbx",
"%r13", "%r14", "%r15", "memory", "cc"
        );
 }

We still need the early clobber constraint ("&") for 'tmp' and 'f' as
they are, in fact, written to early. But 'out' is only ever read, so can
be a normal input operand.

I'll create a proper patch and send it out tomorrow, if you don't beat
me to.


Thanks,
Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

Hi Mathias,

Nice detective work! I just loaded this up on the CI, so we'll see if
this does work across the board.

It sounds like the original code also had this bug -- the "r"(out)
should be in the output constraints, not in the input constraints,
right? Sounds like something I should report to the EverCrypt authors
and also fix upstream too then.

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

On Mon, Dec 6, 2021 at 7:55 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Nice detective work! I just loaded this up on the CI, so we'll see if
> this does work across the board.

Looks like https://git.zx2c4.com/wireguard-linux-compat/commit/?id=42c931dbccf9570f10a84e282daf79f385d51623
is all green on https://www.wireguard.com/build-status/ in the
wireguard-linux-compat category. Let me know if that commit looks okay
to you or if you want to adjust something about it. After that, I'll
cut a new compat snapshot release.

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Hi Jason,

Am 06.12.21 um 20:28 schrieb Jason A. Donenfeld:
> On Mon, Dec 6, 2021 at 7:55 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> Nice detective work! I just loaded this up on the CI, so we'll see if
>> this does work across the board.
> 
> Looks like https://git.zx2c4.com/wireguard-linux-compat/commit/?id=42c931dbccf9570f10a84e282daf79f385d51623
> is all green on https://www.wireguard.com/build-status/ in the
> wireguard-linux-compat category. Let me know if that commit looks okay
> to you or if you want to adjust something about it. After that, I'll
> cut a new compat snapshot release.

ah, you modified the original commit of mine. Yeah, that works too.
However, I'd add the following to the commit log to account for the
output to input operand move:

"""
Also make 'out' an input-only operand as it is only used as such. This
not only allows gcc to optimize its usage further, but also works around
older gcc versions, apparently failing to handle multiple alternatives
correctly, as in failing to initialize the 'out' operand with its input
value.
"""

Thanks,
Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

On Mon, Dec 6, 2021 at 9:54 PM Mathias Krause <minipli@grsecurity.net> wrote:
>
> Hi Jason,
>
> Am 06.12.21 um 20:28 schrieb Jason A. Donenfeld:
> > On Mon, Dec 6, 2021 at 7:55 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >> Nice detective work! I just loaded this up on the CI, so we'll see if
> >> this does work across the board.
> >
> > Looks like https://git.zx2c4.com/wireguard-linux-compat/commit/?id=42c931dbccf9570f10a84e282daf79f385d51623
> > is all green on https://www.wireguard.com/build-status/ in the
> > wireguard-linux-compat category. Let me know if that commit looks okay
> > to you or if you want to adjust something about it. After that, I'll
> > cut a new compat snapshot release.
>
> ah, you modified the original commit of mine. Yeah, that works too.
> However, I'd add the following to the commit log to account for the
> output to input operand move:
>
> """
> Also make 'out' an input-only operand as it is only used as such. This
> not only allows gcc to optimize its usage further, but also works around
> older gcc versions, apparently failing to handle multiple alternatives
> correctly, as in failing to initialize the 'out' operand with its input
> value.
> """
>

Sure, will do.

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Am 06.12.21 um 19:55 schrieb Jason A. Donenfeld:
> Nice detective work! I just loaded this up on the CI, so we'll see if
> this does work across the board.

Thanks.

> It sounds like the original code also had this bug -- the "r"(out)
> should be in the output constraints, not in the input constraints,
> right? Sounds like something I should report to the EverCrypt authors
> and also fix upstream too then.

Yes, probably, but you're mixing up the two. "r"(out) should be an input
operand as it's only read in the inline asm. The other two ('tmp' and
'f') are read and written, so need to be output operands (as they
already are!) but also marked as earlyclobber ("&") as they're modified
before all input operands are read ('out', in this case). It's just a
hint to the compiler to not make the register allocation overlap with
any (other) input operand register.

Thanks,
Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

On Mon, Dec 6, 2021 at 10:00 PM Mathias Krause <minipli@grsecurity.net> wrote:
> Yes, probably, but you're mixing up the two.

Oh, thanks, right.

I'll talk to EverCrypt upstream and see.

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Am 08.12.21 um 15:56 schrieb Jason A. Donenfeld:
> On Mon, Dec 6, 2021 at 10:00 PM Mathias Krause <minipli@grsecurity.net> wrote:
>> Yes, probably, but you're mixing up the two.
> 
> Oh, thanks, right.
> 
> I'll talk to EverCrypt upstream and see.

FWIW, 'out' is also wrongly flagged as output operand in fmul() and
fmul2(). But making it an input operand needs more surgery, as the
operand order changes and this requires some code churn.

Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

CC'ing in Aymeric, who's working on Vale's codegen.

On Thu, Dec 9, 2021 at 8:59 AM Mathias Krause <minipli@grsecurity.net> wrote:
>
> Am 08.12.21 um 15:56 schrieb Jason A. Donenfeld:
> > On Mon, Dec 6, 2021 at 10:00 PM Mathias Krause <minipli@grsecurity.net> wrote:
> >> Yes, probably, but you're mixing up the two.
> >
> > Oh, thanks, right.
> >
> > I'll talk to EverCrypt upstream and see.
>
> FWIW, 'out' is also wrongly flagged as output operand in fmul() and
> fmul2(). But making it an input operand needs more surgery, as the
> operand order changes and this requires some code churn.
>
> Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

CC'ing in Aymeric, who's working on Vale's codegen.

On Thu, Dec 9, 2021 at 8:59 AM Mathias Krause <minipli@grsecurity.net> wrote:
>
> Am 08.12.21 um 15:56 schrieb Jason A. Donenfeld:
> > On Mon, Dec 6, 2021 at 10:00 PM Mathias Krause <minipli@grsecurity.net> wrote:
> >> Yes, probably, but you're mixing up the two.
> >
> > Oh, thanks, right.
> >
> > I'll talk to EverCrypt upstream and see.
>
> FWIW, 'out' is also wrongly flagged as output operand in fmul() and
> fmul2(). But making it an input operand needs more surgery, as the
> operand order changes and this requires some code churn.
>
> Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Aymeric Fromherz]

Thanks for the heads-up. We were being overly conservative during verification of inline assembly code in Vale, and marked several registers as possibly modified while they were only read.

This is now fixed for fmul, fmul2, fsqr and fsqr2, and will be merged into the master branch of EverCrypt shortly.
In the meantime, the diff for the resulting inline assembly after Vale codegen is available here: https://github.com/project-everest/hacl-star/pull/501/commits/1a71adb40c3f78da16e16975dbb1d4de5adeab8c#diff-5aabe9f6aa87508c9d81d4c9e89eff0b06b1e2aeaf5b04eba51da71c5bea6940

Cheers,
Aymeric

----- Mail original -----
> De: "Jason A. Donenfeld" <Jason@zx2c4.com>
> À: "Mathias Krause" <minipli@grsecurity.net>, "aymeric fromherz" <aymeric.fromherz@inria.fr>
> Cc: "WireGuard mailing list" <wireguard@lists.zx2c4.com>
> Envoyé: Vendredi 10 Décembre 2021 23:58:01
> Objet: Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

> CC'ing in Aymeric, who's working on Vale's codegen.
> 
> On Thu, Dec 9, 2021 at 8:59 AM Mathias Krause <minipli@grsecurity.net> wrote:
>>
>> Am 08.12.21 um 15:56 schrieb Jason A. Donenfeld:
>> > On Mon, Dec 6, 2021 at 10:00 PM Mathias Krause <minipli@grsecurity.net> wrote:
>> >> Yes, probably, but you're mixing up the two.
>> >
>> > Oh, thanks, right.
>> >
>> > I'll talk to EverCrypt upstream and see.
>>
>> FWIW, 'out' is also wrongly flagged as output operand in fmul() and
>> fmul2(). But making it an input operand needs more surgery, as the
>> operand order changes and this requires some code churn.
>>
> > Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

Hi Aymeric, Mathias,

On Sat, Dec 11, 2021 at 5:35 PM Aymeric Fromherz
<aymeric.fromherz@inria.fr> wrote:
>
> Thanks for the heads-up. We were being overly conservative during verification of inline assembly code in Vale, and marked several registers as possibly modified while they were only read.
>
> This is now fixed for fmul, fmul2, fsqr and fsqr2, and will be merged into the master branch of EverCrypt shortly.
> In the meantime, the diff for the resulting inline assembly after Vale codegen is available here: https://github.com/project-everest/hacl-star/pull/501/commits/1a71adb40c3f78da16e16975dbb1d4de5adeab8c#diff-5aabe9f6aa87508c9d81d4c9e89eff0b06b1e2aeaf5b04eba51da71c5bea6940

Thanks for that! I've imported this into my staging tree here:
https://w-g.pw/l/rmQK

I'll wait a few days -- and until the Vale PR has been merged -- to
submit that to the linux-crypto mailing list, but there's a preview.
Mathias - I wonder if you still need alternatives with the new codegen
there.

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Hi Jason,

Am 12.12.21 um 22:43 schrieb Jason A. Donenfeld:
> [snip]
>  
> Thanks for that! I've imported this into my staging tree here:
> https://w-g.pw/l/rmQK
> 
> I'll wait a few days -- and until the Vale PR has been merged -- to
> submit that to the linux-crypto mailing list, but there's a preview.
> Mathias - I wonder if you still need alternatives with the new codegen
> there.

yes, the alternative is still needed, as Aymeric's change doesn't lower
the number of required registers -- neither would a "rm" operand
constraint do, as gcc still insists in all constraints to be possible at
least, even if it would choose "m" instead of "r" in the end.

So, after importing Aymeric's changes into wireguard-linux-compat, the
alternatives change is still needed. I can help you create one, if you
want me to (we have that change in grsec anyway, just used named asm
operands here, as the %0/%1/%2/... was rather confusing and hard to follow).

Thanks,
Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

Hi Mathias,

On Mon, Dec 13, 2021 at 8:54 AM Mathias Krause <minipli@grsecurity.net> wrote:
> So, after importing Aymeric's changes into wireguard-linux-compat, the
> alternatives change is still needed. I can help you create one, if you
> want me to (we have that change in grsec anyway, just used named asm
> operands here, as the %0/%1/%2/... was rather confusing and hard to follow).

Sure, I'll take a patch for that. Let's wait until Aymeric
incorporates your suggestion on the out param, merges the PR, and then
I'll reimport that to wireguard-linux-compat, and you can send a
patch.

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

On Mon, Dec 13, 2021 at 12:36 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Mathias,
>
> On Mon, Dec 13, 2021 at 8:54 AM Mathias Krause <minipli@grsecurity.net> wrote:
> > So, after importing Aymeric's changes into wireguard-linux-compat, the
> > alternatives change is still needed. I can help you create one, if you
> > want me to (we have that change in grsec anyway, just used named asm
> > operands here, as the %0/%1/%2/... was rather confusing and hard to follow).
>
> Sure, I'll take a patch for that. Let's wait until Aymeric
> incorporates your suggestion on the out param, merges the PR, and then
> I'll reimport that to wireguard-linux-compat, and you can send a
> patch.

Actually, wasn't so bad, did it myself: https://w-g.pw/l/LjOY
Hopefully nothing to butchered in the process.

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Am 13.12.21 um 17:29 schrieb Jason A. Donenfeld:
> On Mon, Dec 13, 2021 at 12:36 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>>
>> Hi Mathias,
>>
>> On Mon, Dec 13, 2021 at 8:54 AM Mathias Krause <minipli@grsecurity.net> wrote:
>>> So, after importing Aymeric's changes into wireguard-linux-compat, the
>>> alternatives change is still needed. I can help you create one, if you
>>> want me to (we have that change in grsec anyway, just used named asm
>>> operands here, as the %0/%1/%2/... was rather confusing and hard to follow).
>>
>> Sure, I'll take a patch for that. Let's wait until Aymeric
>> incorporates your suggestion on the out param, merges the PR, and then
>> I'll reimport that to wireguard-linux-compat, and you can send a
>> patch.
> 
> Actually, wasn't so bad, did it myself: https://w-g.pw/l/LjOY
> Hopefully nothing to butchered in the process.

Looks good to me. Actually had to fetch the repo and use `git show
--color-words -w` to get a sensible signal-to-noise ratio ;)

Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Hi Aymeric,

the changes look good to me -- quite what we already had in grsec. Just
one more nit. The constraints for the 'out' parameter in fmul(),
fmul2(), fsqr() and fsqr2() can be further lifted to "rm" as 'out' is
only referenced once. This allows gcc to choose either a register or a
memory operand, as it sees fit. In our experiments the latter lead to
much better code gen.

Thanks,
Mathias

Am 11.12.21 um 17:35 schrieb Aymeric Fromherz:
> Thanks for the heads-up. We were being overly conservative during verification of inline assembly code in Vale, and marked several registers as possibly modified while they were only read.
> 
> This is now fixed for fmul, fmul2, fsqr and fsqr2, and will be merged into the master branch of EverCrypt shortly.
> In the meantime, the diff for the resulting inline assembly after Vale codegen is available here: https://github.com/project-everest/hacl-star/pull/501/commits/1a71adb40c3f78da16e16975dbb1d4de5adeab8c#diff-5aabe9f6aa87508c9d81d4c9e89eff0b06b1e2aeaf5b04eba51da71c5bea6940
> 
> Cheers,
> Aymeric
> 
> ----- Mail original -----
>> De: "Jason A. Donenfeld" <Jason@zx2c4.com>
>> À: "Mathias Krause" <minipli@grsecurity.net>, "aymeric fromherz" <aymeric.fromherz@inria.fr>
>> Cc: "WireGuard mailing list" <wireguard@lists.zx2c4.com>
>> Envoyé: Vendredi 10 Décembre 2021 23:58:01
>> Objet: Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches
> 
>> CC'ing in Aymeric, who's working on Vale's codegen.
>>
>> On Thu, Dec 9, 2021 at 8:59 AM Mathias Krause <minipli@grsecurity.net> wrote:
>>>
>>> Am 08.12.21 um 15:56 schrieb Jason A. Donenfeld:
>>>> On Mon, Dec 6, 2021 at 10:00 PM Mathias Krause <minipli@grsecurity.net> wrote:
>>>>> Yes, probably, but you're mixing up the two.
>>>>
>>>> Oh, thanks, right.
>>>>
>>>> I'll talk to EverCrypt upstream and see.
>>>
>>> FWIW, 'out' is also wrongly flagged as output operand in fmul() and
>>> fmul2(). But making it an input operand needs more surgery, as the
>>> operand order changes and this requires some code churn.
>>>
>>> Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Aymeric Fromherz]

Hi Mathias,

Thanks for the comments. Unfortunately, changing "r" to "rm" for the out parameter in a systematic way is quite trick.
Allowing arguments to be passed independently in a register or in memory is currently out of scope of the Vale model we use for verification.
We must decide early on in the verification pipeline if the argument is passed in a register, or if it stored in memory.
Doing this in a sound way would require a significant change to our (trusted) model.

Cheers,
Aymeric

----- Mail original -----
> De: "Mathias Krause" <minipli@grsecurity.net>
> À: "Aymeric Fromherz" <aymeric.fromherz@inria.fr>, "Jason A. Donenfeld" <Jason@zx2c4.com>
> Cc: "WireGuard mailing list" <wireguard@lists.zx2c4.com>
> Envoyé: Lundi 13 Décembre 2021 08:44:51
> Objet: Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

> Hi Aymeric,
> 
> the changes look good to me -- quite what we already had in grsec. Just
> one more nit. The constraints for the 'out' parameter in fmul(),
> fmul2(), fsqr() and fsqr2() can be further lifted to "rm" as 'out' is
> only referenced once. This allows gcc to choose either a register or a
> memory operand, as it sees fit. In our experiments the latter lead to
> much better code gen.
> 
> Thanks,
> Mathias
> 
> Am 11.12.21 um 17:35 schrieb Aymeric Fromherz:
>> Thanks for the heads-up. We were being overly conservative during verification
>> of inline assembly code in Vale, and marked several registers as possibly
>> modified while they were only read.
>> 
>> This is now fixed for fmul, fmul2, fsqr and fsqr2, and will be merged into the
>> master branch of EverCrypt shortly.
>> In the meantime, the diff for the resulting inline assembly after Vale codegen
>> is available here:
>> https://github.com/project-everest/hacl-star/pull/501/commits/1a71adb40c3f78da16e16975dbb1d4de5adeab8c#diff-5aabe9f6aa87508c9d81d4c9e89eff0b06b1e2aeaf5b04eba51da71c5bea6940
>> 
>> Cheers,
>> Aymeric
>> 
>> ----- Mail original -----
>>> De: "Jason A. Donenfeld" <Jason@zx2c4.com>
>>> À: "Mathias Krause" <minipli@grsecurity.net>, "aymeric fromherz"
>>> <aymeric.fromherz@inria.fr>
>>> Cc: "WireGuard mailing list" <wireguard@lists.zx2c4.com>
>>> Envoyé: Vendredi 10 Décembre 2021 23:58:01
>>> Objet: Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches
>> 
>>> CC'ing in Aymeric, who's working on Vale's codegen.
>>>
>>> On Thu, Dec 9, 2021 at 8:59 AM Mathias Krause <minipli@grsecurity.net> wrote:
>>>>
>>>> Am 08.12.21 um 15:56 schrieb Jason A. Donenfeld:
>>>>> On Mon, Dec 6, 2021 at 10:00 PM Mathias Krause <minipli@grsecurity.net> wrote:
>>>>>> Yes, probably, but you're mixing up the two.
>>>>>
>>>>> Oh, thanks, right.
>>>>>
>>>>> I'll talk to EverCrypt upstream and see.
>>>>
>>>> FWIW, 'out' is also wrongly flagged as output operand in fmul() and
>>>> fmul2(). But making it an input operand needs more surgery, as the
>>>> operand order changes and this requires some code churn.
>>>>
> >>> Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Hi Aymeric,

yeah, don't worry. We can keep this change downstream in grsec. Dunno if
Jason wants to take it as well, his call. It was just a side observation
which came out of our need to read and understand the code to provide a
workaround for the gcc bug we were hitting.

Thanks,
Mathias

Am 13.12.21 um 15:20 schrieb Aymeric Fromherz:
> Hi Mathias,
> 
> Thanks for the comments. Unfortunately, changing "r" to "rm" for the out parameter in a systematic way is quite trick.
> Allowing arguments to be passed independently in a register or in memory is currently out of scope of the Vale model we use for verification.
> We must decide early on in the verification pipeline if the argument is passed in a register, or if it stored in memory.
> Doing this in a sound way would require a significant change to our (trusted) model.
> 
> Cheers,
> Aymeric
> 
> ----- Mail original -----
>> De: "Mathias Krause" <minipli@grsecurity.net>
>> À: "Aymeric Fromherz" <aymeric.fromherz@inria.fr>, "Jason A. Donenfeld" <Jason@zx2c4.com>
>> Cc: "WireGuard mailing list" <wireguard@lists.zx2c4.com>
>> Envoyé: Lundi 13 Décembre 2021 08:44:51
>> Objet: Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches
> 
>> Hi Aymeric,
>>
>> the changes look good to me -- quite what we already had in grsec. Just
>> one more nit. The constraints for the 'out' parameter in fmul(),
>> fmul2(), fsqr() and fsqr2() can be further lifted to "rm" as 'out' is
>> only referenced once. This allows gcc to choose either a register or a
>> memory operand, as it sees fit. In our experiments the latter lead to
>> much better code gen.
>>
>> Thanks,
>> Mathias
>>
>> Am 11.12.21 um 17:35 schrieb Aymeric Fromherz:
>>> Thanks for the heads-up. We were being overly conservative during verification
>>> of inline assembly code in Vale, and marked several registers as possibly
>>> modified while they were only read.
>>>
>>> This is now fixed for fmul, fmul2, fsqr and fsqr2, and will be merged into the
>>> master branch of EverCrypt shortly.
>>> In the meantime, the diff for the resulting inline assembly after Vale codegen
>>> is available here:
>>> https://github.com/project-everest/hacl-star/pull/501/commits/1a71adb40c3f78da16e16975dbb1d4de5adeab8c#diff-5aabe9f6aa87508c9d81d4c9e89eff0b06b1e2aeaf5b04eba51da71c5bea6940
>>>
>>> Cheers,
>>> Aymeric
>>>
>>> ----- Mail original -----
>>>> De: "Jason A. Donenfeld" <Jason@zx2c4.com>
>>>> À: "Mathias Krause" <minipli@grsecurity.net>, "aymeric fromherz"
>>>> <aymeric.fromherz@inria.fr>
>>>> Cc: "WireGuard mailing list" <wireguard@lists.zx2c4.com>
>>>> Envoyé: Vendredi 10 Décembre 2021 23:58:01
>>>> Objet: Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches
>>>
>>>> CC'ing in Aymeric, who's working on Vale's codegen.
>>>>
>>>> On Thu, Dec 9, 2021 at 8:59 AM Mathias Krause <minipli@grsecurity.net> wrote:
>>>>>
>>>>> Am 08.12.21 um 15:56 schrieb Jason A. Donenfeld:
>>>>>> On Mon, Dec 6, 2021 at 10:00 PM Mathias Krause <minipli@grsecurity.net> wrote:
>>>>>>> Yes, probably, but you're mixing up the two.
>>>>>>
>>>>>> Oh, thanks, right.
>>>>>>
>>>>>> I'll talk to EverCrypt upstream and see.
>>>>>
>>>>> FWIW, 'out' is also wrongly flagged as output operand in fmul() and
>>>>> fmul2(). But making it an input operand needs more surgery, as the
>>>>> operand order changes and this requires some code churn.
>>>>>
>>>>> Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

On Mon, Dec 13, 2021 at 3:33 PM Mathias Krause <minipli@grsecurity.net> wrote:
> yeah, don't worry. We can keep this change downstream in grsec. Dunno if
> Jason wants to take it as well, his call. It was just a side observation
> which came out of our need to read and understand the code to provide a
> workaround for the gcc bug we were hitting.

I suppose I can load it up in kbench9000 to see if it makes a
difference. If it doesn't matter much, I'd prefer sticking with the
formally verified stuff. But if there is a nice speedup, then I guess
we can revisit more handwavy "obviously this is the same thing"
arguments.

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Am 13.12.21 um 15:37 schrieb Jason A. Donenfeld:
> On Mon, Dec 13, 2021 at 3:33 PM Mathias Krause <minipli@grsecurity.net> wrote:
>> yeah, don't worry. We can keep this change downstream in grsec. Dunno if
>> Jason wants to take it as well, his call. It was just a side observation
>> which came out of our need to read and understand the code to provide a
>> workaround for the gcc bug we were hitting.
> 
> I suppose I can load it up in kbench9000 to see if it makes a
> difference. If it doesn't matter much, I'd prefer sticking with the
> formally verified stuff. But if there is a nice speedup, then I guess
> we can revisit more handwavy "obviously this is the same thing"
> arguments.

Had to hack main.c and run.sh a little, but here are some numbers:

root@box:~# ./run.sh
[+] Setting no-turbo to status 1
[+] Setting non-boot CPUs to status 0
[+] Inserting module to run tests
insmod: ERROR: could not insert module kbench9000.ko: Unknown symbol in
module
[+] Gathering results
                 ever64: 115100 cycles per call
           ever64_out_r: 115080 cycles per call
          ever64_out_rm: 113957 cycles per call
[+] Setting non-boot CPUs to status 1
[+] Setting no-turbo to status 0

Slightly faster.

Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

Hi Mathias,

On Mon, Dec 13, 2021 at 5:32 PM Mathias Krause <minipli@grsecurity.net> wrote:
>
> Am 13.12.21 um 15:37 schrieb Jason A. Donenfeld:
> > On Mon, Dec 13, 2021 at 3:33 PM Mathias Krause <minipli@grsecurity.net> wrote:
> >> yeah, don't worry. We can keep this change downstream in grsec. Dunno if
> >> Jason wants to take it as well, his call. It was just a side observation
> >> which came out of our need to read and understand the code to provide a
> >> workaround for the gcc bug we were hitting.
> >
> > I suppose I can load it up in kbench9000 to see if it makes a
> > difference. If it doesn't matter much, I'd prefer sticking with the
> > formally verified stuff. But if there is a nice speedup, then I guess
> > we can revisit more handwavy "obviously this is the same thing"
> > arguments.
>
> Had to hack main.c and run.sh a little, but here are some numbers:
>
> root@box:~# ./run.sh
> [+] Setting no-turbo to status 1
> [+] Setting non-boot CPUs to status 0
> [+] Inserting module to run tests
> insmod: ERROR: could not insert module kbench9000.ko: Unknown symbol in
> module
> [+] Gathering results
>                  ever64: 115100 cycles per call
>            ever64_out_r: 115080 cycles per call
>           ever64_out_rm: 113957 cycles per call
> [+] Setting non-boot CPUs to status 1
> [+] Setting no-turbo to status 0
>
> Slightly faster.

Huh, that's actually a pretty nice speedup for just changing register
allocation... What CPU is this?

Jason

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Mathias Krause]

Am 13.12.21 um 17:33 schrieb Jason A. Donenfeld:
>> root@box:~# ./run.sh
>> [+] Setting no-turbo to status 1
>> [+] Setting non-boot CPUs to status 0
>> [+] Inserting module to run tests
>> insmod: ERROR: could not insert module kbench9000.ko: Unknown symbol in
>> module
>> [+] Gathering results
>>                  ever64: 115100 cycles per call
>>            ever64_out_r: 115080 cycles per call
>>           ever64_out_rm: 113957 cycles per call
>> [+] Setting non-boot CPUs to status 1
>> [+] Setting no-turbo to status 0
>>
>> Slightly faster.
> 
> Huh, that's actually a pretty nice speedup for just changing register
> allocation... What CPU is this?

It's an "Intel(R) Core(TM) i9-9900 CPU @ 3.10GHz" But shouldn't matter
all that much, as the inline ASM is integer ops only.

Mathias

Re: [PATCH 0/2] wireguard-linux-compat: grsecurity compat patches

[Jason A. Donenfeld]

On Mon, Dec 13, 2021 at 5:39 PM Mathias Krause <minipli@grsecurity.net> wrote:
>
> Am 13.12.21 um 17:33 schrieb Jason A. Donenfeld:
> >> root@box:~# ./run.sh
> >> [+] Setting no-turbo to status 1
> >> [+] Setting non-boot CPUs to status 0
> >> [+] Inserting module to run tests
> >> insmod: ERROR: could not insert module kbench9000.ko: Unknown symbol in
> >> module
> >> [+] Gathering results
> >>                  ever64: 115100 cycles per call
> >>            ever64_out_r: 115080 cycles per call
> >>           ever64_out_rm: 113957 cycles per call
> >> [+] Setting non-boot CPUs to status 1
> >> [+] Setting no-turbo to status 0
> >>
> >> Slightly faster.
> >
> > Huh, that's actually a pretty nice speedup for just changing register
> > allocation... What CPU is this?
>
> It's an "Intel(R) Core(TM) i9-9900 CPU @ 3.10GHz" But shouldn't matter
> all that much, as the inline ASM is integer ops only.

Yet it does. Here are results on a tigerlake, i7-11850H:

[+] Setting no-turbo to status 1
[+] Setting non-boot CPUs to status 0
[+] Inserting module to run tests
[+] Gathering results
    donna64: 141081 cycles per call
     hacl64: 135645 cycles per call
     fiat64: 132547 cycles per call
    sandy2x: 126933 cycles per call
precomp_bmi2: 120276 cycles per call
precomp_adx: 113578 cycles per call
     ever64: 106943 cycles per call
    ever64r: 105924 cycles per call
   ever64rm: 108139 cycles per call
     fiat32: 297854 cycles per call
    donna32: 467667 cycles per call
  tweetnacl: 1195385 cycles per call
[+] Setting non-boot CPUs to status 1
[+] Setting no-turbo to status 0

https://xn--4db.cc/c4B65RUy/diff

Looks like Tigerlake is able to make use of free stack accesses thanks
to the new "fast forwarding" feature.