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

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

[Mark Rutland]

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:

| # echo CFI_FORWARD_LANDING_PAD > /sys/kernel/debug/provoke-crash/DIRECT
| lkdtm: Performing direct entry CFI_FORWARD_LANDING_PAD
| lkdtm: Calling gadget address ...
| Unhandled 64-bit el1h sync exception on CPU0, ESR 0x0000000034000002 -- BTI
| CPU: 0 PID: 152 Comm: bash Not tainted 5.19.0-rc6-00001-g1f3acfdc1799 #3
| Hardware name: linux,dummy-virt (DT)
| pstate: 60400809 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=-c)
| pc : lkdtm_increment_void+0x4/0x20
| lr : lkdtm_CFI_FORWARD_LANDING_PAD+0x30/0x48
| sp : ffff80000a763cf0
| x29: ffff80000a763cf0 x28: ffff0000042c1d80 x27: 0000000000000000
| x26: 0000000000000000 x25: 0000000000000000 x24: 0000000031a6fb80
| x23: ffff000002c2a0c0 x22: ffff80000a763df0 x21: 0000000000000018
| x20: ffff80000a231498 x19: ffff000004075000 x18: 0000000000000006
| x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007200720
| x14: 0720072007200720 x13: ffff800009f82ca8 x12: 0000000000000429
| x11: 0000000000000163 x10: ffff800009fdaca8 x9 : ffff800009f82ca8
| x8 : 00000000ffffefff x7 : ffff800009fdaca8 x6 : 80000000fffff000
| x5 : ffff00007fbcca08 x4 : 0000000000000000 x3 : 0000000000000000
| x2 : 0000000000000000 x1 : ffff8000088a9c94 x0 : ffff80000a334b70
| Kernel panic - not syncing: Unhandled exception
| CPU: 0 PID: 152 Comm: bash Not tainted 5.19.0-rc6-00001-g1f3acfdc1799 #3
| Hardware name: linux,dummy-virt (DT)
| Call trace:
|  dump_backtrace.part.0+0xcc/0xe0
|  show_stack+0x18/0x6c
|  dump_stack_lvl+0x64/0x80
|  dump_stack+0x18/0x34
|  panic+0x170/0x328
|  arm64_exit_nmi.isra.0+0x0/0x80
|  el1h_64_sync_handler+0x64/0xd0
|  el1h_64_sync+0x64/0x68
|  lkdtm_increment_void+0x4/0x20
|  lkdtm_do_action+0x24/0x30
|  direct_entry+0x160/0x174
|  full_proxy_write+0x60/0xbc
|  vfs_write+0xc4/0x2a0
|  ksys_write+0x70/0x104
|  __arm64_sys_write+0x20/0x2c
|  invoke_syscall+0x48/0x114
|  el0_svc_common.constprop.0+0xcc/0xec
|  do_el0_svc+0xa0/0xc0
|  el0_svc+0x2c/0x84
|  el0t_64_sync_handler+0x11c/0x150
|  el0t_64_sync+0x18c/0x190
| Kernel Offset: disabled
| CPU features: 0x000,0003a817,69a418cf
| Memory Limit: none

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 drivers/misc/lkdtm/cfi.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/misc/lkdtm/cfi.c b/drivers/misc/lkdtm/cfi.c
index 666a7f4bc137..7cfdda73f561 100644
--- a/drivers/misc/lkdtm/cfi.c
+++ b/drivers/misc/lkdtm/cfi.c
@@ -43,6 +43,23 @@ static void lkdtm_CFI_FORWARD_PROTO(void)
 	pr_expected_config(CONFIG_CFI_CLANG);
 }
 
