Message-ID: <f6b9049e-cb36-447a-ad8c-166bcb044393@gmail.com>
Date: Wed, 2 Oct 2024 17:19:15 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: malloc clobbers sbrk'd memory
From: "J.W. Jagersma (jwjagersma AT gmail DOT com) [via djgpp AT delorie DOT com]" <djgpp AT delorie DOT com>
To: djgpp AT delorie DOT com
References: <7d97575b-24ac-4d55-be14-266ade57b992 AT gmail DOT com>
Content-Language: en-US
In-Reply-To: <7d97575b-24ac-4d55-be14-266ade57b992@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Reply-To: djgpp AT delorie DOT com
Errors-To: nobody AT delorie DOT com
Precedence: bulk

On 2024-10-01 22:57, J.W. Jagersma wrote:
> I believe I found a bug where malloc sporadically writes past the end
> of a memory chunk.
> 
> I ran into this while working on mcount, which allocates the histogram
> and call graph via sbrk.  When my test program was invoked with some
> specific number of arguments, the first histogram entry was clobbered,
> which in gprof appeared as if the program had spent a lot of time in
> 'start'.  Turns out: the clobber value is the same as the special 'NONE'
> pointer used in nmalloc.c.
> 
> What I believe is happening: the minimum chunk size created by
> __nmalloc_split is DATAOFFSET (which is sizeof(memblock)-4; does *not*
> include prevfree).  But when the high half is then passed to
> mv2freelist, it writes prevfree=NONE, which is now past the end of the
> chunk.
> 
> Solution is probably to define MINSAVE as (ALIGN + sizeof (memblock)),
> regardless of SAVEMEMORY tricks.  It might be worth adding another 16
> bytes or so, since minimum-size chunks are useless and only add clutter
> to the freelist.

Some further analysis:

MINSAVE includes room for alignment, and headers don't require
re-alignment after split, so there *should* be enough space for
'prevfree' (and ALIGN is then also the minimum allocation size).

There is one point where nmalloc can split off a too-small block, which
is when allocating directly from 'lastsbrk' (at nmalloc.c:570).  Here
DATAOFFSET is the minimum size.

The bug then occurs on the next extendsbrk.  Because the other sbrk call
left a gap, the new block is not merged with 'lastsbrk', but instead it
calls mv2freelist(lastsbrk).  And at that point, 'prevfree' can overlap
with the sbrk'd memory.