Add the ability to allocate memory from kfence and trigger a read after
free on that memory to validate that kfence is working properly. This is
used by ChromeOS integration tests to validate that kfence errors can be
collected on user devices and parsed properly.
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/misc/lkdtm/heap.c | 64 +++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
index 0ce4cbf6abda..608872bcc7e0 100644
--- a/drivers/misc/lkdtm/heap.c
+++ b/drivers/misc/lkdtm/heap.c
@@ -4,6 +4,7 @@
* page allocation and slab allocations.
*/
#include "lkdtm.h"
+#include <linux/kfence.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
#include <linux/sched.h>
@@ -132,6 +133,66 @@ static void lkdtm_READ_AFTER_FREE(void)
kfree(val);
}
+#if IS_ENABLED(CONFIG_KFENCE)
+static void lkdtm_KFENCE_READ_AFTER_FREE(void)
+{
+ int *base, val, saw;
+ unsigned long timeout, resched_after;
+ size_t len = 1024;
+ /*
+ * The slub allocator will use the either the first word or
+ * the middle of the allocation to store the free pointer,
+ * depending on configurations. Store in the second word to
+ * avoid running into the freelist.
+ */
+ size_t offset = sizeof(*base);
+
+ /*
+ * 100x the sample interval should be more than enough to ensure we get
+ * a KFENCE allocation eventually.
+ */
+ timeout = jiffies + msecs_to_jiffies(100 * kfence_sample_interval);
+ /*
+ * Especially for non-preemption kernels, ensure the allocation-gate
+ * timer can catch up: after @resched_after, every failed allocation
+ * attempt yields, to ensure the allocation-gate timer is scheduled.
+ */
+ resched_after = jiffies + msecs_to_jiffies(kfence_sample_interval);
+ do {
+ base = kmalloc(len, GFP_KERNEL);
+ if (!base) {
+ pr_err("FAIL: Unable to allocate kfence memory!\n");
+ return;
+ }
+
+ if (is_kfence_address(base)) {
+ val = 0x12345678;
+ base[offset] = val;
+ pr_info("Value in memory before free: %x\n", base[offset]);
+
+ kfree(base);
+
+ pr_info("Attempting bad read from freed memory\n");
+ saw = base[offset];
+ if (saw != val) {
+ /* Good! Poisoning happened, so declare a win. */
+ pr_info("Memory correctly poisoned (%x)\n", saw);
+ } else {
+ pr_err("FAIL: Memory was not poisoned!\n");
+ pr_expected_config_param(CONFIG_INIT_ON_FREE_DEFAULT_ON, "init_on_free");
+ }
+ return;
+ }
+
+ kfree(base);
+ if (time_after(jiffies, resched_after))
+ cond_resched();
+ } while (time_before(jiffies, timeout));
+
+ pr_err("FAIL: kfence memory never allocated!\n");
+}
+#endif
+
static void lkdtm_WRITE_BUDDY_AFTER_FREE(void)
{
unsigned long p = __get_free_page(GFP_KERNEL);
@@ -327,6 +388,9 @@ static struct crashtype crashtypes[] = {
CRASHTYPE(VMALLOC_LINEAR_OVERFLOW),
CRASHTYPE(WRITE_AFTER_FREE),
CRASHTYPE(READ_AFTER_FREE),
+#if IS_ENABLED(CONFIG_KFENCE)
+ CRASHTYPE(KFENCE_READ_AFTER_FREE),
+#endif
CRASHTYPE(WRITE_BUDDY_AFTER_FREE),
CRASHTYPE(READ_BUDDY_AFTER_FREE),
CRASHTYPE(SLAB_INIT_ON_ALLOC),
--
https://chromeos.dev
On Mon, Nov 27, 2023 at 03:49:45PM -0800, Stephen Boyd wrote:
> Add the ability to allocate memory from kfence and trigger a read after
> free on that memory to validate that kfence is working properly. This is
> used by ChromeOS integration tests to validate that kfence errors can be
> collected on user devices and parsed properly.
This looks really good; thanks for adding this!
>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/misc/lkdtm/heap.c | 64 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
> index 0ce4cbf6abda..608872bcc7e0 100644
> --- a/drivers/misc/lkdtm/heap.c
> +++ b/drivers/misc/lkdtm/heap.c
> @@ -4,6 +4,7 @@
> * page allocation and slab allocations.
> */
> #include "lkdtm.h"
> +#include <linux/kfence.h>
> #include <linux/slab.h>
> #include <linux/vmalloc.h>
> #include <linux/sched.h>
> @@ -132,6 +133,66 @@ static void lkdtm_READ_AFTER_FREE(void)
> kfree(val);
> }
>
> +#if IS_ENABLED(CONFIG_KFENCE)
I really try hard to avoid having tests disappear depending on configs,
and instead report the expected failure case (as you have). Can this be
built without the IS_ENABLED() tests?
--
Kees Cook
Adding kfence folks (will add on v2).
Quoting Kees Cook (2023-11-29 12:22:27)
> On Mon, Nov 27, 2023 at 03:49:45PM -0800, Stephen Boyd wrote:
> > Add the ability to allocate memory from kfence and trigger a read after
> > free on that memory to validate that kfence is working properly. This is
> > used by ChromeOS integration tests to validate that kfence errors can be
> > collected on user devices and parsed properly.
>
> This looks really good; thanks for adding this!
>
> >
> > Signed-off-by: Stephen Boyd <[email protected]>
> > ---
> > drivers/misc/lkdtm/heap.c | 64 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 64 insertions(+)
> >
> > diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
> > index 0ce4cbf6abda..608872bcc7e0 100644
> > --- a/drivers/misc/lkdtm/heap.c
> > +++ b/drivers/misc/lkdtm/heap.c
> > @@ -4,6 +4,7 @@
> > * page allocation and slab allocations.
> > */
> > #include "lkdtm.h"
> > +#include <linux/kfence.h>
> > #include <linux/slab.h>
> > #include <linux/vmalloc.h>
> > #include <linux/sched.h>
> > @@ -132,6 +133,66 @@ static void lkdtm_READ_AFTER_FREE(void)
> > kfree(val);
> > }
> >
> > +#if IS_ENABLED(CONFIG_KFENCE)
>
> I really try hard to avoid having tests disappear depending on configs,
> and instead report the expected failure case (as you have). Can this be
> built without the IS_ENABLED() tests?
>
We need IS_ENABLED() for the kfence_sample_interval variable. I suppose
if the config isn't set that variable can be assumed as zero and then
the timeout would hit immediately. We can either define the name
'kfence_sample_interval' as 0 in the header, or put an ifdef in the
function.
---8<---
diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
index 4f467d3972a6..574d0aa726dc 100644
--- a/drivers/misc/lkdtm/heap.c
+++ b/drivers/misc/lkdtm/heap.c
@@ -138,6 +138,14 @@ static void lkdtm_KFENCE_READ_AFTER_FREE(void)
int *base, val, saw;
unsigned long timeout, resched_after;
size_t len = 1024;
+ unsigned long interval;
+
+#ifdef CONFIG_KFENCE
+ interval = kfence_sample_interval;
+#else
+ interval = 0;
+#endif
+
/*
* The slub allocator will use the either the first word or
* the middle of the allocation to store the free pointer,
@@ -150,13 +158,13 @@ static void lkdtm_KFENCE_READ_AFTER_FREE(void)
* 100x the sample interval should be more than enough to ensure we get
* a KFENCE allocation eventually.
*/
- timeout = jiffies + msecs_to_jiffies(100 * kfence_sample_interval);
+ timeout = jiffies + msecs_to_jiffies(100 * interval);
/*
* Especially for non-preemption kernels, ensure the allocation-gate
* timer can catch up: after @resched_after, every failed allocation
* attempt yields, to ensure the allocation-gate timer is scheduled.
*/
- resched_after = jiffies + msecs_to_jiffies(kfence_sample_interval);
+ resched_after = jiffies + msecs_to_jiffies(interval);
do {
base = kmalloc(len, GFP_KERNEL);
if (!base) {
---8<----
diff --git a/include/linux/kfence.h b/include/linux/kfence.h
index 401af4757514..88100cc9caba 100644
--- a/include/linux/kfence.h
+++ b/include/linux/kfence.h
@@ -223,6 +223,8 @@ bool __kfence_obj_info(struct kmem_obj_info *kpp,
void *object, struct slab *sla
#else /* CONFIG_KFENCE */
+#define kfence_sample_interval (0)
+
static inline bool is_kfence_address(const void *addr) { return false; }
static inline void kfence_alloc_pool_and_metadata(void) { }
static inline void kfence_init(void) { }
On Wed, 29 Nov 2023 at 21:42, Stephen Boyd <[email protected]> wrote:
>
> Adding kfence folks (will add on v2).
>
> Quoting Kees Cook (2023-11-29 12:22:27)
> > On Mon, Nov 27, 2023 at 03:49:45PM -0800, Stephen Boyd wrote:
> > > Add the ability to allocate memory from kfence and trigger a read after
> > > free on that memory to validate that kfence is working properly. This is
> > > used by ChromeOS integration tests to validate that kfence errors can be
> > > collected on user devices and parsed properly.
> >
> > This looks really good; thanks for adding this!
> >
> > >
> > > Signed-off-by: Stephen Boyd <[email protected]>
> > > ---
> > > drivers/misc/lkdtm/heap.c | 64 +++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 64 insertions(+)
> > >
> > > diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
> > > index 0ce4cbf6abda..608872bcc7e0 100644
> > > --- a/drivers/misc/lkdtm/heap.c
> > > +++ b/drivers/misc/lkdtm/heap.c
> > > @@ -4,6 +4,7 @@
> > > * page allocation and slab allocations.
> > > */
> > > #include "lkdtm.h"
> > > +#include <linux/kfence.h>
> > > #include <linux/slab.h>
> > > #include <linux/vmalloc.h>
> > > #include <linux/sched.h>
> > > @@ -132,6 +133,66 @@ static void lkdtm_READ_AFTER_FREE(void)
> > > kfree(val);
> > > }
> > >
> > > +#if IS_ENABLED(CONFIG_KFENCE)
> >
> > I really try hard to avoid having tests disappear depending on configs,
> > and instead report the expected failure case (as you have). Can this be
> > built without the IS_ENABLED() tests?
> >
>
> We need IS_ENABLED() for the kfence_sample_interval variable. I suppose
> if the config isn't set that variable can be assumed as zero and then
> the timeout would hit immediately. We can either define the name
> 'kfence_sample_interval' as 0 in the header, or put an ifdef in the
> function.
I think it's fair to put it in the kfence header, so you don't need
the #ifdefs in the test code.
We didn't think anyone should depend on kfence_sample_interval outside
KFENCE code, but probably only tests would anyway.
> ---8<---
> diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c
> index 4f467d3972a6..574d0aa726dc 100644
> --- a/drivers/misc/lkdtm/heap.c
> +++ b/drivers/misc/lkdtm/heap.c
> @@ -138,6 +138,14 @@ static void lkdtm_KFENCE_READ_AFTER_FREE(void)
> int *base, val, saw;
> unsigned long timeout, resched_after;
> size_t len = 1024;
> + unsigned long interval;
> +
> +#ifdef CONFIG_KFENCE
> + interval = kfence_sample_interval;
> +#else
> + interval = 0;
> +#endif
> +
> /*
> * The slub allocator will use the either the first word or
> * the middle of the allocation to store the free pointer,
> @@ -150,13 +158,13 @@ static void lkdtm_KFENCE_READ_AFTER_FREE(void)
> * 100x the sample interval should be more than enough to ensure we get
> * a KFENCE allocation eventually.
> */
> - timeout = jiffies + msecs_to_jiffies(100 * kfence_sample_interval);
> + timeout = jiffies + msecs_to_jiffies(100 * interval);
> /*
> * Especially for non-preemption kernels, ensure the allocation-gate
> * timer can catch up: after @resched_after, every failed allocation
> * attempt yields, to ensure the allocation-gate timer is scheduled.
> */
> - resched_after = jiffies + msecs_to_jiffies(kfence_sample_interval);
> + resched_after = jiffies + msecs_to_jiffies(interval);
> do {
> base = kmalloc(len, GFP_KERNEL);
> if (!base) {
>
> ---8<----
> diff --git a/include/linux/kfence.h b/include/linux/kfence.h
> index 401af4757514..88100cc9caba 100644
> --- a/include/linux/kfence.h
> +++ b/include/linux/kfence.h
> @@ -223,6 +223,8 @@ bool __kfence_obj_info(struct kmem_obj_info *kpp,
> void *object, struct slab *sla
>
> #else /* CONFIG_KFENCE */
>
> +#define kfence_sample_interval (0)
> +
> static inline bool is_kfence_address(const void *addr) { return false; }
> static inline void kfence_alloc_pool_and_metadata(void) { }
> static inline void kfence_init(void) { }
Acked-by: Marco Elver <[email protected]>
FWIW, I've occasionally been using repeatedly invoked READ_AFTER_FREE
to test if KFENCE is working. Having a dedicated test like this seems
more reliable though.