Re: [PATCH] mm/kmemleak: fix checksum computation for per-cpu objects

[Pavel Tikhomirov <ptikhomirov@virtuozzo.com>]

Thanks for fixing!

Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

On 7/3/26 18:17, Breno Leitao wrote:
> The per-cpu object checksum folds each CPU's CRC together with XOR and
> seeds every CRC with 0. Both choices make update_checksum() miss content
> changes:
> 
>   - XOR is self-cancelling, so equal contents on two CPUs cancel out and
>     simultaneous identical changes leave the checksum unchanged.
>   - crc32(0, ...) over all-zero content is 0, so a freshly allocated,
>     zeroed per-cpu area checksums to 0, matching the initial value, and
>     the object is never seen to change.
> 
> See discussions at [0].
> 
> When update_checksum() wrongly reports an actively modified object as
> unchanged, kmemleak stops greying it for an extra scan and can report a
> live per-cpu object as a leak.
> 
> Fold the per-cpu CRC as a single rolling checksum across all CPUs and
> initialise the object checksum to ~0 so the first computed value always
> registers as a change, even for content that hashes to 0.
> reset_checksum() is seeded the same way.
> 
> Link: https://lore.kernel.org/all/akfYImSNDh3OjIfR@gmail.com [0]
> Co-developed-by: Catalin Marinas <catalin.marinas@arm.com>
> Fixes: 6c99d4eb7c5e ("kmemleak: enable tracking for percpu pointers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  mm/kmemleak.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 68a0e30eea1e3..e96e9efd19b0d 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -687,7 +687,7 @@ static struct kmemleak_object *__alloc_object(gfp_t gfp)
>  	atomic_set(&object->use_count, 1);
>  	object->excess_ref = 0;
>  	object->count = 0;			/* white color initially */
> -	object->checksum = 0;
> +	object->checksum = ~0;
>  	object->del_state = 0;
>  
>  	/* task information */
> @@ -981,7 +981,7 @@ static void reset_checksum(unsigned long ptr)
>  	}
>  
>  	raw_spin_lock_irqsave(&object->lock, flags);
> -	object->checksum = 0;
> +	object->checksum = ~0;
>  	raw_spin_unlock_irqrestore(&object->lock, flags);
>  	put_object(object);
>  }
> @@ -1410,7 +1410,8 @@ static bool update_checksum(struct kmemleak_object *object)
>  		for_each_possible_cpu(cpu) {
>  			void *ptr = per_cpu_ptr((void __percpu *)object->pointer, cpu);
>  
> -			object->checksum ^= crc32(0, kasan_reset_tag((void *)ptr), object->size);
> +			object->checksum = crc32(object->checksum,
> +						 kasan_reset_tag((void *)ptr), object->size);
>  		}
>  	} else {
>  		object->checksum = crc32(0, kasan_reset_tag((void *)object->pointer), object->size);
> 
> ---
> base-commit: 6eb8711ece2ce27e52e327a5b7a628ed39b97f45
> change-id: 20260703-kmemleak_checksum-e69602b36a3d
> 
> Best regards,
> --  
> Breno Leitao <leitao@debian.org>
> 

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.