Re: [PATCH v3 7/7] kcfi: Add regression test suite

[Kees Cook <kees@kernel.org>]

On Sat, Sep 13, 2025 at 04:51:21PM -0700, Andrew Pinski wrote:
> On Sat, Sep 13, 2025 at 4:36 PM Kees Cook <kees@kernel.org> wrote:
> > +/* Should have KCFI instrumentation for all indirect calls.  */
> > +
> > +/* x86_64: Complete KCFI check sequence should be present.  */
> > +/* { dg-final { scan-assembler {movl\t\$-?[0-9]+, %r1[01]d\n\taddl\t[^,]+, %r1[01]d\n\tje\t\.Lkcfi_call[0-9]+\n\.Lkcfi_trap[0-9]+:\n\tud2} { target x86_64-*-* } } } */
> > +
> > +/* AArch64: Complete KCFI check sequence should be present.  */
> > +/* { dg-final { scan-assembler {ldur\tw16, \[x[0-9]+, #-[0-9]+\]\n\tmov\tw17, #[0-9]+\n\tmovk\tw17, #[0-9]+, lsl #16\n\tcmp\tw16, w17\n\tb\.eq\t(\.Lkcfi_call[0-9]+)\n\.Lkcfi_trap[0-9]+:\n\tbrk\t#[0-9]+\n\1:\n\tblr\tx[0-9]+} { target aarch64*-*-* } } } */
> > +
> > +/* ARM 32-bit: Complete KCFI check sequence should be present with stack
> > +   spilling.  */
> > +/* { dg-final { scan-assembler {push\t\{r0, r1\}\n\tldr\tr0, \[r[0-9]+, #-[0-9]+\]\n\tmovw\tr1, #[0-9]+\n\tmovt\tr1, #[0-9]+\n\tcmp\tr0, r1\n\tpop\t\{r0, r1\}\n\tbeq\t\.Lkcfi_call[0-9]+\n\.Lkcfi_trap[0-9]+:\n\tudf\t#[0-9]+\n\.Lkcfi_call[0-9]+:\n\tblx\tr[0-9]+} { target arm32 } } } */
> > +
> > +/* RISC-V: Complete KCFI check sequence should be present.  */
> > +/* { dg-final { scan-assembler {lw\tt1, -4\([a-z0-9]+\)\n\tlui\tt2, [0-9]+\n\taddiw\tt2, t2, -?[0-9]+\n\tbeq\tt1, t2, \.Lkcfi_call[0-9]+\n\.Lkcfi_trap[0-9]+:\n\tebreak} { target riscv*-*-* } } } */
> > +
> > +/* Should have trap section with entries.  */
> > +/* { dg-final { scan-assembler {\.kcfi_traps} { target x86_64-*-* } } } */
> > +/* { dg-final { scan-assembler {\.kcfi_traps} { target riscv*-*-* } } } */
> > +
> > +/* AArch64 should NOT have trap section (uses brk immediate instead) */
> > +/* { dg-final { scan-assembler-not {\.kcfi_traps} { target aarch64*-*-* } } } */
> > +
> > +/* ARM 32-bit should NOT have trap section (uses udf immediate instead) */
> > +/* { dg-final { scan-assembler-not {\.kcfi_traps} { target arm32 } } } */
> 
> 
> I think it would be better to use check-function-bodies here rather
> than scan-assembler for the sequences. Maybe each target should have
> its own testcase rather than putting it all in one source.
> Plus I think the target testcase should be part of the target patch
> rather than its own patch to make it easier to review both things
> together. Because while I was reviewing the aarch64 part I was
> thinking where are the testcases for the aarch64 specific changes.

Ah yeah, that works. I spent some time scratching my head over how to
have it not drop labels, but I've gotten a bunch of these converted now.
Some constructs I left, especially "scan-assembler-not" tests for v4.
It's significantly more readable now! Thanks! :)

-Kees

-- 
Kees Cook