Skip to content

Commit 0983063

Browse files
mjp41Copilot
andauthored
Detect cyclic corruption in SeqSet::iterate (#813)
* Detect cyclic corruption in SeqSet::iterate Add a production check to SeqSet::iterate that verifies curr->next->prev == curr on each iteration. A corrupted doubly-linked list can form an unexpected cycle, causing iterate to loop infinitely. Using SNMALLOC_CHECK_MSG (rather than SNMALLOC_ASSERT) ensures the check is active in release builds and aborts with a clear diagnostic instead of hanging. This is aimed to rule out an issue observed in #809 (comment) * Add test to check for corruption * Update src/test/func/seqset/seqset.cc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 6dbf08a commit 0983063

2 files changed

Lines changed: 130 additions & 0 deletions

File tree

src/snmalloc/ds_aal/seqset.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@ namespace snmalloc
160160

161161
while (curr != &head)
162162
{
163+
SNMALLOC_CHECK_MSG(
164+
curr->next->prev == curr,
165+
"Cycle detected in SeqSet, aborting to prevent infinite loop.");
163166
// Read next first, as f may remove curr.
164167
auto next = curr->next;
165168
f(containing(curr));

src/test/func/seqset/seqset.cc

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/**
2+
* Test that demonstrates how SeqSet::iterate detects cyclic corruption.
3+
*
4+
* A double-free of small allocations can corrupt the slab metadata's
5+
* doubly-linked list node, creating a cycle that would cause iterate()
6+
* to loop forever. This test creates that corruption by double-inserting
7+
* an element into a SeqSet and verifies that the production SNMALLOC_CHECK
8+
* fires (aborting the process).
9+
*
10+
* The corruption case is run in a forked child so that the expected abort
11+
* does not kill the test harness itself.
12+
*/
13+
#include <snmalloc/snmalloc.h>
14+
#include <test/helpers.h>
15+
#include <test/setup.h>
16+
17+
#ifndef _WIN32
18+
# include <signal.h>
19+
# include <sys/wait.h>
20+
# include <unistd.h>
21+
#endif
22+
23+
using namespace snmalloc;
24+
25+
/**
26+
* A minimal element that can live in a SeqSet.
27+
* The `node` field must be the first member (offset 0).
28+
*/
29+
struct Element
30+
{
31+
SeqSet<Element>::Node node;
32+
int value;
33+
};
34+
35+
/**
36+
* Iterate a corrupted SeqSet and expect the process to abort.
37+
* Called in a forked child process.
38+
*/
39+
void iterate_corrupted_seqset()
40+
{
41+
// After inserting a, b, c the list is:
42+
// head <-> c <-> b <-> a <-> head
43+
//
44+
// Inserting b a second time re-splices it at the front:
45+
// head->next = &b, b->next = old head->next (c), c->next still == &b
46+
// This creates a cycle: b <-> c <-> b ...
47+
// b->prev = &head, but c->prev is still &b from the first insert
48+
//
49+
// When iterate reaches c it checks c->next->prev == c:
50+
// c->next = &b, b->prev = &head, &head != &c → CHECK fires.
51+
//
52+
// This mirrors what happens when a double-free causes the same slab
53+
// metadata to be inserted into the available list twice.
54+
SeqSet<Element> set;
55+
Element a{}, b{}, c{};
56+
a.value = 1;
57+
b.value = 2;
58+
c.value = 3;
59+
60+
set.insert(&a);
61+
set.insert(&b);
62+
set.insert(&c);
63+
64+
// Double-insert b — simulates a double-free reinserting metadata.
65+
set.insert(&b);
66+
67+
int count = 0;
68+
set.iterate([&](Element*) { ++count; });
69+
70+
// Should never be reached.
71+
_exit(1);
72+
}
73+
74+
int main()
75+
{
76+
setup();
77+
78+
START_TEST("SeqSet cycle detection");
79+
80+
// ---- Normal operation: insert three elements and iterate safely ----
81+
{
82+
SeqSet<Element> set;
83+
Element a{}, b{}, c{};
84+
a.value = 1;
85+
b.value = 2;
86+
c.value = 3;
87+
88+
set.insert(&a);
89+
set.insert(&b);
90+
set.insert(&c);
91+
92+
int count = 0;
93+
set.iterate([&](Element*) { ++count; });
94+
EXPECT(count == 3, "Expected 3 elements, got {}", count);
95+
}
96+
97+
// ---- Double-insert: expect the child to be killed by SIGABRT ----
98+
#ifndef _WIN32
99+
{
100+
pid_t pid = fork();
101+
if (pid < 0)
102+
{
103+
// fork() failed; report this as a test failure.
104+
EXPECT(false, "fork() failed");
105+
}
106+
else if (pid == 0)
107+
{
108+
// Child — will abort inside iterate().
109+
iterate_corrupted_seqset();
110+
// unreachable
111+
}
112+
113+
int status = 0;
114+
waitpid(pid, &status, 0);
115+
116+
EXPECT(
117+
WIFSIGNALED(status) && WTERMSIG(status) == SIGABRT,
118+
"Expected child to abort (SIGABRT), got status {}",
119+
status);
120+
}
121+
#else
122+
printf("Skipping corruption sub-test on Windows\n");
123+
#endif
124+
125+
printf("PASSED\n");
126+
return 0;
127+
}

0 commit comments

Comments
 (0)