Re: [PATCH v2] mm: slub: avoid deref of free pointer in sanity checks if object is invalid

[Harry Yoo <harry.yoo@oracle.com>]

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.

-- 
Cheers,
Harry / Hyeonggon