Skip to content

Commit 567b375

Browse files
authored
Fix appender code that might not initialize memory that might point at huge allocations (#9084)
1 parent 4ef836c commit 567b375

File tree

1 file changed

+58
-1
lines changed

1 file changed

+58
-1
lines changed

std/array.d

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3607,6 +3607,7 @@ if (isDynamicArray!A)
36073607
}
36083608
else
36093609
{
3610+
import core.stdc.string : memcpy, memset;
36103611
// Time to reallocate.
36113612
// We need to almost duplicate what's in druntime, except we
36123613
// have better access to the capacity field.
@@ -3618,6 +3619,15 @@ if (isDynamicArray!A)
36183619
if (u)
36193620
{
36203621
// extend worked, update the capacity
3622+
// if the type has indirections, we need to zero any new
3623+
// data that we requested, as the existing data may point
3624+
// at large unused blocks.
3625+
static if (hasIndirections!T)
3626+
{
3627+
immutable addedSize = u - (_data.capacity * T.sizeof);
3628+
() @trusted { memset(_data.arr.ptr + _data.capacity, 0, addedSize); }();
3629+
}
3630+
36213631
_data.capacity = u / T.sizeof;
36223632
return;
36233633
}
@@ -3633,10 +3643,20 @@ if (isDynamicArray!A)
36333643

36343644
auto bi = (() @trusted => GC.qalloc(nbytes, blockAttribute!T))();
36353645
_data.capacity = bi.size / T.sizeof;
3636-
import core.stdc.string : memcpy;
36373646
if (len)
36383647
() @trusted { memcpy(bi.base, _data.arr.ptr, len * T.sizeof); }();
3648+
36393649
_data.arr = (() @trusted => (cast(Unqual!T*) bi.base)[0 .. len])();
3650+
3651+
// we requested new bytes that are not in the existing
3652+
// data. If T has pointers, then this new data could point at stale
3653+
// objects from the last time this block was allocated. Zero that
3654+
// new data out, it may point at large unused blocks!
3655+
static if (hasIndirections!T)
3656+
() @trusted {
3657+
memset(bi.base + (len * T.sizeof), 0, (newlen - len) * T.sizeof);
3658+
}();
3659+
36403660
_data.tryExtendBlock = true;
36413661
// leave the old data, for safety reasons
36423662
}
@@ -4011,6 +4031,43 @@ if (isDynamicArray!A)
40114031
app2.toString();
40124032
}
40134033

4034+
// https://issues.dlang.org/show_bug.cgi?id=24856
4035+
@system unittest
4036+
{
4037+
import core.memory : GC;
4038+
import std.stdio : writeln;
4039+
import std.algorithm.searching : canFind;
4040+
GC.disable();
4041+
scope(exit) GC.enable();
4042+
void*[] freeme;
4043+
// generate some poison blocks to allocate from.
4044+
auto poison = cast(void*) 0xdeadbeef;
4045+
foreach (i; 0 .. 10)
4046+
{
4047+
auto blk = new void*[7];
4048+
blk[] = poison;
4049+
freeme ~= blk.ptr;
4050+
}
4051+
4052+
foreach (p; freeme)
4053+
GC.free(p);
4054+
4055+
int tests = 0;
4056+
foreach (i; 0 .. 10)
4057+
{
4058+
Appender!(void*[]) app;
4059+
app.put(null);
4060+
// if not a realloc of one of the deadbeef pointers, continue
4061+
if (!freeme.canFind(app.data.ptr))
4062+
continue;
4063+
++tests;
4064+
assert(!app.data.ptr[0 .. app.capacity].canFind(poison), "Appender not zeroing data!");
4065+
}
4066+
// just notify in the log whether this test actually could be done.
4067+
if (tests == 0)
4068+
writeln("WARNING: test of Appender zeroing did not occur");
4069+
}
4070+
40144071
//Calculates an efficient growth scheme based on the old capacity
40154072
//of data, and the minimum requested capacity.
40164073
//arg curLen: The current length

0 commit comments

Comments
 (0)