X-Authentication-Warning: delorie.com: mail set sender to djgpp-workers-bounces using -f X-Recipient: djgpp-workers AT delorie DOT com X-Gm-Message-State: AOJu0YxwJD4/mTtbngaqmkTtYoyPGoKG5ziH+/S1G9osAgupErVP2VV3 m3xtMy0EMJZ+M+NcjLTZhijy9+sE0QpnyAYkw+y+ZYD0IjR1A29cZ6Ejw23GfRxkUrlEgezdwKE sKQ1F8uG7IbU0o6q9SRrnWSP+lco= X-Google-Smtp-Source: AGHT+IHI80kjsdBfcHd8BmsGO5iZtUnenao2UqUvaql96yNlweP4FLqgaAEegx6tVDCjiqBBNa/J5tVVh3o//qlPLHw= X-Received: by 2002:a05:6e02:1e05:b0:365:f79:baa8 with SMTP id g5-20020a056e021e0500b003650f79baa8mr14714626ila.23.1709012057025; Mon, 26 Feb 2024 21:34:17 -0800 (PST) MIME-Version: 1.0 References: <20181121121721 DOT GA1501 AT b21e7fa156c1e52f6cc3a9e0ddf22975> In-Reply-To: From: "Daniel Verkamp (daniel AT drv DOT nu) [via djgpp-workers AT delorie DOT com]" Date: Mon, 26 Feb 2024 21:34:06 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: memalign broken (again) To: djgpp-workers AT delorie DOT com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by delorie.com id 41R5YKlP737279 Reply-To: djgpp-workers AT delorie DOT com On Wed, Nov 21, 2018 at 4:33 AM Ozkan Sezer (sezeroz AT gmail DOT com) [via djgpp-workers AT delorie DOT com] wrote: > > On 11/21/18, Peter Ross wrote: > > hi, > > > > djgpp memalign still fails when: > > > > 1. alignment is >= 16 bytes > > 2. some sequence of memory allocations have already occured. > > > > the test case below is reproducible with djgpp cvs and 2.05+nmemalign.patch > > (patch was found here: > > https://aur.archlinux.org/cgit/aur.git/tree/nmemalign.patch?h=dosbox-djcrx) > > > > tested using freedos 1.2 on physical hardware. > > > > --- > > #include > > #include > > > > #define TEST(alignment, size) \ > > printf("memalign(%d,%d)=", alignment, size); \ > > ptr = memalign(alignment, size); \ > > printf("%p\n", ptr); > > > > int main() > > { > > void * ptr; > > TEST(1, 46) > > TEST(4, 583) > > TEST(64, 773) > > TEST(32, 889) /* <-- reliably fails here */ > > return 0; > > } > > Did a quick build & run, reproduces under dosemu and dosbox, too. Apologies for reviving an old thread, but I think I found the root cause of this bug. The problem is in the branch where the allocated block is not pre-aligned to the desired alignment, which means the block needs to be split into two parts and the first one freed. The size passed to __nmalloc_split(), which is supposed to include the space for the memblock header plus the allocated data region, is calculated as `alignment - misalign`, which is always too small. For example, if the requested alignment is 16 bytes, the calculated size for the split will be 15 bytes, which is too small to contain a memblock structure (20+ bytes). This will result in a corrupted free list when the newly-created memblock is written on top of part of the original one. The size passed to nmalloc() includes extra space (3 * ALIGN in the XTRA definition) to ensure that there is room to insert another memblock header, but the split calculation never uses it. One possible fix (patch below) is to calculate the region to be returned by adding XTRA (which includes the alignment plus the minimal space needed for a new memblock), adjusting the resulting pointer down so that it will be a multiple of the requested alignment (this will always be within the bounds of the allocation due to the XTRA addition), then using the MEMBLKp macro to calculate the appropriate position for the new memblock to be created by the split, and finally using the difference between the original and aligned memblocks as the size for the __nmalloc_split() call. This ensures that the returned pointer is always actually aligned to the requested value, which I don't think the old code would have done even assuming it did not corrupt the free list - the `misalign` calculation is based on the original data location, but the split would adjust this pointer by DATAOFFSET when adding the new memblock, so the resulting split block would have always been misaligned. --- src/libc/ansi/stdlib/nmalign.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libc/ansi/stdlib/nmalign.c b/src/libc/ansi/stdlib/nmalign.c index 541d1a8b..90ee4c4b 100644 --- a/src/libc/ansi/stdlib/nmalign.c +++ b/src/libc/ansi/stdlib/nmalign.c @@ -110,8 +110,9 @@ static inline int invalid(size_t alignment) void *nmemalign(size_t alignment, size_t size) { - memblockp m = NULL; - void *minit; + memblockp m = NULL, m1; + char *minit; + char *malign; ulong misalign; size_t szneed, sz = size; /* preserve arg for hooks */ @@ -141,7 +142,9 @@ void *nmemalign(size_t alignment, size_t size) /* two or more chunks to release */ DBGPRTM(" Complex case, release multiple chunks"); DBGEOLN; - nfree(PTR(__nmalloc_split(&m, alignment - misalign))); + malign = (char *)(((ulong)minit + XTRA - 1) & ~(alignment - 1)); + m1 = MEMBLKp(malign); + nfree(PTR(__nmalloc_split(&m, (ulong)m1 - (ulong)m))); return nrealloc(PTR(m), size); } } /* alignment > ALIGN */ --