linux-hardening - Re: [PATCH v3 4/7] aarch64: Add AArch64 Kernel Control Flow Integrity implementation

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] [day] [month] [year] [list]

Message-ID: <CA+=Sn1k=BFEPhWJFh075r+9dx0Fft6KHmnz64tKLRx0W=XsKew@mail.gmail.com>
Date: Sun, 14 Sep 2025 12:52:52 -0700
From: Andrew Pinski <pinskia@...il.com>
To: Kees Cook <kees@...nel.org>
Cc: Andrew Pinski <andrew.pinski@....qualcomm.com>, Qing Zhao <qing.zhao@...cle.com>, 
	Jakub Jelinek <jakub@...hat.com>, Martin Uecker <uecker@...raz.at>, Richard Biener <rguenther@...e.de>, 
	Joseph Myers <josmyers@...hat.com>, Peter Zijlstra <peterz@...radead.org>, Jan Hubicka <hubicka@....cz>, 
	Richard Earnshaw <richard.earnshaw@....com>, Richard Sandiford <richard.sandiford@....com>, 
	Marcus Shawcroft <marcus.shawcroft@....com>, Kyrylo Tkachov <kyrylo.tkachov@....com>, 
	Kito Cheng <kito.cheng@...il.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Andrew Waterman <andrew@...ive.com>, Jim Wilson <jim.wilson.gcc@...il.com>, 
	Dan Li <ashimida.1990@...il.com>, Sami Tolvanen <samitolvanen@...gle.com>, 
	Ramon de C Valle <rcvalle@...gle.com>, Joao Moreira <joao@...rdrivepizza.com>, 
	Nathan Chancellor <nathan@...nel.org>, Bill Wendling <morbo@...gle.com>, gcc-patches@....gnu.org, 
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH v3 4/7] aarch64: Add AArch64 Kernel Control Flow Integrity implementation

On Sun, Sep 14, 2025 at 12:45 PM Kees Cook <kees@...nel.org> wrote:
>
> On Sat, Sep 13, 2025 at 04:43:29PM -0700, Andrew Pinski wrote:
> > On Sat, Sep 13, 2025 at 4:28 PM Kees Cook <kees@...nel.org> wrote:
> > >
> > > Implement AArch64-specific KCFI backend.
> > >
> > > - Trap debugging through ESR (Exception Syndrome Register) encoding
> > >   in BRK instruction immediate values.
> > >
> > > - Scratch register allocation using w16/w17 (x16/x17) following
> > >   AArch64 procedure call standard for intra-procedure-call registers.
> >
> > How does this interact with BTI and sibcalls? Since for indirect
> > calls, x17 is already used for the address.
> > Why do you need/want to use a fixed register here for the load/compare
> > anyways? Why can't you use any free register?
>
> Ah, yeah, good point. I'm struggling with this on aarch32 too for Ard's
> suggestion about using an eor sequence. So, the problem I haven't been
> able to solve is that call instructions cannot have scratch register
> operands. Or rather, can't when there is register pressure like on
> aarch32, where a spill would be needed. This is in the LRA:
>
>   if (CALL_P (curr_insn))
>     no_output_reloads_p = true;
>
> And that does make perfect sense, there's no place (in the current
> design) to provide a place where the reload would happen. But I also
> can't let the kcfi check/call get split up arbitrarily.
>
> Is there some way I can convince LRA to let me do the restore manually?
>
> > > +const char *
> > > +aarch64_output_kcfi_insn (rtx_insn *insn, rtx *operands)
> > > +{
> > > +  /* KCFI is only supported in LP64 mode.  */
> > > +  if (TARGET_ILP32)
> > > +    {
> > > +      sorry ("%<-fsanitize=kcfi%> is not supported for %<-mabi=ilp32%>");
> >
> > You should reject -fsanitize=kcfi during option processing instead of
> > this late in the compilation.
>
> Where is best to do this on a per-arch basis?

In the case of aarch64, aarch64_override_options_internal looks like a
good place. There are already errors produced here when it comes to
incompatible options. Other targets will have a similar place too; I
was only reviewing the aarch64 specific changes here.

Thanks,
Andrew Pinski

>
> > > +  /* Get KCFI type ID from operand[3].  */
> > > +  uint32_t type_id = (uint32_t) INTVAL (operands[3]);
> >
> > Maybe an assert that `(int32_t)type_id == INTVAL (operands[3])`?
>
> Oh, hm, actually, I think I should be using UINTVAL instead?
>
> > > +  /* Load actual type into w16 from memory at offset using ldur.  */
> > > +  temp_operands[0] = gen_rtx_REG (SImode, R16_REGNUM);
> > > +  temp_operands[1] = target_reg;
> > > +  temp_operands[2] = GEN_INT (offset);
> > > +  output_asm_insn ("ldur\t%w0, [%1, #%2]", temp_operands);
> >
> > Since you are using a fixed register, you don't need the temp_operands[0] here.
> > Also what happens if target_reg is x16? Shouldn't there be an assert
> > on that here?
>
> Yeah, I need to solve the scratch register issue more generally.
>
> > > +  /* Output conditional branch to call label.  */
> > > +  fputs ("\tb.eq\t", asm_out_file);
> > > +  assemble_name (asm_out_file, call_name);
> > > +  fputc ('\n', asm_out_file);
> >
> > There has to be a better way of implementing this.
>
> I couldn't find one that would let me keep the custom label name. I'd
> love to have something better! :)
>
>
> --
> Kees Cook