2021-06-30 15:04:49

by Alexander Potapenko

Subject: [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.

Suggested-by: Marco Elver <[email protected]>
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(-)

diff --git a/mm/kfence/core.c b/mm/kfence/core.c
index 4d21ac44d5d35..33bb20d91bf6a 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -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);
}

--
2.32.0.93.g670b81a890-goog

2021-06-30 15:07:07

by Alexander Potapenko

Subject: [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(+)

diff --git a/mm/kfence/core.c b/mm/kfence/core.c
index 33bb20d91bf6a..1cbdb62e6d0fb 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -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

2021-07-05 14:06:49

by Marco Elver

Subject: 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!


On Wed, 30 Jun 2021 at 17:02, Alexander Potapenko <[email protected]> wrote:
> 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(+)
>
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 33bb20d91bf6a..1cbdb62e6d0fb 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -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
>

2021-07-05 18:40:24

by Souptick Joarder

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

On Wed, Jun 30, 2021 at 8:32 PM Alexander Potapenko <[email protected]> wrote:
>
> 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

Acked-by: Souptick Joarder <[email protected]>

> v4:
> - minor fixes to description and comment formatting
> ---
> mm/kfence/core.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 33bb20d91bf6a..1cbdb62e6d0fb 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -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
>

2021-07-13 16:20:36

by Marco Elver

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

On Mon, 5 Jul 2021 at 16:05, Marco Elver <[email protected]> wrote:
> 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

> On Wed, 30 Jun 2021 at 17:02, Alexander Potapenko <[email protected]> wrote:
> > 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(+)
> >
> > diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> > index 33bb20d91bf6a..1cbdb62e6d0fb 100644
> > --- a/mm/kfence/core.c
> > +++ b/mm/kfence/core.c
> > @@ -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
> >