New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid read in OP_RETURN #3603

Closed
clayton-shopify opened this Issue Apr 11, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@clayton-shopify
Contributor

clayton-shopify commented Apr 11, 2017

The following input to mirb demonstrates a crash:

def g
  raise "test"
end

def g
  def f
    begin
      g
    rescue => e
      raise e
    end
  end

  a f
  begin
    g
  rescue => e
    raise e
  end
end

begin
  g
rescue => e
  raise e
end

a f
begin
  g
rescue => e
  raise e
end

a f
begin
  g
rescue => e
  raise e
end

Note that this input must be supplied to mirb, not mruby.

ASAN report:

==12865==ERROR: AddressSanitizer: SEGV on unknown address 0x7463656a625f (pc 0x000104773098 bp 0x7fff5b606730 sp 0x7fff5b5fec20 T0)
==12865==The signal is caused by a READ memory access.
    #0 0x104773097 in mrb_vm_exec (mirb:x86_64+0x10017c097)
    #1 0x10475e4c9 in mrb_vm_run (mirb:x86_64+0x1001674c9)
    #2 0x1045f96b9 in main mirb.c:549
    #3 0x7fffbbbba234 in start (libdyld.dylib:x86_64+0x5234)

==12865==Register values:
rax = 0x00007463656a625f  rbx = 0xf2f20000f2f2f200  rcx = 0x00007463656a625f  rdx = 0x00001e8c6cad4c4b
rdi = 0x0000100000000000  rsi = 0x0000100000000000  rbp = 0x00007fff5b606730  rsp = 0x00007fff5b5fec20
 r8 = 0x0000100000000000   r9 = 0x00001c5e00001400  r10 = 0x0000100000000000  r11 = 0x00001ecf5b5f1440
r12 = 0xf2f20000f1f1f1f1  r13 = 0x00001fffeb6c0b3c  r14 = 0xf2f20000f2f2f2f2  r15 = 0xf2f2f2f2f2040000
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (mirb:x86_64+0x10017c097) in mrb_vm_exec
==12865==ABORTING
Abort trap: 6

Backtrace:

#0  0x000000000040e7b7 in mrb_vm_exec (mrb=0x6b1010, proc=0x13fb860, pc=0x72c57c) at /home/clayton/git/mruby/src/vm.c:1718
#1  0x000000000040ab53 in mrb_vm_run (mrb=0x6b1010, proc=0x1ad56f0, self=..., stack_keep=5) at /home/clayton/git/mruby/src/vm.c:824
#2  0x0000000000402f3f in main (argc=2, argv=0x7fffffffe488) at /home/clayton/git/mruby/mrbgems/mruby-bin-mirb/tools/mirb/mirb.c:549

This issue was reported by https://hackerone.com/ssarong

@clayton-shopify

This comment has been minimized.

Show comment
Hide comment
@clayton-shopify

clayton-shopify Apr 14, 2017

Contributor

A simpler input producing the same crash:

def g
  def f
    g
  rescue => e
  end

  a f
rescue => e
end

g

f
f
f
f
f
f
f
f
f
f

Adding GC.disable prevents the crash, which suggests a GC bug.

Contributor

clayton-shopify commented Apr 14, 2017

A simpler input producing the same crash:

def g
  def f
    g
  rescue => e
  end

  a f
rescue => e
end

g

f
f
f
f
f
f
f
f
f
f

Adding GC.disable prevents the crash, which suggests a GC bug.

@clayton-shopify

This comment has been minimized.

Show comment
Hide comment
@clayton-shopify

clayton-shopify Apr 16, 2017

Contributor

It seems the proc associated with the f that is defined (on line 2) just before a SystemStackError is later garbage collected during a subsequent execution of f. Changing mark_context_stack to mark_context in final_marking_phase seems to stop the crash by ensuring that f gets marked, but I don't understand the garbage collector well enough to know if that's the right fix.

Contributor

clayton-shopify commented Apr 16, 2017

It seems the proc associated with the f that is defined (on line 2) just before a SystemStackError is later garbage collected during a subsequent execution of f. Changing mark_context_stack to mark_context in final_marking_phase seems to stop the crash by ensuring that f gets marked, but I don't understand the garbage collector well enough to know if that's the right fix.

@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz Apr 17, 2017

Member

Thank you for the hint. That would be great help.

Member

matz commented Apr 17, 2017

Thank you for the hint. That would be great help.

@matz matz closed this in 7614939 Apr 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment