[PATCH v2 0/6] slab: struct slab pointer validation improvements

[PATCH v2 0/6] slab: struct slab pointer validation improvements

[Vlastimil Babka]

See below for v2 changelog. I'm going to apply to slab/for-next

This tries to combine the goals of the first 4 patches from Matthew's
series [1] with points raised during review by Christoph and myself.
Patch 4 from [1] is taken as patch 1 here. In other cases the struct
slab pointer validation is better to move to a place where it can be
performed before touching any of the struct slab fields, rather than
removing it completely.

Further we wrap the validation in a function validate_slab_ptr() and
make the impementation use struct page instead of struct folio to be
compatible with the rest of the series [1]. With further changes towards
memdesc the implementation can change accordingly, if it will still make
sense.

The summary is that we validate pointers from the node partial list when
allocating from it or when validation is triggered from sysfs. When
freeing, we always obtain the slab pointer in a way that the page type
is tested in the process, so we don't need to validate.

Based on:
https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git/log/?h=slab/for-6.18/fixes

[1] https://lore.kernel.org/all/20250910115507.1991829-1-willy@infradead.org/

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
Changes in v2:
- Restore remove_partial() in patch 5 (thanks to Harry)
- Add R-b's from Harry
- Change validate_slab_ptr() from static to static inline.
- In patch 5, wrap check added to alloc_single_from_partial() in #ifdef
  SLUB_DEBUG due to using slab_err() (thanks to lkp report).
- Also in patch 5, move validate_slab_ptr() under SLUB_DEBUG as it
  becomes an unused function otherwise and W=1 CC=clang complains
- Verify no warnings/errors at every step with W=1 CC=clang and
  SLUB_DEBUG either enabled or disabled.
- Link to v1: https://patch.msgid.link/20250911-slub-slab-validation-v1-0-8b67eb3b3dc5@suse.cz

---
Matthew Wilcox (Oracle) (1):
      slab: Remove dead code in free_consistency_checks()

Vlastimil Babka (5):
      slab: wrap debug slab validation in validate_slab_ptr()
      slab: move validate_slab_ptr() from check_slab() to its callers
      slab: move validate_slab_ptr() from alloc_consistency_checks() to its caller
      slab: validate slab before using it in alloc_single_from_partial()
      slab: don't validate slab pointer in free_debug_processing()

 mm/slub.c | 62 ++++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 22 deletions(-)
---
base-commit: 41534d499e50e23571d6b9960498777d93f817ce
change-id: 20250911-slub-slab-validation-0e4f559b0a1d

Best regards,
-- 
Vlastimil Babka <vbabka@suse.cz>

[PATCH v2 1/6] slab: Remove dead code in free_consistency_checks()

[Vlastimil Babka]

From: "Matthew Wilcox (Oracle)" <willy@infradead.org>

