1. Patchew
  2. linux
  3. View series

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

Li Qiong posted 1 patch 2 months ago 
mm/slub.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()
Posted by Li Qiong 2 months ago 
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()
Posted by Harry Yoo 2 months ago 
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()
Posted by Vlastimil Babka 2 months ago 
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()
Posted by liqiong 2 months ago 

在 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()
Posted by Vlastimil Babka 1 month, 1 week ago 
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.
Re: [PATCH v6] mm/slub: avoid accessing metadata when pointer is invalid in object_err()
Posted by Matthew Wilcox 2 months ago 
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()
Posted by Harry Yoo 2 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.