Re: [PATCH] last-modified: fix bug caused by inproper initialized memory

[Junio C Hamano <gitster@pobox.com>]

Toon Claes <toon@iotcl.com> writes:

> git-last-modified(1) uses a scratch bitmap to keep track of paths that
> have been changed between commits. To avoid reallocating a bitmap on
> each call of process_parent(), the scratch bitmap is kept and reused.
> Although, it seems an incorrect length is passed to memset(3).
>
> `struct bitmap` uses `eword_t` to for internal storage. This type is
> typedef'd to uint64_t. To fully zero the memory used by the bitmap,
> multiply the length (saved in `struct bitmap::word_alloc`) by the size
> of `eword_t`.
>
> Reported-by: Anders Kaseorg <andersk@mit.edu>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
> It was reported [1] the tests in t8020 fail on s390x. After some
> research, it seems it was related to s390x being big-endian. Well,
> actually, not really. Using big-endian simply uncovered the problem in
> test.
>
> [1]: https://lore.kernel.org/git/4dc4c8cd-c0cc-4784-8fcf-defa3a051087@mit.edu/
> ---
>  builtin/last-modified.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This dates back to v2.52.0~4 and is clearly a maint material.

Thanks for finding and fixing.

>
> diff --git a/builtin/last-modified.c b/builtin/last-modified.c
> index b0ecbdc540..cc5fd2e795 100644
> --- a/builtin/last-modified.c
> +++ b/builtin/last-modified.c
> @@ -327,7 +327,7 @@ static void process_parent(struct last_modified *lm,
>  	if (!(parent->object.flags & PARENT1))
>  		active_paths_free(lm, parent);
>  
> -	memset(lm->scratch->words, 0x0, lm->scratch->word_alloc);
> +	memset(lm->scratch->words, 0x0, lm->scratch->word_alloc * sizeof(eword_t));
>  	diff_queue_clear(&diff_queued_diff);
>  }
>  
>
> ---
> base-commit: 6ab38b7e9cc7adafc304f3204616a4debd49c6e9
> change-id: 20251126-toon-big-endian-ci-fe62bb361974