зеркало из https://github.com/github/ruby.git
Fix WeakMap use-after-free
[Bug #20688] We cannot free the weakmap_entry before the ST_DELETE because it could hash the key which would read the weakmap_entry and would cause a use-after-free. Instead, we store the entry and free it on the next iteration. For example, the following script triggers a use-after-free in Valgrind: weakmap = ObjectSpace::WeakMap.new 10_000.times { weakmap[Object.new] = Object.new } ==25795== Invalid read of size 8 ==25795== at 0x462297: wmap_cmp (weakmap.c:165) ==25795== by 0x3A2B1C: find_table_bin_ind (st.c:930) ==25795== by 0x3A5EAA: st_general_foreach (st.c:1599) ==25795== by 0x3A5EAA: rb_st_foreach (st.c:1640) ==25795== by 0x25C991: gc_mark_children (default.c:4870) ==25795== by 0x25C991: gc_marks_wb_unprotected_objects_plane (default.c:5565) ==25795== by 0x25C991: rgengc_rememberset_mark_plane (default.c:5557) ==25795== by 0x25C991: rgengc_rememberset_mark (default.c:6233) ==25795== by 0x25C991: gc_marks_start (default.c:6057) ==25795== by 0x25C991: gc_marks (default.c:6077) ==25795== by 0x25C991: gc_start (default.c:6723) ==25795== by 0x260F96: heap_prepare (default.c:2282) ==25795== by 0x260F96: heap_next_free_page (default.c:2489) ==25795== by 0x260F96: newobj_cache_miss (default.c:2598) ==25795== by 0x26197F: newobj_alloc (default.c:2622) ==25795== by 0x26197F: rb_gc_impl_new_obj (default.c:2701) ==25795== by 0x26197F: newobj_of (gc.c:890) ==25795== by 0x26197F: rb_wb_protected_newobj_of (gc.c:917) ==25795== by 0x2DEA88: rb_class_allocate_instance (object.c:131) ==25795== by 0x2E3B18: class_call_alloc_func (object.c:2141) ==25795== by 0x2E3B18: rb_class_alloc (object.c:2113) ==25795== by 0x2E3B18: rb_class_new_instance_pass_kw (object.c:2172) ==25795== by 0x429DDC: vm_call_cfunc_with_frame_ (vm_insnhelper.c:3786) ==25795== by 0x44B08D: vm_sendish (vm_insnhelper.c:5953) ==25795== by 0x44B08D: vm_exec_core (insns.def:898) ==25795== by 0x43A7A4: rb_vm_exec (vm.c:2564) ==25795== by 0x234914: rb_ec_exec_node (eval.c:281) ==25795== Address 0x21603710 is 0 bytes inside a block of size 16 free'd ==25795== at 0x4849B2C: free (vg_replace_malloc.c:989) ==25795== by 0x249651: rb_gc_impl_free (default.c:8527) ==25795== by 0x249651: rb_gc_impl_free (default.c:8508) ==25795== by 0x249651: ruby_sized_xfree.constprop.0 (gc.c:4178) ==25795== by 0x4626EC: ruby_sized_xfree_inlined (gc.h:277) ==25795== by 0x4626EC: wmap_free_entry (weakmap.c:45) ==25795== by 0x4626EC: wmap_mark_weak_table_i (weakmap.c:61) ==25795== by 0x3A5CEF: apply_functor (st.c:1633) ==25795== by 0x3A5CEF: st_general_foreach (st.c:1543) ==25795== by 0x3A5CEF: rb_st_foreach (st.c:1640) ==25795== by 0x25C991: gc_mark_children (default.c:4870) ==25795== by 0x25C991: gc_marks_wb_unprotected_objects_plane (default.c:5565) ==25795== by 0x25C991: rgengc_rememberset_mark_plane (default.c:5557) ==25795== by 0x25C991: rgengc_rememberset_mark (default.c:6233) ==25795== by 0x25C991: gc_marks_start (default.c:6057) ==25795== by 0x25C991: gc_marks (default.c:6077) ==25795== by 0x25C991: gc_start (default.c:6723) ==25795== by 0x260F96: heap_prepare (default.c:2282) ==25795== by 0x260F96: heap_next_free_page (default.c:2489) ==25795== by 0x260F96: newobj_cache_miss (default.c:2598) ==25795== by 0x26197F: newobj_alloc (default.c:2622) ==25795== by 0x26197F: rb_gc_impl_new_obj (default.c:2701) ==25795== by 0x26197F: newobj_of (gc.c:890) ==25795== by 0x26197F: rb_wb_protected_newobj_of (gc.c:917) ==25795== by 0x2DEA88: rb_class_allocate_instance (object.c:131) ==25795== by 0x2E3B18: class_call_alloc_func (object.c:2141) ==25795== by 0x2E3B18: rb_class_alloc (object.c:2113) ==25795== by 0x2E3B18: rb_class_new_instance_pass_kw (object.c:2172) ==25795== by 0x429DDC: vm_call_cfunc_with_frame_ (vm_insnhelper.c:3786) ==25795== by 0x44B08D: vm_sendish (vm_insnhelper.c:5953) ==25795== by 0x44B08D: vm_exec_core (insns.def:898) ==25795== by 0x43A7A4: rb_vm_exec (vm.c:2564) ==25795== Block was alloc'd at ==25795== at 0x484680F: malloc (vg_replace_malloc.c:446) ==25795== by 0x25CE9E: rb_gc_impl_malloc (default.c:8542) ==25795== by 0x462A39: wmap_aset_replace (weakmap.c:423) ==25795== by 0x3A5542: rb_st_update (st.c:1487) ==25795== by 0x462B8E: wmap_aset (weakmap.c:452) ==25795== by 0x429DDC: vm_call_cfunc_with_frame_ (vm_insnhelper.c:3786) ==25795== by 0x44B08D: vm_sendish (vm_insnhelper.c:5953) ==25795== by 0x44B08D: vm_exec_core (insns.def:898) ==25795== by 0x43A7A4: rb_vm_exec (vm.c:2564) ==25795== by 0x234914: rb_ec_exec_node (eval.c:281) ==25795== by 0x2369B8: ruby_run_node (eval.c:319) ==25795== by 0x15D675: rb_main (main.c:43) ==25795== by 0x15D675: main (main.c:62)
This commit is contained in:
Родитель
34bf724a9b
Коммит
df9a6aa943
|
@ -258,4 +258,11 @@ class TestWeakMap < Test::Unit::TestCase
|
|||
|
||||
assert_equal b, @wm[1]
|
||||
end
|
||||
|
||||
def test_use_after_free_bug_20688
|
||||
assert_normal_exit(<<~RUBY)
|
||||
weakmap = ObjectSpace::WeakMap.new
|
||||
10_000.times { weakmap[Object.new] = Object.new }
|
||||
RUBY
|
||||
end
|
||||
end
|
||||
|
|
16
weakmap.c
16
weakmap.c
|
@ -43,6 +43,8 @@ wmap_live_p(VALUE obj)
|
|||
struct wmap_foreach_data {
|
||||
int (*func)(struct weakmap_entry *, st_data_t);
|
||||
st_data_t arg;
|
||||
|
||||
struct weakmap_entry *dead_entry;
|
||||
};
|
||||
|
||||
static int
|
||||
|
@ -50,6 +52,11 @@ wmap_foreach_i(st_data_t key, st_data_t val, st_data_t arg)
|
|||
{
|
||||
struct wmap_foreach_data *data = (struct wmap_foreach_data *)arg;
|
||||
|
||||
if (data->dead_entry != NULL) {
|
||||
ruby_sized_xfree(data->dead_entry, sizeof(struct weakmap_entry));
|
||||
data->dead_entry = NULL;
|
||||
}
|
||||
|
||||
struct weakmap_entry *entry = (struct weakmap_entry *)key;
|
||||
RUBY_ASSERT(&entry->val == (VALUE *)val);
|
||||
|
||||
|
@ -65,7 +72,11 @@ wmap_foreach_i(st_data_t key, st_data_t val, st_data_t arg)
|
|||
return ret;
|
||||
}
|
||||
else {
|
||||
ruby_sized_xfree(entry, sizeof(struct weakmap_entry));
|
||||
/* We cannot free the weakmap_entry here because the ST_DELETE could
|
||||
* hash the key which would read the weakmap_entry and would cause a
|
||||
* use-after-free. Instead, we store this entry and free it on the next
|
||||
* iteration. */
|
||||
data->dead_entry = entry;
|
||||
|
||||
return ST_DELETE;
|
||||
}
|
||||
|
@ -77,9 +88,12 @@ wmap_foreach(struct weakmap *w, int (*func)(struct weakmap_entry *, st_data_t),
|
|||
struct wmap_foreach_data foreach_data = {
|
||||
.func = func,
|
||||
.arg = arg,
|
||||
.dead_entry = NULL,
|
||||
};
|
||||
|
||||
st_foreach(w->table, wmap_foreach_i, (st_data_t)&foreach_data);
|
||||
|
||||
ruby_sized_xfree(foreach_data.dead_entry, sizeof(struct weakmap_entry));
|
||||
}
|
||||
|
||||
static int
|
||||
|
|
Загрузка…
Ссылка в новой задаче