+/*
+ * 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");
+}
+
 /*
  * This can stay local to LKDTM, as there should not be a production reason
  * to disable PAC && SCS.
@@ -177,6 +194,7 @@ static void lkdtm_CFI_BACKWARD(void)
 
 static struct crashtype crashtypes[] = {
 	CRASHTYPE(CFI_FORWARD_PROTO),
+	CRASHTYPE(CFI_FORWARD_LANDING_PAD),
 	CRASHTYPE(CFI_BACKWARD),
 };
 
-- 
2.30.2


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

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

[Mark Rutland]

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:

| # echo CFI_FORWARD_LANDING_PAD > /sys/kernel/debug/provoke-crash/DIRECT
| lkdtm: Performing direct entry CFI_FORWARD_LANDING_PAD
| lkdtm: Calling gadget address ...
| Unhandled 64-bit el1h sync exception on CPU0, ESR 0x0000000034000002 -- BTI
| CPU: 0 PID: 152 Comm: bash Not tainted 5.19.0-rc6-00001-g1f3acfdc1799 #3
| Hardware name: linux,dummy-virt (DT)
| pstate: 60400809 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=-c)
| pc : lkdtm_increment_void+0x4/0x20
| lr : lkdtm_CFI_FORWARD_LANDING_PAD+0x30/0x48
| sp : ffff80000a763cf0
| x29: ffff80000a763cf0 x28: ffff0000042c1d80 x27: 0000000000000000
| x26: 0000000000000000 x25: 0000000000000000 x24: 0000000031a6fb80
| x23: ffff000002c2a0c0 x22: ffff80000a763df0 x21: 0000000000000018
| x20: ffff80000a231498 x19: ffff000004075000 x18: 0000000000000006
| x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007200720
| x14: 0720072007200720 x13: ffff800009f82ca8 x12: 0000000000000429
| x11: 0000000000000163 x10: ffff800009fdaca8 x9 : ffff800009f82ca8
| x8 : 00000000ffffefff x7 : ffff800009fdaca8 x6 : 80000000fffff000
| x5 : ffff00007fbcca08 x4 : 0000000000000000 x3 : 0000000000000000
| x2 : 0000000000000000 x1 : ffff8000088a9c94 x0 : ffff80000a334b70
| Kernel panic - not syncing: Unhandled exception
| CPU: 0 PID: 152 Comm: bash Not tainted 5.19.0-rc6-00001-g1f3acfdc1799 #3
| Hardware name: linux,dummy-virt (DT)
| Call trace:
|  dump_backtrace.part.0+0xcc/0xe0
|  show_stack+0x18/0x6c
|  dump_stack_lvl+0x64/0x80
|  dump_stack+0x18/0x34
|  panic+0x170/0x328
|  arm64_exit_nmi.isra.0+0x0/0x80
|  el1h_64_sync_handler+0x64/0xd0
|  el1h_64_sync+0x64/0x68
|  lkdtm_increment_void+0x4/0x20
|  lkdtm_do_action+0x24/0x30
|  direct_entry+0x160/0x174
|  full_proxy_write+0x60/0xbc
|  vfs_write+0xc4/0x2a0
|  ksys_write+0x70/0x104
|  __arm64_sys_write+0x20/0x2c
|  invoke_syscall+0x48/0x114
|  el0_svc_common.constprop.0+0xcc/0xec
|  do_el0_svc+0xa0/0xc0
|  el0_svc+0x2c/0x84
|  el0t_64_sync_handler+0x11c/0x150
|  el0t_64_sync+0x18c/0x190
| Kernel Offset: disabled
| CPU features: 0x000,0003a817,69a418cf
| Memory Limit: none

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 drivers/misc/lkdtm/cfi.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/misc/lkdtm/cfi.c b/drivers/misc/lkdtm/cfi.c
index 666a7f4bc137..7cfdda73f561 100644
--- a/drivers/misc/lkdtm/cfi.c
+++ b/drivers/misc/lkdtm/cfi.c
@@ -43,6 +43,23 @@ static void lkdtm_CFI_FORWARD_PROTO(void)
 	pr_expected_config(CONFIG_CFI_CLANG);
 }
 
+/*
+ * 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");
+}
+
 /*
  * This can stay local to LKDTM, as there should not be a production reason
  * to disable PAC && SCS.
@@ -177,6 +194,7 @@ static void lkdtm_CFI_BACKWARD(void)
 
 static struct crashtype crashtypes[] = {
 	CRASHTYPE(CFI_FORWARD_PROTO),
+	CRASHTYPE(CFI_FORWARD_LANDING_PAD),
 	CRASHTYPE(CFI_BACKWARD),
 };
 
-- 
2.30.2

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

[Mark Brown]

[-- 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

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

[Mark Brown]

[-- Attachment #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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Mark Rutland]

On Wed, Jul 13, 2022 at 05:13:57PM +0100, Mark Brown wrote:
> 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?

How about I just add a comment for now? e.g.

	/*
	 * Skip past a landing pad instruction.
	 * On arm64 all instructions are 4-byte aligned, and x86's ENDBR is 4
	 * bytes.
	 */

Thanks,
Mark.

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

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

[Mark Rutland]

On Wed, Jul 13, 2022 at 05:13:57PM +0100, Mark Brown wrote:
> 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?

How about I just add a comment for now? e.g.

	/*
	 * Skip past a landing pad instruction.
	 * On arm64 all instructions are 4-byte aligned, and x86's ENDBR is 4
	 * bytes.
	 */

Thanks,
Mark.

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

[Mark Brown]

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

On Thu, Jul 14, 2022 at 12:31:54PM +0100, Mark Rutland wrote:
> On Wed, Jul 13, 2022 at 05:13:57PM +0100, Mark Brown wrote:

> > 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?

> How about I just add a comment for now? e.g.

> 	/*
> 	 * Skip past a landing pad instruction.
> 	 * On arm64 all instructions are 4-byte aligned, and x86's ENDBR is 4
> 	 * bytes.
> 	 */

I'd rather have something that will definitely either fail or skip the
test so it can't silently pass, if people need to look at the source to
discover this there's more chance they'll mistakenly think things are
working.  Something like

#ifdef __aarch64__
#define LANDING_PAD_SKIP 4
#elif defined (__x86_64__)
#define LANDING_PAD_SKIP 4
#endif

then #ifdef LANDING_PAD_SKIP around the test perhaps?

[-- 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

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On Thu, Jul 14, 2022 at 12:31:54PM +0100, Mark Rutland wrote:
> On Wed, Jul 13, 2022 at 05:13:57PM +0100, Mark Brown wrote:

> > 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?

> How about I just add a comment for now? e.g.

> 	/*
> 	 * Skip past a landing pad instruction.
> 	 * On arm64 all instructions are 4-byte aligned, and x86's ENDBR is 4
> 	 * bytes.
> 	 */

I'd rather have something that will definitely either fail or skip the
test so it can't silently pass, if people need to look at the source to
discover this there's more chance they'll mistakenly think things are
working.  Something like

#ifdef __aarch64__
#define LANDING_PAD_SKIP 4
#elif defined (__x86_64__)
#define LANDING_PAD_SKIP 4
#endif

then #ifdef LANDING_PAD_SKIP around the test perhaps?

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