linux-hardening - Re: [PATCH v9 5/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 for Android: free password hash cracker in your pocket

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

Message-ID: <202512121725.9CC129F0@keescook>
Date: Fri, 12 Dec 2025 17:40:50 -0800
From: Kees Cook <kees@...nel.org>
To: Andrew Pinski <andrew.pinski@....qualcomm.com>
Cc: Qing Zhao <qing.zhao@...cle.com>, Uros Bizjak <ubizjak@...il.com>,
	Joseph Myers <josmyers@...hat.com>,
	Richard Biener <rguenther@...e.de>,
	Jeff Law <jeffreyalaw@...il.com>, Andrew Pinski <pinskia@...il.com>,
	Jakub Jelinek <jakub@...hat.com>, Martin Uecker <uecker@...raz.at>,
	Peter Zijlstra <peterz@...radead.org>,
	Ard Biesheuvel <ardb@...nel.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>,
	"Osterlund, Sebastian" <sebastian.osterlund@...el.com>,
	"Constable, Scott D" <scott.d.constable@...el.com>,
	gcc-patches@....gnu.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v9 5/7] aarch64: Add AArch64 Kernel Control Flow
 Integrity implementation

On Fri, Dec 12, 2025 at 02:47:57PM -0800, Andrew Pinski wrote:
> On Tue, Dec 9, 2025 at 6:23 PM Kees Cook <kees@...nel.org> wrote:
> > Implement AArch64-specific KCFI backend.
> [...]
> 
> From an aarch64 point of view this is ok (except for a few minor
> things [see below in the patch itself] which can be bundled up with
> the fixes for the review of the middle-end parts and don't need an
> extra approval).

Thanks! I will go through these and fix them up for v10.

> The only open question with the aarch64 side of things (and other
> targets) is how to represent the kfci calls, should there be wrapping
> or not.
> Right now I am ok with the wrapping but I am not a fan of it because
> it introduces complexity to the call patterns. It is definitely
> something that should be looked into but I am not going to block this
> set of patches for it.
> And using define_subst/define_subst_attr if we decide to stay with the
> wrapping with the kfci rtl, is something to look into but that can
> wait too.

Yeah, I would love more guidance on this; Uros suggested using
define_subst during the x86 review, and I was able to use it there
because x86's expansion and call patterns don't use PARALLEL. When I
tried to apply define_subst to the other architectures with PARALLEL it
got extremely complex, and ended up being more complicated than what I
already had. See this for my (brief) comments on this attempt in v7:
https://lore.kernel.org/linux-hardening/20251117201219.makes.617-kees@kernel.org/

Maybe I just didn't see the right way to do it, so I'm happy to try
something different here if there's a better method.

> [...]
> > +const char *
> > +aarch64_output_kcfi_insn (rtx_insn *insn, rtx *operands)
> > +{
> > +  /* Target register is operands[0].  */
> > +  rtx target_reg = operands[0];
> > +  gcc_assert (REG_P (target_reg));
> > +
> > +  /* Get KCFI type ID from operand[3].  */
> > +  uint32_t type_id = (uint32_t) INTVAL (operands[3]);
> 
> Use UINTVAL .

Now fixed (for all archs).

> > +
> > +  /* Calculate typeid offset from call target.  */
> > +  HOST_WIDE_INT offset = -kcfi_get_typeid_offset ();
> > +
> > +  /* Get unique label number for this KCFI check.  */
> > +  int labelno = kcfi_next_labelno ();
> > +
> > +  /* Generate custom label names.  */
> > +  char trap_name[32];
> > +  char call_name[32];
> > +  ASM_GENERATE_INTERNAL_LABEL (trap_name, "Lkcfi_trap", labelno);
> > +  ASM_GENERATE_INTERNAL_LABEL (call_name, "Lkcfi_call", labelno);
> > +
> > +  /* Load actual type into w16 from memory at offset using ldur.  */
> > +  rtx temp_operands[2];
> > +  temp_operands[0] = target_reg;
> > +  temp_operands[1] = GEN_INT (offset);
> > +  output_asm_insn ("ldur\tw16, [%0, #%1]", temp_operands);
> > +
> > +  /* Load expected type into w17 using mov/movk sequence.  */
> > +  fprintf (asm_out_file, "\tmov\tw17, #%u\n", type_id & 0xFFFF);
> > +  fprintf (asm_out_file, "\tmovk\tw17, #%u, lsl #16\n", (type_id >> 16) & 0xFFFF);
> > +
> > +  /* Compare types.  */
> > +  fprintf (asm_out_file, "\tcmp\tw16, w17\n");
> > +
> > +  /* 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);
> > +
> > +  /* Output trap label and BRK instruction.  */
> > +  ASM_OUTPUT_LABEL (asm_out_file, trap_name);
> > +
> > +  /* Calculate and emit BRK with ESR encoding.  */
> > +  unsigned type_index = R17_REGNUM;
> > +  unsigned addr_index = REGNO (target_reg) - R0_REGNUM;
> > +  unsigned esr_value = 0x8000 | ((type_index & 31) << 5) | (addr_index & 31);
> > +
> > +  fprintf (asm_out_file, "\tbrk\t#%u\n", esr_value);
> 
> It might be useful to print the hex instead of the decimal here.
> Either way is ok.

Yeah, that's more human-readable. Now changed.

-Kees

-- 
Kees Cook