[PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()

[PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()

[Li Qiong]

object_err() reports details of an object for further debugging, such as
the freelist pointer, redzone, etc. However, if the pointer is invalid,
attempting to access object metadata can lead to a crash since it does
not point to a valid object.

In case the pointer is NULL or check_valid_pointer() returns false for
the pointer, only print the pointer value and skip accessing metadata.

Fixes: 81819f0fc828 ("SLUB core")
Cc: <stable@vger.kernel.org>
Signed-off-by: Li Qiong <liqiong@nfschina.com>
---
v2:
- rephrase the commit message, add comment for object_err().
v3:
- check object pointer in object_err().
v4:
- restore changes in alloc_consistency_checks().
v5:
- rephrase message, fix code style.
v6:
- add checking 'object' if NULL.
---
 mm/slub.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mm/slub.c b/mm/slub.c
index 31e11ef256f9..972cf2bb2ee6 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1104,7 +1104,12 @@ static void object_err(struct kmem_cache *s, struct slab *slab,
 		return;
 
 	slab_bug(s, reason);
-	print_trailer(s, slab, object);
+	if (!object || !check_valid_pointer(s, slab, object)) {
+		print_slab_info(slab);
+		pr_err("Invalid pointer 0x%p\n", object);
+	} else {
+		print_trailer(s, slab, object);
+	}
 	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
 
 	WARN_ON(1);
-- 
2.30.2

Re: [PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()

[Harry Yoo]

On Mon, Aug 04, 2025 at 10:57:59AM +0800, Li Qiong wrote:
> object_err() reports details of an object for further debugging, such as
> the freelist pointer, redzone, etc. However, if the pointer is invalid,
> attempting to access object metadata can lead to a crash since it does
> not point to a valid object.
> 
> In case the pointer is NULL or check_valid_pointer() returns false for
> the pointer, only print the pointer value and skip accessing metadata.
> 
> Fixes: 81819f0fc828 ("SLUB core")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Li Qiong <liqiong@nfschina.com>
> ---
> v2:
> - rephrase the commit message, add comment for object_err().
> v3:
> - check object pointer in object_err().
> v4:
> - restore changes in alloc_consistency_checks().
> v5:
> - rephrase message, fix code style.
> v6:
> - add checking 'object' if NULL.
> ---

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

>  mm/slub.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 31e11ef256f9..972cf2bb2ee6 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1104,7 +1104,12 @@ static void object_err(struct kmem_cache *s, struct slab *slab,
>  		return;
>  
>  	slab_bug(s, reason);
> -	print_trailer(s, slab, object);
> +	if (!object || !check_valid_pointer(s, slab, object)) {
> +		print_slab_info(slab);
> +		pr_err("Invalid pointer 0x%p\n", object);
> +	} else {
> +		print_trailer(s, slab, object);
> +	}
>  	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
>  
>  	WARN_ON(1);
> -- 
> 2.30.2
> 

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()

[Vlastimil Babka]

On 8/4/25 04:57, Li Qiong wrote:
> object_err() reports details of an object for further debugging, such as
> the freelist pointer, redzone, etc. However, if the pointer is invalid,
> attempting to access object metadata can lead to a crash since it does
> not point to a valid object.
> 
> In case the pointer is NULL or check_valid_pointer() returns false for
> the pointer, only print the pointer value and skip accessing metadata.

We should explain that this is not theoretical so justify the stable cc, so
I would add:

One known path to the crash is when alloc_consistency_checks() determines
the pointer to the allocated object is invalid beause of a freelist
corruption, and calls object_err() to report it. The debug code should
report and handle the corruption gracefully and not crash in the process.

If you agree, I can do this when picking up the patch after merge window, no
need to resend.
> Fixes: 81819f0fc828 ("SLUB core")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Li Qiong <liqiong@nfschina.com>
> ---
> v2:
> - rephrase the commit message, add comment for object_err().
> v3:
> - check object pointer in object_err().
> v4:
> - restore changes in alloc_consistency_checks().
> v5:
> - rephrase message, fix code style.
> v6:
> - add checking 'object' if NULL.
> ---
>  mm/slub.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 31e11ef256f9..972cf2bb2ee6 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1104,7 +1104,12 @@ static void object_err(struct kmem_cache *s, struct slab *slab,
>  		return;
>  
>  	slab_bug(s, reason);
> -	print_trailer(s, slab, object);
> +	if (!object || !check_valid_pointer(s, slab, object)) {
> +		print_slab_info(slab);
> +		pr_err("Invalid pointer 0x%p\n", object);
> +	} else {
> +		print_trailer(s, slab, object);
> +	}
>  	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
>  
>  	WARN_ON(1);

Re: [PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()

[liqiong]

在 2025/8/4 23:19, Vlastimil Babka 写道:
> On 8/4/25 04:57, Li Qiong wrote:
>> object_err() reports details of an object for further debugging, such as
>> the freelist pointer, redzone, etc. However, if the pointer is invalid,
>> attempting to access object metadata can lead to a crash since it does
>> not point to a valid object.
>>
>> In case the pointer is NULL or check_valid_pointer() returns false for
>> the pointer, only print the pointer value and skip accessing metadata.
> We should explain that this is not theoretical so justify the stable cc, so
> I would add:
>
> One known path to the crash is when alloc_consistency_checks() determines
> the pointer to the allocated object is invalid beause of a freelist
> corruption, and calls object_err() to report it. The debug code should
> report and handle the corruption gracefully and not crash in the process.
>
> If you agree, I can do this when picking up the patch after merge window, no
> need to resend.

Agree, thanks.


>> Fixes: 81819f0fc828 ("SLUB core")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Li Qiong <liqiong@nfschina.com>
>> ---
>> v2:
>> - rephrase the commit message, add comment for object_err().
>> v3:
>> - check object pointer in object_err().
>> v4:
>> - restore changes in alloc_consistency_checks().
>> v5:
>> - rephrase message, fix code style.
>> v6:
>> - add checking 'object' if NULL.
>> ---
>>  mm/slub.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 31e11ef256f9..972cf2bb2ee6 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -1104,7 +1104,12 @@ static void object_err(struct kmem_cache *s, struct slab *slab,
>>  		return;
>>  
>>  	slab_bug(s, reason);
>> -	print_trailer(s, slab, object);
>> +	if (!object || !check_valid_pointer(s, slab, object)) {
>> +		print_slab_info(slab);
>> +		pr_err("Invalid pointer 0x%p\n", object);
>> +	} else {
>> +		print_trailer(s, slab, object);
>> +	}
>>  	add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
>>  
>>  	WARN_ON(1);

Re: [PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()

[Matthew Wilcox]

On Mon, Aug 04, 2025 at 05:19:59PM +0200, Vlastimil Babka wrote:
> On 8/4/25 04:57, Li Qiong wrote:
> > object_err() reports details of an object for further debugging, such as
> > the freelist pointer, redzone, etc. However, if the pointer is invalid,
> > attempting to access object metadata can lead to a crash since it does
> > not point to a valid object.
> > 
> > In case the pointer is NULL or check_valid_pointer() returns false for
> > the pointer, only print the pointer value and skip accessing metadata.

You realy need to get the nfschina mail system fixed.  None of your
messages are making it through to linux-mm.  Either that or start
sending emails from a different provider.

> > Fixes: 81819f0fc828 ("SLUB core")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Li Qiong <liqiong@nfschina.com>

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Re: [PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()

[Harry Yoo]

On Mon, Aug 04, 2025 at 05:19:59PM +0200, Vlastimil Babka wrote:
> On 8/4/25 04:57, Li Qiong wrote:
> > object_err() reports details of an object for further debugging, such as
> > the freelist pointer, redzone, etc. However, if the pointer is invalid,
> > attempting to access object metadata can lead to a crash since it does
> > not point to a valid object.
> > 
> > In case the pointer is NULL or check_valid_pointer() returns false for
> > the pointer, only print the pointer value and skip accessing metadata.
> 
> We should explain that this is not theoretical so justify the stable cc, so
> I would add:
> 
> One known path to the crash is when alloc_consistency_checks() determines
> the pointer to the allocated object is invalid beause of a freelist

nit: beause -> because

> corruption, and calls object_err() to report it. The debug code should
> report and handle the corruption gracefully and not crash in the process.
>
> If you agree, I can do this when picking up the patch after merge window, no
> need to resend.
>
> > Fixes: 81819f0fc828 ("SLUB core")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Li Qiong <liqiong@nfschina.com>
> > ---

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()

[Vlastimil Babka]

On 8/5/25 03:24, liqiong wrote:
> 
> 
> 在 2025/8/4 23:19, Vlastimil Babka 写道:
>> On 8/4/25 04:57, Li Qiong wrote:
>>> object_err() reports details of an object for further debugging, such as
>>> the freelist pointer, redzone, etc. However, if the pointer is invalid,
>>> attempting to access object metadata can lead to a crash since it does
>>> not point to a valid object.
>>>
>>> In case the pointer is NULL or check_valid_pointer() returns false for
>>> the pointer, only print the pointer value and skip accessing metadata.
>> We should explain that this is not theoretical so justify the stable cc, so
>> I would add:
>>
>> One known path to the crash is when alloc_consistency_checks() determines
>> the pointer to the allocated object is invalid beause of a freelist
>> corruption, and calls object_err() to report it. The debug code should
>> report and handle the corruption gracefully and not crash in the process.
>>
>> If you agree, I can do this when picking up the patch after merge window, no
>> need to resend.
> 
> Agree, thanks.

Merged to -next, thanks.