Fix copying dicts

We didn't properly copy the opaque value before (a heap-allocated allocator
pointer), resulting in a use-after-free of the opaque on the copied map if
the old map was destroyed.
This commit is contained in:
Laria 2022-02-15 22:32:15 +01:00
parent 8df0991415
commit 40320aaa4a
3 changed files with 43 additions and 9 deletions

View file

@ -94,11 +94,22 @@ copy_value(const struct apfl_hashmap map, void *dest, void *src)
}
}
static void
on_deinit(const struct apfl_hashmap map)
static bool
copy_opaque(const struct apfl_hashmap map, void **dest, void *src)
{
if (HAS_CALLBACK(map, on_deinit)) {
INVOKE_CALLBACK_NOARGS(map, on_deinit);
if (HAS_CALLBACK(map, copy_opaque)) {
return map.callbacks.copy_opaque(dest, src);
} else {
*dest = src;
return true;
}
}
static void
deinit_opaque(const struct apfl_hashmap map)
{
if (HAS_CALLBACK(map, deinit_opaque)) {
INVOKE_CALLBACK_NOARGS(map, deinit_opaque);
}
}
@ -363,7 +374,7 @@ apfl_hashmap_deinit(struct apfl_hashmap *map)
map->buckets = NULL;
}
on_deinit(*map);
deinit_opaque(*map);
}
struct apfl_hashmap
@ -381,7 +392,13 @@ apfl_hashmap_copy(struct apfl_hashmap *dst, struct apfl_hashmap src)
size_t keysize = src.keysize;
size_t valsize = src.valsize;
if (!hashmap_init(dst, src.allocator, src.callbacks, src.nbuckets, keysize, valsize)) {
struct apfl_hashmap_callbacks dst_callbacks = src.callbacks;
if (!copy_opaque(src, &dst_callbacks.opaque, src.callbacks.opaque)) {
return false;
}
if (!hashmap_init(dst, src.allocator, dst_callbacks, src.nbuckets, keysize, valsize)) {
deinit_opaque(*dst);
return false;
}

View file

@ -45,8 +45,11 @@ struct apfl_hashmap_callbacks {
// If not provided, the bytes will be copied with memcpy.
void (*copy_value) (void *opaque, void *dest, void *src);
// Copies the opaque value. Returns true on success, false on failure.
bool (*copy_opaque) (void **dst, void *src);
// Called on deinitialization of the hashmap, if provided.
void (*on_deinit) (void *opaque);
void (*deinit_opaque)(void *opaque);
};
struct apfl_hashmap {

View file

@ -85,8 +85,21 @@ dict_copy_key_or_value(void *opaque, void *_dest, void *_src)
*dest = apfl_value_incref(*src);
}
static bool
dict_copy_opaque(void **_dst, void *src)
{
struct apfl_allocator *allocator = src;
struct apfl_allocator *dst;
if ((dst = ALLOC_OBJ(*allocator, struct apfl_allocator)) == NULL) {
return false;
}
*dst = *allocator;
*_dst = dst;
return true;
}
static void
dict_on_deinit(void *opaque)
dict_deinit_opaque(void *opaque)
{
struct apfl_allocator *allocator = opaque;
FREE_OBJ(*allocator, allocator);
@ -112,7 +125,8 @@ init_hashmap_for_dict(struct apfl_allocator allocator, struct apfl_hashmap *map)
.destroy_value = dict_destroy_key_or_value,
.copy_key = dict_copy_key_or_value,
.copy_value = dict_copy_key_or_value,
.on_deinit = dict_on_deinit,
.copy_opaque = dict_copy_opaque,
.deinit_opaque = dict_deinit_opaque,
},
sizeof(struct apfl_value),
sizeof(struct apfl_value)