Project

General

Profile

Actions

Bug #20688

closed

Use-after-free in WeakMap and WeakKeyMap

Added by peterzhu2118 (Peter Zhu) 2 months ago. Updated 2 months ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:118910]

Description

GitHub PR: https://github.com/ruby/ruby/pull/11421

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)
Actions #1

Updated by peterzhu2118 (Peter Zhu) 2 months ago

  • Status changed from Open to Closed

Applied in changeset commit:git|df9a6aa94330cbf414afcd957d1b87defc67e1c5.


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)

Updated by peterzhu2118 (Peter Zhu) 2 months ago

I opened a PR to backport to Ruby 3.3: https://github.com/ruby/ruby/pull/11439

Actions #3

Updated by peterzhu2118 (Peter Zhu) 2 months ago

  • Backport changed from 3.1: DONTNEED, 3.2: DONTNEED, 3.3: REQUIRED to 3.1: DONTNEED, 3.2: DONTNEED, 3.3: DONE
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0