Re: [PATCH 0/4] plugging some mmap() leaks

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

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> On 06/03/2026 6:37 pm, Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>> 
>>> When compiling with the NO_MMAP build variable set, the built-in
>>> 'git_mmap()' and 'git_munmap()' compatability routines use simple
>>> memory allocation and file I/O to emulate the required behaviour.
>>> The current implementation is vunerable to the "double-delete" bug
>>> (where the pointer returned by malloc() is passed to free() two or
>>> more times), should the mapped memory block address be passed to
>>> munmap() multiple times.
>> 
>> Sorry if I am missing something glaringly obvious, but quite
>> honestly I am confused.  Wouldn't it be a bug to call munmap() again
>> on the same region of memory obtained from mmap() and then already
>> unmapped by calling munmap()?
>
> Yes. The (second) call to munmap() with the (already unmapped) memory
> region would return -1 with errno set to EINVAL.
>
> The emulation layer does not detect this situation and simply calls
> free() on the given pointer. Hence the 'double-delete' bug.
>
>> Or can the emulation layer cause such a second free() even if the
>> munmap() is done once and only once per memory region obtained from
>> a single mmap()?
>
> No. If you only git_munmap() once for a given memory region, everything
> is fine.

Hmph.  You make it sound as if we have some code that calls munmap()
on something that we are not sure if we have unmapped just in case,
trusting that it won't crash us and instead give us EINVAL, and that
is very much deliberate?  Unless we have such a code, bending over
backwards to track what has already been unmapped and return -1 with
EINVAL from munmap() for a second call is of dubious value, no?  I
still must be missing something...

Thanks.