kfence: move the size check to the beginning of __kfence_alloc()

PATCH Superseded Alexander Potapenko 3 participants 5 messages Jun 30, 2021 – Jul 13, 2021

Subsystems (3)

Stable branch Memory management Kfence

lore

Share & tools

lore

Alexander Potapenko

author maintainer

2021-06-30 15:02 UTC #0 lore raw

PATCH v4 1/2 kfence: move the size check to the beginning of __kfence_alloc()

Check the allocation size before toggling kfence_allocation_gate. This way allocations that can't be served by KFENCE will not result in waiting for another CONFIG_KFENCE_SAMPLE_INTERVAL without allocating anything.

Cc Andrew Morton <[email protected]>

Cc Dmitry Vyukov <[email protected]>

Cc Marco Elver <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc [email protected] # 5.12+

Signed-off-by Alexander Potapenko <[email protected]>

Reviewed-by Marco Elver <[email protected]>

---
 mm/kfence/core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

mm/kfence/core.c +7 -3

index 4d21ac44d5d35..33bb20d91bf6a 100644
@@ -733,6 +733,13 @@ void kfence_shutdown_cache(struct kmem_cache *s)
 
 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)
+		return NULL;
+
 	/*
 	 * allocation_gate only needs to become non-zero, so it doesn't make
 	 * sense to continue writing to it and pay the associated contention

@@ -757,9 +764,6 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags)
 	if (!READ_ONCE(kfence_enabled))
 		return NULL;
 
-	if (size > PAGE_SIZE)
-		return NULL;
-
 	return kfence_guarded_alloc(s, size, flags);
 }

Signature · 1 line

2.32.0.93.g670b81a890-goog

Alexander Potapenko

author maintainer

2021-06-30 15:02 UTC #1 lore raw

PATCH v4 2/2 kfence: skip all GFP_ZONEMASK allocations

Allocation requests outside ZONE_NORMAL (MOVABLE, HIGHMEM or DMA) cannot be fulfilled by KFENCE, because KFENCE memory pool is located in a zone different from the requested one.

Because callers of kmem_cache_alloc() may actually rely on the allocation to reside in the requested zone (e.g. memory allocations done with __GFP_DMA must be DMAable), skip all allocations done with GFP_ZONEMASK and/or respective SLAB flags (SLAB_CACHE_DMA and SLAB_CACHE_DMA32).

Fixes 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure")

Cc Andrew Morton <[email protected]>

Cc Dmitry Vyukov <[email protected]>

Cc Marco Elver <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Souptick Joarder <[email protected]>

Cc [email protected] # 5.12+

Signed-off-by Alexander Potapenko <[email protected]>

Reviewed-by Marco Elver <[email protected]>

---

v2:
- added parentheses around the GFP clause, as requested by Marco
v3:
- ignore GFP_ZONEMASK, which also covers __GFP_HIGHMEM and __GFP_MOVABLE
- move the flag check at the beginning of the function, as requested by Souptick Joarder
v4:
- minor fixes to description and comment formatting

---
 mm/kfence/core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

mm/kfence/core.c +9 -0

index 33bb20d91bf6a..1cbdb62e6d0fb 100644
@@ -740,6 +740,15 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags)
 	if (size > PAGE_SIZE)
 		return NULL;
 
+	/*
+	 * Skip allocations from non-default zones, including DMA. We cannot
+	 * guarantee that pages in the KFENCE pool will have the requested
+	 * properties (e.g. reside in DMAable memory).
+	 */
+	if ((flags & GFP_ZONEMASK) ||
+	    (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32)))
+		return NULL;
+
 	/*
 	 * allocation_gate only needs to become non-zero, so it doesn't make
 	 * sense to continue writing to it and pay the associated contention

Signature · 1 line

2.32.0.93.g670b81a890-goog

Marco Elver

maintainer

2021-07-05 14:06 UTC #2 lore raw

Re: [PATCH v4 2/2] kfence: skip all GFP_ZONEMASK allocations

Andrew,

This series is ready to be picked up.

If possible, kindly consider including it in an upcoming batch of mainline fixes.

Thank you!

Quoted 57 lines · Alexander Potapenko

Allocation requests outside ZONE_NORMAL (MOVABLE, HIGHMEM or DMA) cannot be fulfilled by KFENCE, because KFENCE memory pool is located in a zone different from the requested one.
Because callers of kmem_cache_alloc() may actually rely on the allocation to reside in the requested zone (e.g. memory allocations done with __GFP_DMA must be DMAable), skip all allocations done with GFP_ZONEMASK and/or respective SLAB flags (SLAB_CACHE_DMA and SLAB_CACHE_DMA32).
Fixes 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure")
Cc Andrew Morton <[email protected]>
Cc Dmitry Vyukov <[email protected]>
Cc Marco Elver <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Souptick Joarder <[email protected]>
Cc [email protected] # 5.12+
Signed-off-by Alexander Potapenko <[email protected]>
Reviewed-by Marco Elver <[email protected]>
---
v2:
- added parentheses around the GFP clause, as requested by Marco
v3:
- ignore GFP_ZONEMASK, which also covers __GFP_HIGHMEM and __GFP_MOVABLE
- move the flag check at the beginning of the function, as requested by Souptick Joarder
v4:
- minor fixes to description and comment formatting
---
 mm/kfence/core.c | 9 +++++++++
 1 file changed, 9 insertions(+)
mm/kfence/core.c +9 -1
index 33bb20d91bf6a..1cbdb62e6d0fb 100644
@@ -740,6 +740,15 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags)
        if (size > PAGE_SIZE)
                return NULL;
 
+       /*
+        * Skip allocations from non-default zones, including DMA. We cannot
+        * guarantee that pages in the KFENCE pool will have the requested
+        * properties (e.g. reside in DMAable memory).
+        */
+       if ((flags & GFP_ZONEMASK) ||
+           (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32)))
+               return NULL;
+
        /*
         * allocation_gate only needs to become non-zero, so it doesn't make
         * sense to continue writing to it and pay the associated contention
--
2.32.0.93.g670b81a890-goog

Marco Elver

maintainer

2021-07-13 16:19 UTC #3 lore raw

Re: [PATCH v4 2/2] kfence: skip all GFP_ZONEMASK allocations

Quoted 8 lines · Marco Elver

Andrew,
This series is ready to be picked up.
If possible, kindly consider including it in an upcoming batch of mainline fixes.
Thank you!

It'd be good to get this sorted -- please take another look.

Many thanks,
-- Marco

Quoted 58 lines · Alexander Potapenko

Quoted 57 lines
Allocation requests outside ZONE_NORMAL (MOVABLE, HIGHMEM or DMA) cannot be fulfilled by KFENCE, because KFENCE memory pool is located in a zone different from the requested one.
Because callers of kmem_cache_alloc() may actually rely on the allocation to reside in the requested zone (e.g. memory allocations done with __GFP_DMA must be DMAable), skip all allocations done with GFP_ZONEMASK and/or respective SLAB flags (SLAB_CACHE_DMA and SLAB_CACHE_DMA32).
Fixes 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure")
Cc Andrew Morton <[email protected]>
Cc Dmitry Vyukov <[email protected]>
Cc Marco Elver <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Souptick Joarder <[email protected]>
Cc [email protected] # 5.12+
Signed-off-by Alexander Potapenko <[email protected]>
Reviewed-by Marco Elver <[email protected]>
---
v2:
- added parentheses around the GFP clause, as requested by Marco
v3:
- ignore GFP_ZONEMASK, which also covers __GFP_HIGHMEM and __GFP_MOVABLE
- move the flag check at the beginning of the function, as requested by Souptick Joarder
v4:
- minor fixes to description and comment formatting
---
 mm/kfence/core.c | 9 +++++++++
 1 file changed, 9 insertions(+)
mm/kfence/core.c +9 -1
index 33bb20d91bf6a..1cbdb62e6d0fb 100644
@@ -740,6 +740,15 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags)
        if (size > PAGE_SIZE)
                return NULL;
 
+       /*
+        * Skip allocations from non-default zones, including DMA. We cannot
+        * guarantee that pages in the KFENCE pool will have the requested
+        * properties (e.g. reside in DMAable memory).
+        */
+       if ((flags & GFP_ZONEMASK) ||
+           (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32)))
+               return NULL;
+
        /*
         * allocation_gate only needs to become non-zero, so it doesn't make
         * sense to continue writing to it and pay the associated contention
--
2.32.0.93.g670b81a890-goog

Souptick Joarder

Acked-by on v4 2/2

2021-07-05 18:39 UTC #4 lore raw

Re: [PATCH v4 2/2] kfence: skip all GFP_ZONEMASK allocations

In reply to Alexander Potapenko

Quoted 21 lines · Alexander Potapenko

Allocation requests outside ZONE_NORMAL (MOVABLE, HIGHMEM or DMA) cannot be fulfilled by KFENCE, because KFENCE memory pool is located in a zone different from the requested one.
Because callers of kmem_cache_alloc() may actually rely on the allocation to reside in the requested zone (e.g. memory allocations done with __GFP_DMA must be DMAable), skip all allocations done with GFP_ZONEMASK and/or respective SLAB flags (SLAB_CACHE_DMA and SLAB_CACHE_DMA32).
Fixes 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure")
Cc Andrew Morton <[email protected]>
Cc Dmitry Vyukov <[email protected]>
Cc Marco Elver <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Souptick Joarder <[email protected]>
Cc [email protected] # 5.12+
Signed-off-by Alexander Potapenko <[email protected]>
Reviewed-by Marco Elver <[email protected]>

Quoted 8 lines

---
v2:
- added parentheses around the GFP clause, as requested by Marco
v3:
- ignore GFP_ZONEMASK, which also covers __GFP_HIGHMEM and __GFP_MOVABLE
- move the flag check at the beginning of the function, as requested by Souptick Joarder

Acked-by Souptick Joarder <[email protected]>

Quoted 29 lines

v4:
- minor fixes to description and comment formatting

---
 mm/kfence/core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

mm/kfence/core.c +9 -1

index 33bb20d91bf6a..1cbdb62e6d0fb 100644
@@ -740,6 +740,15 @@ void *__kfence_alloc(struct kmem_cache *s, size_t size, gfp_t flags)
        if (size > PAGE_SIZE)
                return NULL;
 
+       /*
+        * Skip allocations from non-default zones, including DMA. We cannot
+        * guarantee that pages in the KFENCE pool will have the requested
+        * properties (e.g. reside in DMAable memory).
+        */
+       if ((flags & GFP_ZONEMASK) ||
+           (s->flags & (SLAB_CACHE_DMA | SLAB_CACHE_DMA32)))
+               return NULL;
+
        /*
         * allocation_gate only needs to become non-zero, so it doesn't make
         * sense to continue writing to it and pay the associated contention
--

2.32.0.93.g670b81a890-goog