Project

General

Profile

Bug #14834

rb_profile_frames SEGV when PC adjusted on IFUNC

Added by kivikakk (Ashe Connor) 16 days ago. Updated 16 days ago.

Status:
Assigned
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.6.0dev (2018-06-08 trunk 63606) [x86_64-linux]
[ruby-core:87449]

Description

Since r62052, we increment ec->cfp->pc by one pointer width (e.g. 8 bytes) in gc_event_hook_body around the EXEC_EVENT_HOOK call.

This becomes a problem when the hook is on an IFUNC: in this case, pc == 0x0, meaning we increment it to a non-zero value during that call.

rb_profile_frames uses the following check to determine if frame info should be recorded:

    if (cfp->iseq && cfp->pc) {

The example here is stackprof, which calls rb_profile_frames in a gc event hook. This will segfault currently, as the above check will pass.

calc_lineno then attempts to calculate the line number:

        size_t pos = (size_t)(pc - iseq->body->iseq_encoded);

This fails for a variety of reasons: iseq_encoded isn't valid because iseq isn't an rb_iseq_t underneath, producing an essentially random value, and pc is 0x8, so we underflow and eventually cause an overrun in succ_index_lookup with a huge pos argument.

We instead only adjust PC if it appears to be a valid pointer in the first place.

pc-treatment.diff (777 Bytes) pc-treatment.diff kivikakk (Ashe Connor), 06/08/2018 05:06 AM

History

#1 [ruby-core:87450] Updated by kivikakk (Ashe Connor) 16 days ago

It's also worth noting:

  • is the increment of pc with ec->cfp->pc++ correct? What if the instruction is multiple values wide?
  • there are similar pc increment/decrement pairs around EXEC_EVENT_HOOK calls in vm_trace. Do we need to address these too?

Ping tenderlovemaking (Aaron Patterson) as he helped me with this.

#2 [ruby-core:87451] Updated by kivikakk (Ashe Connor) 16 days ago

Further:

  • is the original change correct? Some instructions have the handle_frame? property, which means they do increment the PC by the instruction width first. It's those that don't have that property which now increment the PC after. The gc hook caller always increments PC, assuming non-handle_frame? instructions.

#3 [ruby-core:87453] Updated by shyouhei (Shyouhei Urabe) 16 days ago

  • Assignee set to shyouhei (Shyouhei Urabe)
  • Status changed from Open to Assigned

Thanks reporting! Will handle it.

Also available in: Atom PDF