linux-kernel - RE: [PATCH 14/14] x86/fineibt: Add FineIBT+BHI mitigation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <PH7PR11MB7572CA5A7558B2B8F2BEAEE7BB4A2@PH7PR11MB7572.namprd11.prod.outlook.com>
Date: Mon, 28 Oct 2024 05:45:00 +0000
From: "Constable, Scott D" <scott.d.constable@...el.com>
To: Peter Zijlstra <peterz@...radead.org>, Josh Poimboeuf
	<jpoimboe@...nel.org>
CC: "x86@...nel.org" <x86@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Milburn, Alyssa" <alyssa.milburn@...el.com>,
	"joao@...rdrivepizza.com" <joao@...rdrivepizza.com>,
	"andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>,
	"jose.marchesi@...cle.com" <jose.marchesi@...cle.com>, "hjl.tools@...il.com"
	<hjl.tools@...il.com>, "ndesaulniers@...gle.com" <ndesaulniers@...gle.com>,
	"samitolvanen@...gle.com" <samitolvanen@...gle.com>, "nathan@...nel.org"
	<nathan@...nel.org>, "ojeda@...nel.org" <ojeda@...nel.org>, "kees@...nel.org"
	<kees@...nel.org>, "alexei.starovoitov@...il.com"
	<alexei.starovoitov@...il.com>
Subject: RE: [PATCH 14/14] x86/fineibt: Add FineIBT+BHI mitigation

> > On Fri, Sep 27, 2024 at 09:49:10PM +0200, Peter Zijlstra wrote:
> > > Due to FineIBT weakness, add an additional mitigation for BHI.
> > >
> > > Use the 5 bytes of the nop at -1 and the 4 byte poison to squirrel in a BHI mitigation.
> > >
> > > Relies on clang-cfi to emit an additional piece of magic in the kCFI pre-amble, identifying which function arguments are pointers.
> > >
> > > An additional u8 (next to the existing u32) is emitted like:
> > >
> > >	movl	0x12345678, %eax	// CFI hash
> > >	movb	0x12, %al		// CFI args
> > >
> > > This u8 is a bitmask, where BIT(n) indicates the n'th argument is a pointer, notably the 6 possible argument registers are:
> > >
> > >	rdi, rsi, rdx, rcx, r8 and r9
> > >
> > > Single bit can be inlined, while 2-4 bits have combinatoric stubs with the required magic in. Anything more will fall back to __bhi_args_all which additionally poisons %rsp for good measure, in case things overflowed to the stack.

Hi Peter,

Instead of:

	movl	0x12345678, %eax	// CFI hash
	movb	0x12, %al		// CFI args

Can we do this?
	
	movb	0x12, %al		// CFI args
	movl	0x12345678, %eax	// CFI hash

The latter ordering does not affect the kCFI hash's location, whereas the former ordering shifts the location of the kCFI hash backward by two bytes, which creates more work for the compiler.

Also, when I build LLVM and Linux with these patches, my kernel crashes. The root cause appears to be a call from bpf_dispatcher_nop_func:

   0xffffffff813e69f1 <+209>:   mov    %rbx,%rdi
   0xffffffff813e69f4 <+212>:   mov    $0x9b5328da,%r10d
   0xffffffff813e69fa <+218>:   sub    $0x10,%r11
   0xffffffff813e69fe <+222>:   nopl   0x0(%rax)
   0xffffffff813e6a02 <+226>:   call   *%r11     # This is the problematic call

to some function that I don't see in the kernel image, and that I suspect is being generated at runtime:

   0xffffffffa0000794:  int3                # The call instruction lands here...
   0xffffffffa0000795:  int3
   0xffffffffa0000796:  int3
   0xffffffffa0000797:  int3
   0xffffffffa0000798:  int3
   0xffffffffa0000799:  int3
   0xffffffffa000079a:  int3
   0xffffffffa000079b:  int3
   0xffffffffa000079c:  int3
   0xffffffffa000079d:  int3
   0xffffffffa000079e:  int3
   0xffffffffa000079f:  int3
   0xffffffffa00007a0:  int3
   0xffffffffa00007a1:  int3
   0xffffffffa00007a2:  int3
   0xffffffffa00007a3:  int3
   0xffffffffa00007a4:  endbr64                # ... but it should land here
   0xffffffffa00007a8:  sub    $0x9b5328da,%r10d
   0xffffffffa00007af:  je     0xffffffffa00007b3
   0xffffffffa00007b1:  ud2
   0xffffffffa00007b3:  cs cmovne %r10,%rdi
   0xffffffffa00007b8:  nopl   0x0(%rax,%rax,1)
   0xffffffffa00007bd:  nopl   (%rax)
   0xffffffffa00007c0:  push   %rbp
   0xffffffffa00007c1:  mov    %rsp,%rbp
   0xffffffffa00007c4:  endbr64

Regards,

Scott Constable

> On Fri, Sep 27, 2024 at 06:50:06PM -0700, Josh Poimboeuf wrote:
> > On Fri, Sep 27, 2024 at 09:49:10PM +0200, Peter Zijlstra wrote:
> > > @@ -1190,6 +1214,8 @@ static __init int cfi_parse_cmdline(char
> > >  			cfi_mode = CFI_KCFI;
> > >  		} else if (!strcmp(str, "fineibt")) {
> > >  			cfi_mode = CFI_FINEIBT;
> > > +		} else if (IS_ENABLED(CONFIG_X86_KERNEL_IBT_PLUS) && !strcmp(str, "fineibt+bhi")) {
> > > +			cfi_mode = CFI_FINEIBT_BHI;
> > >  		} else if (!strcmp(str, "norand")) {
> > >  			cfi_rand = false;
> > >  		} else {
> > 
> > Do we need to hook this in with bugs.c somehow so it skips the other 
> > BHI mitigations?
>
> Yeah.. those didn't exist when I started this code :-) But yeah, once we get to the point of doing this patch for real -- the compiler(s) have the required features implemented properly and everyrhing, this should be hooked up better.