Re: [PATCH] lkdtm: cfi: add test for HW landing pad CFI

From: Mark Brown <broonie@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Kees Cook <keescook@chromium.org>,
	peterz@infradead.org
Subject: Re: [PATCH] lkdtm: cfi: add test for HW landing pad CFI
Date: Wed, 13 Jul 2022 17:13:57 +0100	[thread overview]
Message-ID: <Ys7vRTThFVNz7moO@sirena.org.uk> (raw)
In-Reply-To: <20220713151815.295520-1-mark.rutland@arm.com>

[-- Attachment #1.1: Type: text/plain, Size: 1582 bytes --]

On Wed, Jul 13, 2022 at 04:18:15PM +0100, Mark Rutland wrote:
> Some architectures have coarse-grained HW CFI schemes where indirect
> branches must target a "landing pad" instruction (e.g. BTI on arm64,
> ENDBR on x86). These prevent gadgetization of arbitrary portions of
> functions.

> Add a test which checks these work as expected.

> For example, on arm64 HW with BTI this should result in a BTI exception
> being taken:

> +/*
> + * This tries to call an indirect function with an address which is not a
> + * function entry point. This should be caught by architectures with "landing
> + * pad" instructions (e.g. BTI on arm64, or ENDBR on x86).
> + */
> +static void lkdtm_CFI_FORWARD_LANDING_PAD(void)
> +{
> +	void (*func)(int *);
> +
> +	func = (void *)((unsigned long)lkdtm_increment_void + 4);
> +
> +	pr_info("Calling gadget address ...\n");
> +	func(&called_count);
> +
> +	pr_err("FAIL: survived gadget function call!\n");
> +}

Incrementing the address by 4 here is the right number for arm64 and it
looks like it's also right for the x86_64 ENDBR64 instruction but are we
guaranteed that it'll do the right thing for other architectures,
especially those with variable length instructions - couldn't we just
get an illegal instruction exception due to ending up pointing at
something that isn't the start of an instruction even if CFI isn't
active?

Not sure that worrying about that at this point isn't making perfect the
enemy of good though, it could be dealt with later.  Perhaps just put
the offset behind a #define to make it a tiny bit more discoverable?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel