From: Dominique MARTINET <dominique.martinet@atmark-techno.com>
To: Rich Felker <dalias@libc.org>
Cc: musl@lists.openwall.com
Subject: Re: [musl] infinite loop in mallocng's try_avail
Date: Fri, 27 Jan 2023 15:20:51 +0900 [thread overview]
Message-ID: <Y9NtQ8SXevqZZWCx@atmark-techno.com> (raw)
In-Reply-To: <Y9DQxQs0t8rBoNLC@atmark-techno.com>
[-- Attachment #1: Type: text/plain, Size: 1496 bytes --]
Dominique MARTINET wrote on Wed, Jan 25, 2023 at 03:48:37PM +0900:
> I'll add a circular buffer to log things like the active[0] at entry and
> its mask values, then set my board up to reproduce again, which will
> probably bring us to next Monday.
I've reproduced with that, it seems to confirm that we entered
try_avail() with m->avail == 0 and the next element had freed == 0...
(format: '__func__ (__LINE__): <text>', m is printed with %p,
masks with %x -- lines moved due to the debug statements, I've attached
both the patch and full log to this mail for history, however ugly the
code is)
In particular, m->next is logged as identical to m here, but when
looking at gdb "almost immediately" after we can see that m->next isn't
m anymore:
----
alloc_slot (324): 0x2436f40: avail 0, freed 0
try_avail (145): new m: 0x2436f88, avail 3ffffffe, freed 0
try_avail (171): mask 0, mem active_idx: 29, m/m->next 0x2436f88/0x2436f88
try_avail (178): BUGGED
(gdb) p (*pm)
$6 = (struct meta *) 0x2436f88
(gdb) p (*pm)->next
$8 = (struct meta *) 0x2436ee0
----
This is on a single core arm board (i.MX6 ULL), so there should be no
room for cache problems, and there aren't any thread, but... openrc
handles SIGCHLD, and I just confirmed it calls free() in its signal
handler.....
Since malloc/free aren't signal-safe, that explains everything we've
seen and it's a bug I can now fix in openrc (also quite recomforting to
confirm this isn't a musl bug)
Thank you for your help!
--
Dominique
[-- Attachment #2: 0001-mallocng-debug-statements.patch --]
[-- Type: text/x-diff, Size: 3416 bytes --]
From d95eba3d44b7e3154fc2a89755494f80d49e0e59 Mon Sep 17 00:00:00 2001
From: Dominique Martinet <dominique.martinet@atmark-techno.com>
Date: Wed, 25 Jan 2023 16:19:37 +0900
Subject: [PATCH] mallocng debug statements
---
src/malloc/mallocng/malloc.c | 43 ++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/src/malloc/mallocng/malloc.c b/src/malloc/mallocng/malloc.c
index d695ab8ec982..99855e7e0bd9 100644
--- a/src/malloc/mallocng/malloc.c
+++ b/src/malloc/mallocng/malloc.c
@@ -39,6 +39,24 @@ static const uint8_t med_cnt_tab[4] = { 28, 24, 20, 32 };
struct malloc_context ctx = { 0 };
+char dbg2_buf[1024*3] = { 0 };
+size_t dbg2_off = 0;
+
+#define dbg2(fmt, args...) do { \
+ if (dbg2_off > sizeof(dbg2_buf) - 100) dbg2_off = 0; \
+ dbg2_off += snprintf(dbg2_buf + dbg2_off, sizeof(dbg2_buf) - dbg2_off - 1, \
+ "%s (%d): " fmt "\n", __func__, __LINE__, ##args); \
+} while (0)
+
+char dbg_buf[1024*100] = { 0 };
+size_t dbg_off = 0;
+
+#define dbg(fmt, args...) do { \
+ if (dbg_off > sizeof(dbg_buf) - 200) dbg_off = 0; \
+ dbg_off += snprintf(dbg_buf + dbg_off, sizeof(dbg_buf) - dbg_off - 1, \
+ "%s (%d): " fmt "\n", __func__, __LINE__, ##args); \
+} while (0)
+
struct meta *alloc_meta(void)
{
struct meta *m;
@@ -123,9 +141,13 @@ static uint32_t try_avail(struct meta **pm)
dequeue(pm, m);
m = *pm;
if (!m) return 0;
+ if (m->sizeclass == 0)
+ dbg("new m: %p, avail %x, freed %x", m, m->avail_mask, m->freed_mask);
} else {
m = m->next;
*pm = m;
+ if (m->sizeclass == 0)
+ dbg("new m: %p, avail %x, freed %x", m, m->avail_mask, m->freed_mask);
}
mask = m->freed_mask;
@@ -136,6 +158,8 @@ static uint32_t try_avail(struct meta **pm)
m = m->next;
*pm = m;
mask = m->freed_mask;
+ if (m->sizeclass == 0)
+ dbg("new m: %p, avail %x, freed %x", m, m->avail_mask, m->freed_mask);
}
// activate more slots in a not-fully-active group
@@ -143,10 +167,19 @@ static uint32_t try_avail(struct meta **pm)
// any other group with free slots. this avoids
// touching & dirtying as-yet-unused pages.
if (!(mask & ((2u<<m->mem->active_idx)-1))) {
+ if (m->sizeclass == 0) {
+ dbg("mask %x, mem active_idx: %d, m/m->next %p/%p", mask, m->mem->active_idx, m, m->next);
+ }
if (m->next != m) {
m = m->next;
*pm = m;
} else {
+ if (m->sizeclass == 0) {
+ dbg("BUGGED");
+ char msg[] = "\n\nSHOULD NEVER GET HERE!\n\n\n";
+ write(2, msg, sizeof(msg));
+ while(1);
+ }
int cnt = m->mem->active_idx + 2;
int size = size_classes[m->sizeclass]*UNIT;
int span = UNIT + size*cnt;
@@ -280,11 +313,18 @@ static struct meta *alloc_group(int sc, size_t req)
m->last_idx = cnt-1;
m->freeable = 1;
m->sizeclass = sc;
+ dbg2("%p: sc %d idx %d", m, sc, active_idx);
+ dbg("%p: sc %d idx %d", m, sc, active_idx);
return m;
}
static int alloc_slot(int sc, size_t req)
{
+ if (sc == 0) {
+ dbg("%p: avail %x, freed %x", ctx.active[sc],
+ ctx.active[sc] ? ctx.active[sc]->avail_mask : 0,
+ ctx.active[sc] ? ctx.active[sc]->freed_mask : 0);
+ }
uint32_t first = try_avail(&ctx.active[sc]);
if (first) return a_ctz_32(first);
@@ -293,6 +333,9 @@ static int alloc_slot(int sc, size_t req)
g->avail_mask--;
queue(&ctx.active[sc], g);
+ if (sc == 0) {
+ dbg("%p: avail %x", g, g->avail_mask);
+ }
return 0;
}
--
2.39.0
[-- Attachment #3: dbg.log.xz --]
[-- Type: application/x-xz, Size: 3356 bytes --]
prev parent reply other threads:[~2023-01-27 6:21 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-23 1:33 Dominique MARTINET
2023-01-24 8:37 ` Rich Felker
2023-01-25 0:33 ` Dominique MARTINET
2023-01-25 5:53 ` Rich Felker
2023-01-25 6:48 ` Dominique MARTINET
2023-01-27 6:20 ` Dominique MARTINET [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y9NtQ8SXevqZZWCx@atmark-techno.com \
--to=dominique.martinet@atmark-techno.com \
--cc=dalias@libc.org \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).