linux-kernel - Re: [PATCH 11/14] llvm: kCFI pointer stuff

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: <CAADnVQK8s4N_W_BH5zPZ7V-NW9FRegK27Nk-67UqiJzCxrdtxQ@mail.gmail.com>
Date: Mon, 30 Sep 2024 09:59:11 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>, 
	alyssa.milburn@...el.com, scott.d.constable@...el.com, 
	Joao Moreira <joao@...rdrivepizza.com>, Andrew Cooper <andrew.cooper3@...rix.com>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, "Jose E. Marchesi" <jose.marchesi@...cle.com>, 
	"H.J. Lu" <hjl.tools@...il.com>, Nick Desaulniers <ndesaulniers@...gle.com>, 
	Sami Tolvanen <samitolvanen@...gle.com>, Nathan Chancellor <nathan@...nel.org>, ojeda@...nel.org, 
	Kees Cook <kees@...nel.org>
Subject: Re: [PATCH 11/14] llvm: kCFI pointer stuff

On Mon, Sep 30, 2024 at 1:27 AM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Sun, Sep 29, 2024 at 10:53:05AM -0700, Alexei Starovoitov wrote:
>
> > > +  // Extend the kCFI meta-data with a byte that has a bit set for each argument
> > > +  // register that contains a pointer. Specifically for x86_64, which has 6
> > > +  // argument registers:
> > > +  //
> > > +  //   bit0 - rdi
> > > +  //   bit1 - rsi
> > > +  //   bit2 - rdx
> > > +  //   bit3 - rcx
> > > +  //   bit4 - r8
> > > +  //   bit5 - r9
> > > +  //
> > > +  // bit6 will denote any pointer on stack (%rsp), and all 7 bits set will
> > > +  // indicate vararg or any other 'unknown' configuration. Leaving bit7 for
> > > +  // error states.
> > > +  //
> > > +  // XXX: should be conditional on some new x86_64 specific 'bhi' argument.
> > > +  EmitAndCountInstruction(MCInstBuilder(X86::MOV8ri)
> > > +                 .addReg(X86::AL)
> > > +                 .addImm(getKCFIPointerArgs(F)));
> >
> > If I'm reading this correctly it will be an 8-bit move which
> > doesn't clear upper bits.
> > If consumer is in assembly it's ok-ish,
> > but it's an argument to __bhi_args_foo functions,
> > so should be properly zero extended per call convention.
>
> These kCFI 'instructions' are never executed. Their sole purpose is to
> encode the immediates. They are instructions because they live in .text
> and having them this way makes disassemly work nicely. As such, we've
> taken to using the 1 byte move instruction to carry them with the least
> amounts of bytes.
>
> The consumer is the kernel instruction decoder, we take the immediate
> and use that.

I see... and after decoding imm bits in mov %al insn the kernel will
insert a call to corresponding __bhi_args_* stub that will use
cmovne on corresponding register(s) to sanitize the value?
That was difficult to grasp.
A design doc would have helped.

I wonder whether this whole complexity is worth it vs
always calling __bhi_args_all()