2021-09-17 19:49:29

by Marco Elver

[permalink] [raw]
Subject: [PATCH 1/3] kfence: count unexpectedly skipped allocations

Maintain a counter to count allocations that are skipped due to being
incompatible (oversized, incompatible gfp flags) or no capacity.

This is to compute the fraction of allocations that could not be
serviced by KFENCE, which we expect to be rare.

Signed-off-by: Marco Elver <[email protected]>
---
mm/kfence/core.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/mm/kfence/core.c b/mm/kfence/core.c
index 7a97db8bc8e7..2755800f3e2a 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -112,6 +112,8 @@ enum kfence_counter_id {
KFENCE_COUNTER_FREES,
KFENCE_COUNTER_ZOMBIES,
KFENCE_COUNTER_BUGS,
+ KFENCE_COUNTER_SKIP_INCOMPAT,
+ KFENCE_COUNTER_SKIP_CAPACITY,
KFENCE_COUNTER_COUNT,
};
static atomic_long_t counters[KFENCE_COUNTER_COUNT];
@@ -121,6 +123,8 @@ static const char *const counter_names[] = {
[KFENCE_COUNTER_FREES] = "total frees",
[KFENCE_COUNTER_ZOMBIES] = "zombie allocations",
[KFENCE_COUNTER_BUGS] = "total bugs",
+ [KFENCE_COUNTER_SKIP_INCOMPAT] = "skipped allocations (incompatible)",
+ [KFENCE_COUNTER_SKIP_CAPACITY] = "skipped allocations (capacity)",
};
static_assert(ARRAY_SIZE(counter_names) == KFENCE_COUNTER_COUNT);

@@ -272,7 +276,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g
}
raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags);
if (!meta)
- return NULL;
+ goto no_capacity;

if (unlikely(!raw_spin_trylock_irqsave(&meta->lock, flags))) {
/*
@@ -289,7 +293,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g
list_add_tail(&meta->list, &kfence_freelist);
raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags);

- return NULL;
+ goto no_capacity;
}

meta->addr = metadata_to_pageaddr(meta);
@@ -349,6 +353,10 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g
atomic_long_inc(&counters[KFENCE_COUNTER_ALLOCS]);

return addr;
+
+no_capacity:
+ atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_CAPACITY]);
+ return NULL;
}

static void kfence_guarded_free(void *addr, struct kfence_metadata *meta, bool zombie)
@@ -740,8 +748,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags)
* Perform size check before switching kfence_allocation_gate, so that
* we don't disable KFENCE without making an allocation.
*/
- if (size > PAGE_SIZE)
+ if (size > PAGE_SIZE) {
+ atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]);
return NULL;
+ }

/*
* Skip allocations from non-default zones, including DMA. We cannot
@@ -749,8 +759,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags)
* properties (e.g. reside in DMAable memory).
*/
if ((flags & GFP_ZONEMASK) ||
- (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32)))
+ (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32))) {
+ atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]);
return NULL;
+ }

/*
* allocation_gate only needs to become non-zero, so it doesn't make
--
2.33.0.464.g1972c5931b-goog


2021-09-17 19:49:46

by Marco Elver

[permalink] [raw]
Subject: [PATCH 3/3] kfence: add note to documentation about skipping covered allocations

Add a note briefly mentioning the new policy about "skipping currently
covered allocations if pool close to full." Since this has a notable
impact on KFENCE's bug-detection ability on systems with large uptimes,
it is worth pointing out the feature.

Signed-off-by: Marco Elver <[email protected]>
---
Documentation/dev-tools/kfence.rst | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/Documentation/dev-tools/kfence.rst b/Documentation/dev-tools/kfence.rst
index 0fbe3308bf37..e698234999d6 100644
--- a/Documentation/dev-tools/kfence.rst
+++ b/Documentation/dev-tools/kfence.rst
@@ -269,6 +269,14 @@ tail of KFENCE's freelist, so that the least recently freed objects are reused
first, and the chances of detecting use-after-frees of recently freed objects
is increased.

+If pool utilization reaches 75% or above, to reduce the probability of the pool
+containing ~100% allocated objects yet ensure diverse coverage of allocations,
+KFENCE limits currently covered allocations of the same source from further
+filling up the pool. A side-effect is that this also limits frequent long-lived
+allocations of the same source filling up the pool permanently, thereby
+reducing the risk of the pool becoming full and the sampled allocation rate
+dropping to zero.
+
Interface
---------

--
2.33.0.464.g1972c5931b-goog

2021-09-17 21:07:10

by Dmitry Vyukov

[permalink] [raw]
Subject: Re: [PATCH 1/3] kfence: count unexpectedly skipped allocations

On Fri, 17 Sept 2021 at 13:08, 'Marco Elver' via kasan-dev
<[email protected]> wrote:
>
> Maintain a counter to count allocations that are skipped due to being
> incompatible (oversized, incompatible gfp flags) or no capacity.
>
> This is to compute the fraction of allocations that could not be
> serviced by KFENCE, which we expect to be rare.
>
> Signed-off-by: Marco Elver <[email protected]>
> ---
> mm/kfence/core.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 7a97db8bc8e7..2755800f3e2a 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -112,6 +112,8 @@ enum kfence_counter_id {
> KFENCE_COUNTER_FREES,
> KFENCE_COUNTER_ZOMBIES,
> KFENCE_COUNTER_BUGS,
> + KFENCE_COUNTER_SKIP_INCOMPAT,
> + KFENCE_COUNTER_SKIP_CAPACITY,
> KFENCE_COUNTER_COUNT,
> };
> static atomic_long_t counters[KFENCE_COUNTER_COUNT];
> @@ -121,6 +123,8 @@ static const char *const counter_names[] = {
> [KFENCE_COUNTER_FREES] = "total frees",
> [KFENCE_COUNTER_ZOMBIES] = "zombie allocations",
> [KFENCE_COUNTER_BUGS] = "total bugs",
> + [KFENCE_COUNTER_SKIP_INCOMPAT] = "skipped allocations (incompatible)",
> + [KFENCE_COUNTER_SKIP_CAPACITY] = "skipped allocations (capacity)",
> };
> static_assert(ARRAY_SIZE(counter_names) == KFENCE_COUNTER_COUNT);
>
> @@ -272,7 +276,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g
> }
> raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags);
> if (!meta)
> - return NULL;
> + goto no_capacity;
>
> if (unlikely(!raw_spin_trylock_irqsave(&meta->lock, flags))) {
> /*
> @@ -289,7 +293,7 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g
> list_add_tail(&meta->list, &kfence_freelist);
> raw_spin_unlock_irqrestore(&kfence_freelist_lock, flags);
>
> - return NULL;
> + goto no_capacity;

Do we expect this case to be so rare that we don't care?
Strictly speaking it's not no_capacity. So if I see large no_capacity
numbers, the first question I will have is: is it really no_capacity,
or some other case that we mixed together?



> }
>
> meta->addr = metadata_to_pageaddr(meta);
> @@ -349,6 +353,10 @@ static void *kfence_guarded_alloc(struct kmem_cache *cache, size_t size, gfp_t g
> atomic_long_inc(&counters[KFENCE_COUNTER_ALLOCS]);
>
> return addr;
> +
> +no_capacity:
> + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_CAPACITY]);
> + return NULL;
> }
>
> static void kfence_guarded_free(void *addr, struct kfence_metadata *meta, bool zombie)
> @@ -740,8 +748,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags)
> * Perform size check before switching kfence_allocation_gate, so that
> * we don't disable KFENCE without making an allocation.
> */
> - if (size > PAGE_SIZE)
> + if (size > PAGE_SIZE) {
> + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]);
> return NULL;
> + }
>
> /*
> * Skip allocations from non-default zones, including DMA. We cannot
> @@ -749,8 +759,10 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags)
> * properties (e.g. reside in DMAable memory).
> */
> if ((flags & GFP_ZONEMASK) ||
> - (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32)))
> + (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32))) {
> + atomic_long_inc(&counters[KFENCE_COUNTER_SKIP_INCOMPAT]);
> return NULL;
> + }
>
> /*
> * allocation_gate only needs to become non-zero, so it doesn't make
> --
> 2.33.0.464.g1972c5931b-goog
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20210917110756.1121272-1-elver%40google.com.