Re: [PATCH 1/3] slab: check the return value of check_bytes_and_report()

[Vlastimil Babka <vbabka@suse.cz>]

On 5/31/24 10:31 AM, Chengming Zhou wrote:
> On 2024/5/30 23:20, Christoph Lameter (Ampere) wrote:
>> On Tue, 28 May 2024, Chengming Zhou wrote:
>> 
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index 0809760cf789..de57512734ac 100644
>>> --- a/mm/slub.c
>>> +++ b/mm/slub.c
>>> @@ -1324,9 +1324,10 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
>>>         }
>>>     } else {
>>>         if ((s->flags & SLAB_POISON) && s->object_size < s->inuse) {
>>> -            check_bytes_and_report(s, slab, p, "Alignment padding",
>>> +            if (!check_bytes_and_report(s, slab, p, "Alignment padding",
>>>                 endobject, POISON_INUSE,
>>> -                s->inuse - s->object_size);
>>> +                s->inuse - s->object_size))
>>> +                return 0;
>>>         }
>>>     }
>> 
>> This change means we will then skip the rest of the checks in check_object() such as the poison check.
> 
> Yeah, only when this padding checking failed.
> 
> Now, we always abort checking and return 0 when the first checking error happens,
> such as redzones checking above.

Yes your patch will make it consistent. But IMHO it would be better to do
all the checks without skipping, report their specific error findings in
check_bytes_and_report() but not print_trailer(). Once all checks were done,
if any found an error, print the trailer once from check_object(). Thoughts?

> Thanks.