Skip to content

Commit

Permalink
Fix out of bounds write in array deserialization
Browse files Browse the repository at this point in the history
The array was allocated based on the serialized `elsize` of the array,
however, unions get an extra selector array after the regular storage
which was not allocated (because we didn't know it was gonna be a union
array at the time when we allocated it). According to
a48eeef we cannot look at the element
type to allocate the array, so we need to serialize a bit to indicate
that we will have a union array.

Fixes #28998
  • Loading branch information
Keno committed Sep 4, 2018
1 parent c8450d8 commit e7d7259
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 15 deletions.
15 changes: 9 additions & 6 deletions src/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ size_t jl_arr_xtralloc_limit = 0;
#define MAXINTVAL (((size_t)-1)>>1)

static jl_array_t *_new_array_(jl_value_t *atype, uint32_t ndims, size_t *dims,
int isunboxed, int elsz)
int isunboxed, int isunion, int elsz)
{
jl_ptls_t ptls = jl_get_ptls_states();
size_t i, tot, nel=1;
Expand All @@ -67,7 +67,7 @@ static jl_array_t *_new_array_(jl_value_t *atype, uint32_t ndims, size_t *dims,
jl_error("invalid Array dimensions");
nel = prod;
}
int isunion = atype != NULL && jl_is_uniontype(jl_tparam0(atype));
assert(atype == NULL || isunion == jl_is_uniontype(jl_tparam0(atype)));
if (isunboxed) {
wideint_t prod = (wideint_t)elsz * (wideint_t)nel;
if (prod > (wideint_t) MAXINTVAL)
Expand Down Expand Up @@ -149,18 +149,19 @@ static inline jl_array_t *_new_array(jl_value_t *atype, uint32_t ndims, size_t *
jl_value_t *eltype = jl_tparam0(atype);
size_t elsz = 0, al = 0;
int isunboxed = jl_islayout_inline(eltype, &elsz, &al);
int isunion = jl_is_uniontype(eltype);
if (!isunboxed) {
elsz = sizeof(void*);
al = elsz;
}

return _new_array_(atype, ndims, dims, isunboxed, elsz);
return _new_array_(atype, ndims, dims, isunboxed, isunion, elsz);
}

jl_array_t *jl_new_array_for_deserialization(jl_value_t *atype, uint32_t ndims, size_t *dims,
int isunboxed, int elsz)
int isunboxed, int isunion, int elsz)
{
return _new_array_(atype, ndims, dims, isunboxed, elsz);
return _new_array_(atype, ndims, dims, isunboxed, isunion, elsz);
}

#ifndef JL_NDEBUG
Expand Down Expand Up @@ -1115,8 +1116,10 @@ JL_DLLEXPORT jl_array_t *jl_array_copy(jl_array_t *ary)
{
size_t elsz = ary->elsize;
size_t len = jl_array_len(ary);
int isunion = jl_is_uniontype(jl_tparam0(jl_typeof(ary)));
jl_array_t *new_ary = _new_array_(jl_typeof(ary), jl_array_ndims(ary),
&ary->nrows, !ary->flags.ptrarray, elsz);
&ary->nrows, !ary->flags.ptrarray,
isunion, elsz);
memcpy(new_ary->data, ary->data, len * elsz);
// ensure isbits union arrays copy their selector bytes correctly
if (jl_array_isbitsunion(ary))
Expand Down
20 changes: 12 additions & 8 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -640,14 +640,15 @@ static void jl_serialize_value_(jl_serializer_state *s, jl_value_t *v, int as_li
}
else if (jl_is_array(v)) {
jl_array_t *ar = (jl_array_t*)v;
if (ar->flags.ndims == 1 && ar->elsize < 128) {
int isunion = jl_is_uniontype(jl_tparam0(jl_typeof(ar)));
if (ar->flags.ndims == 1 && ar->elsize <= 0x3f) {
write_uint8(s->s, TAG_ARRAY1D);
write_uint8(s->s, (ar->flags.ptrarray<<7) | (ar->elsize & 0x7f));
write_uint8(s->s, (ar->flags.ptrarray<<7) | (isunion << 6) | (ar->elsize & 0x3f));
}
else {
write_uint8(s->s, TAG_ARRAY);
write_uint16(s->s, ar->flags.ndims);
write_uint16(s->s, (ar->flags.ptrarray<<15) | (ar->elsize & 0x7fff));
write_uint16(s->s, (ar->flags.ptrarray << 15) | (isunion << 14) | (ar->elsize & 0x3fff));
}
for (i = 0; i < ar->flags.ndims; i++)
jl_serialize_value(s, jl_box_long(jl_array_dim(ar,i)));
Expand Down Expand Up @@ -1211,7 +1212,7 @@ static void write_mod_list(ios_t *s, jl_array_t *a)
}

// "magic" string and version header of .ji file
static const int JI_FORMAT_VERSION = 6;
static const int JI_FORMAT_VERSION = 7;
static const char JI_MAGIC[] = "\373jli\r\n\032\n"; // based on PNG signature
static const uint16_t BOM = 0xFEFF; // byte-order marker
static void write_header(ios_t *s)
Expand Down Expand Up @@ -1496,18 +1497,20 @@ static jl_value_t *jl_deserialize_value_array(jl_serializer_state *s, uint8_t ta
{
int usetable = (s->mode != MODE_IR);
int16_t i, ndims;
int isunboxed, elsize;
int isunboxed, isunion, elsize;
if (tag == TAG_ARRAY1D) {
ndims = 1;
elsize = read_uint8(s->s);
isunboxed = !(elsize >> 7);
elsize = elsize & 0x7f;
isunion = elsize >> 6;
elsize = elsize & 0x3f;
}
else {
ndims = read_uint16(s->s);
elsize = read_uint16(s->s);
isunboxed = !(elsize >> 15);
elsize = elsize & 0x7fff;
isunion = elsize >> 14;
elsize = elsize & 0x3fff;
}
uintptr_t pos = backref_list.len;
if (usetable)
Expand All @@ -1516,7 +1519,8 @@ static jl_value_t *jl_deserialize_value_array(jl_serializer_state *s, uint8_t ta
for (i = 0; i < ndims; i++) {
dims[i] = jl_unbox_long(jl_deserialize_value(s, NULL));
}
jl_array_t *a = jl_new_array_for_deserialization((jl_value_t*)NULL, ndims, dims, isunboxed, elsize);
jl_array_t *a = jl_new_array_for_deserialization(
(jl_value_t*)NULL, ndims, dims, isunboxed, isunion, elsize);
if (usetable)
backref_list.items[pos] = a;
jl_value_t *aty = jl_deserialize_value(s, &jl_astaggedvalue(a)->type);
Expand Down
2 changes: 1 addition & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ JL_DLLEXPORT jl_value_t *jl_argument_datatype(jl_value_t *argt JL_PROPAGATES_ROO
jl_value_t *jl_nth_slot_type(jl_value_t *sig JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT;
void jl_compute_field_offsets(jl_datatype_t *st);
jl_array_t *jl_new_array_for_deserialization(jl_value_t *atype, uint32_t ndims, size_t *dims,
int isunboxed, int elsz);
int isunboxed, int isunion, int elsz);
void jl_module_run_initializer(jl_module_t *m);
jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var) JL_NOTSAFEPOINT;
extern jl_array_t *jl_module_init_order JL_GLOBALLY_ROOTED;
Expand Down
8 changes: 8 additions & 0 deletions test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ try
const x28297 = Result(missing)
# issue #28998
const x28998 = [missing, 2, missing, 6, missing,
missing, missing, missing,
missing, missing, missing,
missing, missing, 6]
let some_method = which(Base.include, (String,))
# global const some_method // FIXME: support for serializing a direct reference to an external Method not implemented
global const some_linfo =
Expand Down Expand Up @@ -180,6 +186,8 @@ try
@test Foo.abigint_x::BigInt + 1 == big"125"

@test Foo.x28297.result === missing

@test Foo.x28998[end] == 6
end

cachedir = joinpath(dir, "compiled", "v$(VERSION.major).$(VERSION.minor)")
Expand Down

0 comments on commit e7d7259

Please sign in to comment.