On Fri, Jul 25, 2025 at 06:47:01PM +0200, Vlastimil Babka wrote:
On 7/25/25 08:49, 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.
I don't know where this patch came from (was it cc'd to linux-mm? i don't see it)
+/*
- object - should be a valid object.
- check_valid_pointer(s, slab, object) should be true.
- */
This comment is very confusing. It tries to ape kernel-doc style, but if it were kernel-doc, the word before the hyphen should be the name of the function, and it isn't. If we did use kernel-doc for this, we'd use @object to denote that we're documenting the argument.
But I don't see the need to pretend this is related to kernel-doc. This would be better:
/* * 'object' must be a valid pointer into this slab. ie * check_valid_pointer() would return true */
I'm sure better wording for that is possible ...
if (!check_valid_pointer(s, slab, object)) {
object_err(s, slab, object, "Freelist Pointer check fails");
slab_err(s, slab, "Invalid object pointer 0x%p", object);
No, the error message is now wrong. It's not an object, it's the freelist pointer.
slab_err(s, slab, "Invalid freelist pointer %p", object);
(the 0x%p is wrong because it will print 0x twice)
But I think there are even more things wrong here. Like slab_err() is not nerely as severe as slab_bug(), which is what used to be called. And object_err() adds a taint, which this skips.
Altogether, this is a poorly thought out patch and should be dropped.