Re: [PATCH] pack-bitmap: fix memory leak if `load_bitmap_entries_v1` failed

From: Jeff King <peff@peff.net>
Date: May 12, 2025, 13:13 UTC
Message-ID: <20250512131315.GD1191360@coredump.intra.peff.net>
In-Reply-To: <pull.1962.git.git.1747052530271.gitgitgadget@gmail.com>

On Mon, May 12, 2025 at 12:22:10PM +0000, Lidong Yan via GitGitGadget wrote:

Show 7 quoted lines

> From: Lidong Yan <502024330056@smail.nju.edu.cn>
> 
> In pack-bitmap.c:load_bitmap_entries_v1, the function `read_bitmap_1`
> allocates a bitmap and reads index data into it. However, if any of
> the validation checks following the allocation fail, the allocated bitmap
> is not freed, resulting in a memory leak. To avoid this, the validation
> checks should be performed before the bitmap is allocated.

Thanks, this looks correct to me.

Show 10 quoted lines

> @@ -388,10 +388,6 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
>  			return error(_("corrupt ewah bitmap: commit index %u out of range"),
>  				     (unsigned)commit_idx_pos);
>  
> -		bitmap = read_bitmap_1(index);
> -		if (!bitmap)
> -			return -1;
> -
>  		if (xor_offset > MAX_XOR_OFFSET || xor_offset > i)
>  			return error(_("corrupted bitmap pack index"));

I noticed that this code is also within a loop, so we could still return early on the next loop iteration. But by that point we will have called store_bitmap() on the result, so we only have to worry about leaking the bitmap from the current loop iteration.

-Peff