mailing list of musl libc
 help / color / mirror / code / Atom feed
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 --]

      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).