For debugging, object_err() prints free pointer of the object. However, if check_valid_pointer() returns false for a object, dereferncing `object + s->offset` can lead to a crash. Therefore, print the object's address in such cases.
Fixes: bb192ed9aa71 ("mm/slub: Convert most struct page to struct slab by spatch") 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(). --- mm/slub.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c index 31e11ef256f9..d3abae5a2193 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1104,7 +1104,11 @@ static void object_err(struct kmem_cache *s, struct slab *slab, return;
slab_bug(s, reason); - print_trailer(s, slab, object); + if (!check_valid_pointer(s, slab, object)) { + print_slab_info(slab); + pr_err("invalid object 0x%p\n", object); + } else + print_trailer(s, slab, object); add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
WARN_ON(1); @@ -1587,7 +1591,7 @@ static inline int alloc_consistency_checks(struct kmem_cache *s, return 0;
if (!check_valid_pointer(s, slab, object)) { - object_err(s, slab, object, "Freelist Pointer check fails"); + slab_err(s, slab, "Freelist Pointer(0x%p) check fails", object); return 0; }
On Tue, Jul 29, 2025 at 04:14:55PM +0800, Li Qiong wrote:
For debugging, object_err() prints free pointer of the object. However, if check_valid_pointer() returns false for a object, dereferncing `object + s->offset` can lead to a crash. Therefore, print the object's address in such cases.
As the code changed a bit, I think the commit message could better reflect what this patch actually does.
Fixes: bb192ed9aa71 ("mm/slub: Convert most struct page to struct slab by spatch")
As Vlastimil mentioned in previous version, this is not the first commit that introduced this problem.
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().
mm/slub.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c index 31e11ef256f9..d3abae5a2193 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1104,7 +1104,11 @@ static void object_err(struct kmem_cache *s, struct slab *slab, return; slab_bug(s, reason);
- print_trailer(s, slab, object);
- if (!check_valid_pointer(s, slab, object)) {
print_slab_info(slab);pr_err("invalid object 0x%p\n", object);
Can we just handle this inside print_trailer() because that's the function that prints the object's free pointer, metadata, etc.?
Also, the message should start with a capital letter.
and "invalid object" sounds misleading because it's the pointer that is invalid. Perhaps simply "Invalid pointer 0x%p\n"? (What would be the most comprehensive message here? :P)
- } else
print_trailer(s, slab, object);
add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE); WARN_ON(1); @@ -1587,7 +1591,7 @@ static inline int alloc_consistency_checks(struct kmem_cache *s, return 0; if (!check_valid_pointer(s, slab, object)) {
object_err(s, slab, object, "Freelist Pointer check fails");
slab_err(s, slab, "Freelist Pointer(0x%p) check fails", object);
Do we really need this hunk after making object_err() resiliant against wild pointers?
}
在 2025/7/29 21:41, Harry Yoo 写道:
On Tue, Jul 29, 2025 at 04:14:55PM +0800, Li Qiong wrote:
For debugging, object_err() prints free pointer of the object. However, if check_valid_pointer() returns false for a object, dereferncing `object + s->offset` can lead to a crash. Therefore, print the object's address in such cases.
As the code changed a bit, I think the commit message could better reflect what this patch actually does.
Yes.
Fixes: bb192ed9aa71 ("mm/slub: Convert most struct page to struct slab by spatch")
As Vlastimil mentioned in previous version, this is not the first commit that introduced this problem.
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().
mm/slub.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c index 31e11ef256f9..d3abae5a2193 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1104,7 +1104,11 @@ static void object_err(struct kmem_cache *s, struct slab *slab, return; slab_bug(s, reason);
- print_trailer(s, slab, object);
- if (!check_valid_pointer(s, slab, object)) {
print_slab_info(slab);pr_err("invalid object 0x%p\n", object);Can we just handle this inside print_trailer() because that's the function that prints the object's free pointer, metadata, etc.?
Maybe it's clearer , if object pointer being invalid, don't enter print_trailer()， print_trailer() prints valid object.
Also, the message should start with a capital letter.
and "invalid object" sounds misleading because it's the pointer that is invalid. Perhaps simply "Invalid pointer 0x%p\n"? (What would be the most comprehensive message here? :P)
Make sense, will change it.
- } else
print_trailer(s, slab, object);WARN_ON(1); @@ -1587,7 +1591,7 @@ static inline int alloc_consistency_checks(struct kmem_cache *s, return 0; if (!check_valid_pointer(s, slab, object)) {
object_err(s, slab, object, "Freelist Pointer check fails");
slab_err(s, slab, "Freelist Pointer(0x%p) check fails", object);Do we really need this hunk after making object_err() resiliant against wild pointers?
That's the origin issue, it may be inappropriate to use object_err(), if check_valid_pointer being false.
}
On Wed, Jul 30, 2025 at 09:46:09AM +0800, liqiong wrote:
在 2025/7/29 21:41, Harry Yoo 写道:
On Tue, Jul 29, 2025 at 04:14:55PM +0800, Li Qiong wrote:
Fixes: bb192ed9aa71 ("mm/slub: Convert most struct page to struct slab by spatch")
As Vlastimil mentioned in previous version, this is not the first commit that introduced this problem.
Please don't forget to update Fixes: tag :)
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().
mm/slub.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c index 31e11ef256f9..d3abae5a2193 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1104,7 +1104,11 @@ static void object_err(struct kmem_cache *s, struct slab *slab, return; slab_bug(s, reason);
- print_trailer(s, slab, object);
- if (!check_valid_pointer(s, slab, object)) {
print_slab_info(slab);pr_err("invalid object 0x%p\n", object);Can we just handle this inside print_trailer() because that's the function that prints the object's free pointer, metadata, etc.?
Maybe it's clearer , if object pointer being invalid, don't enter print_trailer()， print_trailer() prints valid object.
You're probably right. No strong opinion. object_err() is the only user anyway.
- } else
print_trailer(s, slab, object);WARN_ON(1); @@ -1587,7 +1591,7 @@ static inline int alloc_consistency_checks(struct kmem_cache *s, return 0; if (!check_valid_pointer(s, slab, object)) {
object_err(s, slab, object, "Freelist Pointer check fails");
slab_err(s, slab, "Freelist Pointer(0x%p) check fails", object);Do we really need this hunk after making object_err() resiliant against wild pointers?
That's the origin issue, it may be inappropriate to use object_err(), if check_valid_pointer being false.
That was the original issue, but you're making it not crash even if with bad pointers are passed?
}
在 2025/7/30 13:04, Harry Yoo 写道:
On Wed, Jul 30, 2025 at 09:46:09AM +0800, liqiong wrote:
在 2025/7/29 21:41, Harry Yoo 写道:
On Tue, Jul 29, 2025 at 04:14:55PM +0800, Li Qiong wrote:
Fixes: bb192ed9aa71 ("mm/slub: Convert most struct page to struct slab by spatch")
As Vlastimil mentioned in previous version, this is not the first commit that introduced this problem.
Please don't forget to update Fixes: tag :)
It seems that it's the first commit: 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().
mm/slub.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c index 31e11ef256f9..d3abae5a2193 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1104,7 +1104,11 @@ static void object_err(struct kmem_cache *s, struct slab *slab, return; slab_bug(s, reason);
- print_trailer(s, slab, object);
- if (!check_valid_pointer(s, slab, object)) {
print_slab_info(slab);pr_err("invalid object 0x%p\n", object);Can we just handle this inside print_trailer() because that's the function that prints the object's free pointer, metadata, etc.?
Maybe it's clearer , if object pointer being invalid, don't enter print_trailer()， print_trailer() prints valid object.
You're probably right. No strong opinion. object_err() is the only user anyway.
- } else
print_trailer(s, slab, object);WARN_ON(1); @@ -1587,7 +1591,7 @@ static inline int alloc_consistency_checks(struct kmem_cache *s, return 0; if (!check_valid_pointer(s, slab, object)) {
object_err(s, slab, object, "Freelist Pointer check fails");
slab_err(s, slab, "Freelist Pointer(0x%p) check fails", object);Do we really need this hunk after making object_err() resiliant against wild pointers?
That's the origin issue, it may be inappropriate to use object_err(), if check_valid_pointer being false.
That was the original issue, but you're making it not crash even if with bad pointers are passed?
Make sense, fix in object_err(), it wouldn't crash and print the message.
}
linux-stable-mirror@lists.linaro.org
-
Harry Yoo -
Li Qiong -
liqiong