We already know that slab is a valid slab as that's checked by the
caller.  In the future, we won't be able to get to a slab pointer
from a non-slab page.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/slub.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 3062f56bf49882538ba5af407de9f69c451f2e29..56143bfd1ae319d384981c810a5ed84af00f4afa 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1684,10 +1684,7 @@ static inline int free_consistency_checks(struct kmem_cache *s,
 		return 0;
 
 	if (unlikely(s != slab->slab_cache)) {
-		if (!folio_test_slab(slab_folio(slab))) {
-			slab_err(s, slab, "Attempt to free object(0x%p) outside of slab",
-				 object);
-		} else if (!slab->slab_cache) {
+		if (!slab->slab_cache) {
 			slab_err(NULL, slab, "No slab cache for object 0x%p",
 				 object);
 		} else {

-- 
2.51.0

[PATCH v2 2/6] slab: wrap debug slab validation in validate_slab_ptr()

[Vlastimil Babka]

This will make it clear where we currently cast struct slab to folio
only to check the slab type, and allow to change the implementation
later with memdesc conversion.

For now use a struct page based implementation instead of struct folio
to be compatible with further upcoming changes.

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/slub.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 56143bfd1ae319d384981c810a5ed84af00f4afa..75e4388d507d1abcbce8c7d5d2581381de46cf4d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -821,6 +821,15 @@ static inline unsigned int get_orig_size(struct kmem_cache *s, void *object)
 	return *(unsigned int *)p;
 }
 
+/*
+ * For debugging context when we want to check if the struct slab pointer
+ * appears to be valid.
+ */
+static inline bool validate_slab_ptr(struct slab *slab)
+{
+	return PageSlab(slab_page(slab));
+}
+
 #ifdef CONFIG_SLUB_DEBUG
 static unsigned long object_map[BITS_TO_LONGS(MAX_OBJS_PER_PAGE)];
 static DEFINE_SPINLOCK(object_map_lock);
@@ -1453,7 +1462,7 @@ static int check_slab(struct kmem_cache *s, struct slab *slab)
 {
 	int maxobj;
 
-	if (!folio_test_slab(slab_folio(slab))) {
+	if (!validate_slab_ptr(slab)) {
 		slab_err(s, slab, "Not a valid slab page");
 		return 0;
 	}
@@ -1653,7 +1662,7 @@ static noinline bool alloc_debug_processing(struct kmem_cache *s,
 	return true;
 
 bad:
-	if (folio_test_slab(slab_folio(slab))) {
+	if (validate_slab_ptr(slab)) {
 		/*
 		 * If this is a slab page then lets do the best we can
 		 * to avoid issues in the future. Marking all objects
@@ -2818,7 +2827,7 @@ static void *alloc_single_from_partial(struct kmem_cache *s,
 	slab->inuse++;
 
 	if (!alloc_debug_processing(s, slab, object, orig_size)) {
-		if (folio_test_slab(slab_folio(slab)))
+		if (validate_slab_ptr(slab))
 			remove_partial(n, slab);
 		return NULL;
 	}

-- 
2.51.0

[PATCH v2 3/6] slab: move validate_slab_ptr() from check_slab() to its callers

[Vlastimil Babka]

We will want to do the validation earlier in some callers or remove it
completely, so extract it from check_slab() first. No functional change.

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/slub.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 75e4388d507d1abcbce8c7d5d2581381de46cf4d..6fb24010c17019ed304b4ef61f402212dd634efb 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1458,15 +1458,15 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
 	return ret;
 }
 
+/*
+ * Checks if the slab state looks sane. Assumes the struct slab pointer
+ * was either obtained in a way that ensures it's valid, or validated
+ * by validate_slab_ptr()
+ */
 static int check_slab(struct kmem_cache *s, struct slab *slab)
 {
 	int maxobj;
 
-	if (!validate_slab_ptr(slab)) {
-		slab_err(s, slab, "Not a valid slab page");
-		return 0;
-	}
-
 	maxobj = order_objects(slab_order(slab), s->size);
 	if (slab->objects > maxobj) {
 		slab_err(s, slab, "objects %u > max %u",
@@ -1633,6 +1633,11 @@ void setup_slab_debug(struct kmem_cache *s, struct slab *slab, void *addr)
 static inline int alloc_consistency_checks(struct kmem_cache *s,
 					struct slab *slab, void *object)
 {
+	if (!validate_slab_ptr(slab)) {
+		slab_err(s, slab, "Not a valid slab page");
+		return 0;
+	}
+
 	if (!check_slab(s, slab))
 		return 0;
 
@@ -3485,6 +3490,11 @@ static inline bool free_debug_processing(struct kmem_cache *s,
 	int cnt = 0;
 
 	if (s->flags & SLAB_CONSISTENCY_CHECKS) {
+		if (!validate_slab_ptr(slab)) {
+			slab_err(s, slab, "Not a valid slab page");
+			goto out;
+		}
+
 		if (!check_slab(s, slab))
 			goto out;
 	}
@@ -6519,6 +6529,11 @@ static void validate_slab(struct kmem_cache *s, struct slab *slab,
 	void *p;
 	void *addr = slab_address(slab);
 
+	if (!validate_slab_ptr(slab)) {
+		slab_err(s, slab, "Not a valid slab page");
+		return;
+	}
+
 	if (!check_slab(s, slab) || !on_freelist(s, slab, NULL))
 		return;
 

-- 
2.51.0

[PATCH v2 4/6] slab: move validate_slab_ptr() from alloc_consistency_checks() to its caller

[Vlastimil Babka]

In alloc_debug_processing() we can call validate_slab_ptr() upfront and
then don't need to recheck when alloc_consistency_checks() fails for
other reasons.

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/slub.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 6fb24010c17019ed304b4ef61f402212dd634efb..12ad42f3d2e066b02340f2c30a85422583af3c5d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1633,11 +1633,6 @@ void setup_slab_debug(struct kmem_cache *s, struct slab *slab, void *addr)
 static inline int alloc_consistency_checks(struct kmem_cache *s,
 					struct slab *slab, void *object)
 {
-	if (!validate_slab_ptr(slab)) {
-		slab_err(s, slab, "Not a valid slab page");
-		return 0;
-	}
-
 	if (!check_slab(s, slab))
 		return 0;
 
@@ -1656,6 +1651,11 @@ static noinline bool alloc_debug_processing(struct kmem_cache *s,
 			struct slab *slab, void *object, int orig_size)
 {
 	if (s->flags & SLAB_CONSISTENCY_CHECKS) {
+		if (!validate_slab_ptr(slab)) {
+			slab_err(s, slab, "Not a valid slab page");
+			return false;
+		}
+
 		if (!alloc_consistency_checks(s, slab, object))
 			goto bad;
 	}
@@ -1667,17 +1667,15 @@ static noinline bool alloc_debug_processing(struct kmem_cache *s,
 	return true;
 
 bad:
-	if (validate_slab_ptr(slab)) {
-		/*
-		 * If this is a slab page then lets do the best we can
-		 * to avoid issues in the future. Marking all objects
-		 * as used avoids touching the remaining objects.
-		 */
-		slab_fix(s, "Marking all objects used");
-		slab->inuse = slab->objects;
-		slab->freelist = NULL;
-		slab->frozen = 1; /* mark consistency-failed slab as frozen */
-	}
+	/*
+	 * Let's do the best we can to avoid issues in the future. Marking all
+	 * objects as used avoids touching the remaining objects.
+	 */
+	slab_fix(s, "Marking all objects used");
+	slab->inuse = slab->objects;
+	slab->freelist = NULL;
+	slab->frozen = 1; /* mark consistency-failed slab as frozen */
+
 	return false;
 }
 

-- 
2.51.0

[PATCH v2 5/6] slab: validate slab before using it in alloc_single_from_partial()

[Vlastimil Babka]

We touch slab->freelist and slab->inuse before checking the slab pointer
is actually sane. Do that validation first, which will be safer. We can
thus also remove the check from alloc_debug_processing().

This adds a new "s->flags & SLAB_CONSISTENCY_CHECKS" test but
alloc_single_from_partial() is only called for caches with debugging
enabled so it's acceptable.

In alloc_single_from_new_slab() we just created the struct slab and call
alloc_debug_processing() to mainly set up redzones, tracking etc, while
not really expecting the consistency checks to fail. Thus don't validate
it there.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/slub.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 12ad42f3d2e066b02340f2c30a85422583af3c5d..e5b53d1debddd3fe0f941f579a1043a5b976e50b 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -821,6 +821,8 @@ static inline unsigned int get_orig_size(struct kmem_cache *s, void *object)
 	return *(unsigned int *)p;
 }
 
+#ifdef CONFIG_SLUB_DEBUG
+
 /*
  * For debugging context when we want to check if the struct slab pointer
  * appears to be valid.
@@ -830,7 +832,6 @@ static inline bool validate_slab_ptr(struct slab *slab)
 	return PageSlab(slab_page(slab));
 }
 
-#ifdef CONFIG_SLUB_DEBUG
 static unsigned long object_map[BITS_TO_LONGS(MAX_OBJS_PER_PAGE)];
 static DEFINE_SPINLOCK(object_map_lock);
 
@@ -1651,11 +1652,6 @@ static noinline bool alloc_debug_processing(struct kmem_cache *s,
 			struct slab *slab, void *object, int orig_size)
 {
 	if (s->flags & SLAB_CONSISTENCY_CHECKS) {
-		if (!validate_slab_ptr(slab)) {
-			slab_err(s, slab, "Not a valid slab page");
-			return false;
-		}
-
 		if (!alloc_consistency_checks(s, slab, object))
 			goto bad;
 	}
@@ -2825,13 +2821,21 @@ static void *alloc_single_from_partial(struct kmem_cache *s,
 
 	lockdep_assert_held(&n->list_lock);
 
+#ifdef SLUB_DEBUG
+	if (s->flags & SLAB_CONSISTENCY_CHECKS) {
+		if (!validate_slab_ptr(slab)) {
+			slab_err(s, slab, "Not a valid slab page");
+			return NULL;
+		}
+	}
+#endif
+
 	object = slab->freelist;
 	slab->freelist = get_freepointer(s, object);
 	slab->inuse++;
 
 	if (!alloc_debug_processing(s, slab, object, orig_size)) {
-		if (validate_slab_ptr(slab))
-			remove_partial(n, slab);
+		remove_partial(n, slab);
 		return NULL;
 	}
 

-- 
2.51.0

[PATCH v2 6/6] slab: don't validate slab pointer in free_debug_processing()

[Vlastimil Babka]

The struct slab pointer has been obtained from the object being freed on
all the paths that lead to this function. In all cases this already
includes the test for slab type of the struct page which struct slab is
overlaying. Thus we would not reach this function if it was not a valid
slab pointer in the first place.

One less obvious case is that kmem_cache_free() trusts virt_to_slab()
blindly so it may be NULL if the slab type check is false. But with
SLAB_CONSISTENCY_CHECKS, cache_from_obj() called also from
kmem_cache_free() catches this and returns NULL, which terminates
freeing immediately.

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/slub.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index e5b53d1debddd3fe0f941f579a1043a5b976e50b..6fe02b1d3fe9d4101465190ebefd6df41f887fb9 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3492,11 +3492,6 @@ static inline bool free_debug_processing(struct kmem_cache *s,
 	int cnt = 0;
 
 	if (s->flags & SLAB_CONSISTENCY_CHECKS) {
-		if (!validate_slab_ptr(slab)) {
-			slab_err(s, slab, "Not a valid slab page");
-			goto out;
-		}
-
 		if (!check_slab(s, slab))
 			goto out;
 	}

-- 
2.51.0

Re: [PATCH v2 5/6] slab: validate slab before using it in alloc_single_from_partial()

[Harry Yoo]

On Mon, Sep 15, 2025 at 03:55:12PM +0200, Vlastimil Babka wrote:
> We touch slab->freelist and slab->inuse before checking the slab pointer
> is actually sane. Do that validation first, which will be safer. We can
> thus also remove the check from alloc_debug_processing().
> 
> This adds a new "s->flags & SLAB_CONSISTENCY_CHECKS" test but
> alloc_single_from_partial() is only called for caches with debugging
> enabled so it's acceptable.
> 
> In alloc_single_from_new_slab() we just created the struct slab and call
> alloc_debug_processing() to mainly set up redzones, tracking etc, while
> not really expecting the consistency checks to fail. Thus don't validate
> it there.
> 
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  mm/slub.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 12ad42f3d2e066b02340f2c30a85422583af3c5d..e5b53d1debddd3fe0f941f579a1043a5b976e50b 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -821,6 +821,8 @@ static inline unsigned int get_orig_size(struct kmem_cache *s, void *object)
>  	return *(unsigned int *)p;
>  }
>  
> +#ifdef CONFIG_SLUB_DEBUG
> +
>  /*
>   * For debugging context when we want to check if the struct slab pointer
>   * appears to be valid.
> @@ -830,7 +832,6 @@ static inline bool validate_slab_ptr(struct slab *slab)
>  	return PageSlab(slab_page(slab));
>  }
>  
> -#ifdef CONFIG_SLUB_DEBUG
>  static unsigned long object_map[BITS_TO_LONGS(MAX_OBJS_PER_PAGE)];
>  static DEFINE_SPINLOCK(object_map_lock);
>  
> @@ -1651,11 +1652,6 @@ static noinline bool alloc_debug_processing(struct kmem_cache *s,
>  			struct slab *slab, void *object, int orig_size)
>  {
>  	if (s->flags & SLAB_CONSISTENCY_CHECKS) {
> -		if (!validate_slab_ptr(slab)) {
> -			slab_err(s, slab, "Not a valid slab page");
> -			return false;
> -		}
> -
>  		if (!alloc_consistency_checks(s, slab, object))
>  			goto bad;
>  	}
> @@ -2825,13 +2821,21 @@ static void *alloc_single_from_partial(struct kmem_cache *s,
>  
>  	lockdep_assert_held(&n->list_lock);
>  
> +#ifdef SLUB_DEBUG

I'm sure you meant CONFIG_SLUB_DEBUG ;)

With that adjusted, looks good to me,
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

> +	if (s->flags & SLAB_CONSISTENCY_CHECKS) {
> +		if (!validate_slab_ptr(slab)) {
> +			slab_err(s, slab, "Not a valid slab page");
> +			return NULL;
> +		}
> +	}
> +#endif
> +
>  	object = slab->freelist;
>  	slab->freelist = get_freepointer(s, object);
>  	slab->inuse++;
>  
>  	if (!alloc_debug_processing(s, slab, object, orig_size)) {
> -		if (validate_slab_ptr(slab))
> -			remove_partial(n, slab);
> +		remove_partial(n, slab);
>  		return NULL;
>  	}

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH v2 5/6] slab: validate slab before using it in alloc_single_from_partial()

[Vlastimil Babka]

On 9/15/25 16:25, Harry Yoo wrote:
> On Mon, Sep 15, 2025 at 03:55:12PM +0200, Vlastimil Babka wrote:
>> @@ -2825,13 +2821,21 @@ static void *alloc_single_from_partial(struct kmem_cache *s,
>>  
>>  	lockdep_assert_held(&n->list_lock);
>>  
>> +#ifdef SLUB_DEBUG
> 
> I'm sure you meant CONFIG_SLUB_DEBUG ;)
> 
> With that adjusted, looks good to me,
> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

Doh yes, thanks a lot :)
>> +	if (s->flags & SLAB_CONSISTENCY_CHECKS) {
>> +		if (!validate_slab_ptr(slab)) {
>> +			slab_err(s, slab, "Not a valid slab page");
>> +			return NULL;
>> +		}
>> +	}
>> +#endif
>> +
>>  	object = slab->freelist;
>>  	slab->freelist = get_freepointer(s, object);
>>  	slab->inuse++;
>>  
>>  	if (!alloc_debug_processing(s, slab, object, orig_size)) {
>> -		if (validate_slab_ptr(slab))
>> -			remove_partial(n, slab);
>> +		remove_partial(n, slab);
>>  		return NULL;
>>  	}
>