Re: [PATCH v2 1/3] slab: make check_object() more consistent

[Chengming Zhou <chengming.zhou@linux.dev>]

On 2024/6/6 16:28, Vlastimil Babka wrote:
> On 6/5/24 9:13 AM, Chengming Zhou wrote:
>> Now check_object() calls check_bytes_and_report() multiple times to
>> check every section of the object it cares about, like left and right
>> redzones, object poison, paddings poison and freepointer. It will
>> abort the checking process and return 0 once it finds an error.
>>
[...]
>> -	/* Check free pointer validity */
>> -	if (!check_valid_pointer(s, slab, get_freepointer(s, p))) {
>> +	/*
>> +	 * Cannot check freepointer while object is allocated if
>> +	 * object and freepointer overlap.
>> +	 */
>> +	if (!freeptr_outside_object(s) && val == SLUB_RED_ACTIVE &&
> 
> Seems this condition should have been logically flipped?

Ah, right, will fix.

> 
>> +	    !check_valid_pointer(s, slab, get_freepointer(s, p))) {
>>  		object_err(s, slab, p, "Freepointer corrupt");
>>  		/*
>>  		 * No choice but to zap it and thus lose the remainder
>> @@ -1370,9 +1368,14 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
>>  		 * another error because the object count is now wrong.
>>  		 */
>>  		set_freepointer(s, p, NULL);
>> -		return 0;
> 
> Should set ret = 0 here?

Yes.

> 
>>  	}
>> -	return 1;
>> +
>> +	if (!ret && !slab_add_kunit_errors()) {
> 
> Also 5/6 of slub_kunit tests now fail as we increased the number of recorded

My bad, I didn't test with slub_kunit, will test later.

> errors vs expected. Either the slab_add_kunit_errors() test above should
> have a variant (parameter?) so it will only detect we are in slab-kunit test
> (to suppress the printing and taint) but doesn't increase slab_errors (we

I think this way is simpler for me, only suppress the printing but doesn't
increase slab_errors, will take this way and test again.

Thanks!

> increased them for the individual issues already), or simply raise the
> expectations of the tests so it matches the new implementation.
>