kotovalexarian-likes-github/ruby--ruby
1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00
Code Releases Activity

Fix use-after-free with interacting TracePoints

Browse source 
`vm_trace_hook()` runs global hooks before running local hooks.
Previously, we read the local hook list before running the global hooks
which led to use-after-free when a global hook frees the local hook
list. A global hook can do this by disabling a local TracePoint, for
example.

Delay local hook list loading until after running the global hooks.

Issue discovered by Jeremy Evans in GH-5862.

[Bug #18730]
This commit is contained in:
Alan Wu 2022-04-29 18:54:16 -04:00
parent 3bef9584a8
commit a687756284
Notes: git 2022-05-31 02:54:45 +09:00
3 changed files with 49 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

17
test/ruby/test_gc_compact.rb
View file

@ -200,4 +200,21 @@ class TestGCCompact < Test::Unit::TestCase
GC.compact
assert_equal count + 1, GC.stat(:compact_count)
end
def test_compacting_from_trace_point
obj = Object.new
def obj.tracee
:ret # expected to emit both line and call event from one instruction
end
results = []
TracePoint.new(:call, :line) do |tp|
results << tp.event
GC.verify_compaction_references
end.enable(target: obj.method(:tracee)) do
obj.tracee
end
assert_equal([:call, :line], results)
end
end

14
test/ruby/test_settracefunc.rb
View file

@ -2561,6 +2561,20 @@ CODE
end
bar
EOS
assert_normal_exit(<<-EOS, 'Bug #18730')
def bar
42
end
tp_line = TracePoint.new(:line) do |tp0|
tp_multi1 = TracePoint.new(:return, :b_return, :line) do |tp|
tp0.disable
end
tp_multi1.enable
end
tp_line.enable(target: method(:bar))
bar
EOS
end
def test_stat_exists

23
vm_insnhelper.c
View file

@ -5629,7 +5629,7 @@ NOINLINE(static void vm_trace(rb_execution_context_t *ec, rb_control_frame_t *re
static inline void
vm_trace_hook(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, const VALUE *pc,
rb_event_flag_t pc_events, rb_event_flag_t target_event,
rb_hook_list_t *global_hooks, rb_hook_list_t *local_hooks, VALUE val)
rb_hook_list_t *global_hooks, rb_hook_list_t *const *local_hooks_ptr, VALUE val)
{
rb_event_flag_t event = pc_events & target_event;
VALUE self = GET_SELF();
@ -5644,6 +5644,8 @@ vm_trace_hook(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, const VAL
reg_cfp->pc--;
}
// Load here since global hook above can add and free local hooks
rb_hook_list_t *local_hooks = *local_hooks_ptr;
if (local_hooks != NULL) {
if (event & local_hooks->events) {
/* increment PC because source line is calculated with PC-1 */
@ -5672,7 +5674,7 @@ rb_vm_opt_cfunc_p(CALL_CACHE cc, int insn)
#define VM_TRACE_HOOK(target_event, val) do { \
if ((pc_events & (target_event)) & enabled_flags) { \
vm_trace_hook(ec, reg_cfp, pc, pc_events, (target_event), global_hooks, local_hooks, (val)); \
vm_trace_hook(ec, reg_cfp, pc, pc_events, (target_event), global_hooks, local_hooks_ptr, (val)); \
} \
} while (0)
@ -5688,13 +5690,16 @@ vm_trace(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp)
}
else {
const rb_iseq_t *iseq = reg_cfp->iseq;
VALUE iseq_val = (VALUE)iseq;
size_t pos = pc - ISEQ_BODY(iseq)->iseq_encoded;
rb_event_flag_t pc_events = rb_iseq_event_flags(iseq, pos);
rb_hook_list_t *local_hooks = iseq->aux.exec.local_hooks;
rb_hook_list_t *const *local_hooks_ptr = &iseq->aux.exec.local_hooks;
rb_event_flag_t iseq_local_events = local_hooks != NULL ? local_hooks->events : 0;
rb_hook_list_t *bmethod_local_hooks = NULL;
rb_hook_list_t **bmethod_local_hooks_ptr = NULL;
rb_event_flag_t bmethod_local_events = 0;
bool bmethod_frame = VM_FRAME_BMETHOD_P(reg_cfp);
const bool bmethod_frame = VM_FRAME_BMETHOD_P(reg_cfp);
enabled_flags |= iseq_local_events;
VM_ASSERT((iseq_local_events & ~ISEQ_TRACE_EVENTS) == 0);
@ -5703,6 +5708,7 @@ vm_trace(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp)
const rb_callable_method_entry_t *me = rb_vm_frame_method_entry(reg_cfp);
VM_ASSERT(me->def->type == VM_METHOD_TYPE_BMETHOD);
bmethod_local_hooks = me->def->body.bmethod.hooks;
bmethod_local_hooks_ptr = &me->def->body.bmethod.hooks;
if (bmethod_local_hooks) {
bmethod_local_events = bmethod_local_hooks->events;
}
@ -5745,7 +5751,7 @@ vm_trace(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp)
/* check traces */
if ((pc_events & RUBY_EVENT_B_CALL) && bmethod_frame && (bmethod_events & RUBY_EVENT_CALL)) {
/* b_call instruction running as a method. Fire call event. */
vm_trace_hook(ec, reg_cfp, pc, RUBY_EVENT_CALL, RUBY_EVENT_CALL, global_hooks, bmethod_local_hooks, Qundef);
vm_trace_hook(ec, reg_cfp, pc, RUBY_EVENT_CALL, RUBY_EVENT_CALL, global_hooks, bmethod_local_hooks_ptr, Qundef);
}
VM_TRACE_HOOK(RUBY_EVENT_CLASS | RUBY_EVENT_CALL | RUBY_EVENT_B_CALL, Qundef);
VM_TRACE_HOOK(RUBY_EVENT_LINE, Qundef);
@ -5754,8 +5760,15 @@ vm_trace(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp)
VM_TRACE_HOOK(RUBY_EVENT_END | RUBY_EVENT_RETURN | RUBY_EVENT_B_RETURN, TOPN(0));
if ((pc_events & RUBY_EVENT_B_RETURN) && bmethod_frame && (bmethod_events & RUBY_EVENT_RETURN)) {
/* b_return instruction running as a method. Fire return event. */
vm_trace_hook(ec, reg_cfp, pc, RUBY_EVENT_RETURN, RUBY_EVENT_RETURN, global_hooks, bmethod_local_hooks, TOPN(0));
vm_trace_hook(ec, reg_cfp, pc, RUBY_EVENT_RETURN, RUBY_EVENT_RETURN, global_hooks, bmethod_local_hooks_ptr, TOPN(0));
}
// Pin the iseq since `local_hooks_ptr` points inside the iseq's slot on the GC heap.
// We need the pointer to stay valid in case compaction happens in a trace hook.
//
// Similar treatment is unnecessary for `bmethod_local_hooks_ptr` since
// storage for `rb_method_definition_t` is not on the GC heap.
RB_GC_GUARD(iseq_val);
}
}
}