On Mon, Jul 28, 2025 at 05:08:57PM +0800, liqiong wrote:
在 2025/7/28 13:24, Harry Yoo 写道:
On Mon, Jul 28, 2025 at 04:29:22AM +0100, Matthew Wilcox wrote:
On Mon, Jul 28, 2025 at 10:06:42AM +0800, liqiong wrote:
In this case it's an object pointer, not a freelist pointer. Or am I misunderstanding something?
Actually, in alloc_debug_processing() the pointer came from slab->freelist, so I think saying either "invalid freelist pointer" or "invalid object pointer" make sense...
free_consistency_checks() has 'slab_err(s, slab, "Invalid object pointer 0x%p", object);' Maybe it is better, alloc_consisency_checks() has the same message.
No. Think about it.
Haha, since I suggested that change, I feel like I have to rethink it and respond... Maybe I'm wrong again, but I love to be proven wrong :)
On second thought,
Unlike free_consistency_checks() where an arbitrary address can be passed, alloc_consistency_check() is not passed arbitrary addresses but only addresses from the freelist. So if a pointer is invalid there, it means the freelist pointer is invalid. And in all other parts of slub.c, such cases are described as "Free(list) pointer", or "Freechain" being invalid or corrupted.
So to stay consistent "Invalid freelist pointer" would be the right choice :) Sorry for the confusion.
Anyway, Li, to make progress on this I think it make sense to start by making object_err() resiliant against invalid pointers (as suggested by Matthew)? If you go down that path, this patch might no longer be required to fix the bug anyway...
And the change would be quite small. Most part of print_trailer() is printing metadata and raw content of the object, which is risky when the pointer is invalid. In that case we'd only want to print the address of the invalid pointer and the information about slab (print_slab_info()) and nothing more.
Got it, I will a v3 patch, changing the message, and keep it simple, dropping the comments of object_err(), just fix the issue.
Well, I was saying let's start from making object_err() against wild pointers [1] per Matthew's suggestion.
And with that this patch won't be necessary to fix the issue and will be more robust against similar mistakes like this?
[1] https://lore.kernel.org/linux-mm/aIPZXSnkDF5r-PR5@casper.infradead.org