Skip to content

Fix use-after-free in mrb_obj_alloc()#6329

Merged
matz merged 1 commit into
mruby:masterfrom
dearblue:mrb_obj_alloc
Aug 25, 2024
Merged

Fix use-after-free in mrb_obj_alloc()#6329
matz merged 1 commit into
mruby:masterfrom
dearblue:mrb_obj_alloc

Conversation

@dearblue

@dearblue dearblue commented Aug 24, 2024

Copy link
Copy Markdown
Contributor

When GC occurs during the expansion of the GC arena by gc_protect() in mrb_obj_alloc(), the object page just allocated by add_heap() is released.
Therefore, as soon as control returns from gc_protect(), there is a possibility of illegal writing or reading to the address just released.

This issue was discovered during the investigation of #6326.

I think this problem can be seen in the following build configuration file.
The key is to define MRB_HEAP_PAGE_SIZE=1.

# build configuration file

MRuby::Build.new do
  toolchain
  enable_test
  enable_debug
  defines << %w(MRB_HEAP_PAGE_SIZE=1)
  defines << %w(MRB_GC_STRESS)
  enable_sanitizer "address,undefined"
  gem core: "mruby-proc-ext"
end
...SNIP...

Proc [15.2.17] : .
Proc.new [15.2.17.3.1] : .
Proc#[] [15.2.17.4.1] : .
Proc#arity [15.2.17.4.2] : .
Proc#call [15.2.17.4.3] : .
Proc#call proc args pos block : .
Proc#call proc args pos rest post : =================================================================
==69524==ERROR: AddressSanitizer: heap-use-after-free on address 0x507000ca89b0 at pc 0x00000049e73a bp 0x7ffffffe1d20 sp 0x7ffffffe14d8
WRITE of size 48 at 0x507000ca89b0 thread T0
    #0 0x49e739 in __asan_memcpy /usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:63:3
    #1 0x566742 in mrb_obj_alloc /var/tmp/mruby-204b81c56/src/gc.c:509:8
    #2 0x4f283d in ary_new_capa /var/tmp/mruby-204b81c56/src/array.c:48:22
    #3 0x4f2e3b in ary_new_from_values /var/tmp/mruby-204b81c56/src/array.c:99:22
    #4 0x4f2dec in mrb_ary_new_from_values /var/tmp/mruby-204b81c56/src/array.c:110:22
    #5 0x6ff229 in mrb_vm_exec /var/tmp/mruby-204b81c56/src/vm.c:2654:17
    #6 0x6aae55 in mrb_vm_run /var/tmp/mruby-204b81c56/src/vm.c:1349:12
    #7 0x6a18f8 in mrb_top_run /var/tmp/mruby-204b81c56/src/vm.c:3081:10
    #8 0x597d5c in load_irep /var/tmp/mruby-204b81c56/src/load.c:682:10
    #9 0x5979dd in mrb_load_irep_cxt /var/tmp/mruby-204b81c56/src/load.c:690:10
    #10 0x597df8 in mrb_load_irep /var/tmp/mruby-204b81c56/src/load.c:702:10
    #11 0x4f176a in GENERATED_TMP_mrb_mruby_test_gem_test /var/tmp/mruby-204b81c56/build/sans/mrbgems/mruby-test/gem_test.c:16346:3
    #12 0x4db580 in mrbgemtest_init /var/tmp/mruby-204b81c56/build/sans/mrbgems/mruby-test/mrbtest.c:120:5
    #13 0x4d6a9f in main /var/tmp/mruby-204b81c56/mrbgems/mruby-test/driver.c:299:3
    #14 0x800b9da69 in __libc_start1 (/lib/libc.so.7+0x85a69)
    #15 0x42647f in _start /usr/src/lib/csu/amd64/crt1_s.S:83

0x507000ca89b0 is located 32 bytes inside of 80-byte region [0x507000ca8990,0x507000ca89e0)
freed by thread T0 here:
    #0 0x49fe76 in free /usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3
    #1 0x4f26e3 in mrb_default_allocf /var/tmp/mruby-204b81c56/src/allocf.c:22:5
    #2 0x565761 in mrb_free /var/tmp/mruby-204b81c56/src/gc.c:259:3
    #3 0x5764ba in incremental_sweep_phase /var/tmp/mruby-204b81c56/src/gc.c:1072:7
    #4 0x57233f in incremental_gc /var/tmp/mruby-204b81c56/src/gc.c:1121:20
    #5 0x56bfab in incremental_gc_finish /var/tmp/mruby-204b81c56/src/gc.c:1137:5
    #6 0x56c627 in clear_all_old /var/tmp/mruby-204b81c56/src/gc.c:1167:3
    #7 0x564ac7 in mrb_full_gc /var/tmp/mruby-204b81c56/src/gc.c:1225:5
    #8 0x5641f6 in mrb_realloc_simple /var/tmp/mruby-204b81c56/src/gc.c:194:5
    #9 0x56526f in mrb_realloc /var/tmp/mruby-204b81c56/src/gc.c:211:8
    #10 0x568b7c in gc_protect /var/tmp/mruby-204b81c56/src/gc.c:387:34
    #11 0x56672a in mrb_obj_alloc /var/tmp/mruby-204b81c56/src/gc.c:508:3
    #12 0x4f283d in ary_new_capa /var/tmp/mruby-204b81c56/src/array.c:48:22
    #13 0x4f2e3b in ary_new_from_values /var/tmp/mruby-204b81c56/src/array.c:99:22
    #14 0x4f2dec in mrb_ary_new_from_values /var/tmp/mruby-204b81c56/src/array.c:110:22
    #15 0x6ff229 in mrb_vm_exec /var/tmp/mruby-204b81c56/src/vm.c:2654:17
    #16 0x6aae55 in mrb_vm_run /var/tmp/mruby-204b81c56/src/vm.c:1349:12
    #17 0x6a18f8 in mrb_top_run /var/tmp/mruby-204b81c56/src/vm.c:3081:10
    #18 0x597d5c in load_irep /var/tmp/mruby-204b81c56/src/load.c:682:10
    #19 0x5979dd in mrb_load_irep_cxt /var/tmp/mruby-204b81c56/src/load.c:690:10
    #20 0x597df8 in mrb_load_irep /var/tmp/mruby-204b81c56/src/load.c:702:10
    #21 0x4f176a in GENERATED_TMP_mrb_mruby_test_gem_test /var/tmp/mruby-204b81c56/build/sans/mrbgems/mruby-test/gem_test.c:16346:3
    #22 0x4db580 in mrbgemtest_init /var/tmp/mruby-204b81c56/build/sans/mrbgems/mruby-test/mrbtest.c:120:5
    #23 0x4d6a9f in main /var/tmp/mruby-204b81c56/mrbgems/mruby-test/driver.c:299:3
    #24 0x800b9da69 in __libc_start1 (/lib/libc.so.7+0x85a69)
    #25 0x42647f in _start /usr/src/lib/csu/amd64/crt1_s.S:83
    #26 0x800a5b007  (<unknown module>)

previously allocated by thread T0 here:
    #0 0x4a0217 in realloc /usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:82:3
    #1 0x4f26fd in mrb_default_allocf /var/tmp/mruby-204b81c56/src/allocf.c:27:12
    #2 0x56439a in mrb_realloc_simple /var/tmp/mruby-204b81c56/src/gc.c:197:8
    #3 0x56526f in mrb_realloc /var/tmp/mruby-204b81c56/src/gc.c:211:8
    #4 0x5654a8 in mrb_malloc /var/tmp/mruby-204b81c56/src/gc.c:227:10
    #5 0x56557f in mrb_calloc /var/tmp/mruby-204b81c56/src/gc.c:245:9
    #6 0x56788d in add_heap /var/tmp/mruby-204b81c56/src/gc.c:299:41
    #7 0x565e66 in mrb_obj_alloc /var/tmp/mruby-204b81c56/src/gc.c:498:5
    #8 0x4f283d in ary_new_capa /var/tmp/mruby-204b81c56/src/array.c:48:22
    #9 0x4f2e3b in ary_new_from_values /var/tmp/mruby-204b81c56/src/array.c:99:22
    #10 0x4f2dec in mrb_ary_new_from_values /var/tmp/mruby-204b81c56/src/array.c:110:22
    #11 0x6ff229 in mrb_vm_exec /var/tmp/mruby-204b81c56/src/vm.c:2654:17
    #12 0x6aae55 in mrb_vm_run /var/tmp/mruby-204b81c56/src/vm.c:1349:12
    #13 0x6a18f8 in mrb_top_run /var/tmp/mruby-204b81c56/src/vm.c:3081:10
    #14 0x597d5c in load_irep /var/tmp/mruby-204b81c56/src/load.c:682:10
    #15 0x5979dd in mrb_load_irep_cxt /var/tmp/mruby-204b81c56/src/load.c:690:10
    #16 0x597df8 in mrb_load_irep /var/tmp/mruby-204b81c56/src/load.c:702:10
    #17 0x4f176a in GENERATED_TMP_mrb_mruby_test_gem_test /var/tmp/mruby-204b81c56/build/sans/mrbgems/mruby-test/gem_test.c:16346:3
    #18 0x4db580 in mrbgemtest_init /var/tmp/mruby-204b81c56/build/sans/mrbgems/mruby-test/mrbtest.c:120:5
    #19 0x4d6a9f in main /var/tmp/mruby-204b81c56/mrbgems/mruby-test/driver.c:299:3
    #20 0x800b9da69 in __libc_start1 (/lib/libc.so.7+0x85a69)
    #21 0x42647f in _start /usr/src/lib/csu/amd64/crt1_s.S:83
    #22 0x800a5b007  (<unknown module>)

SUMMARY: AddressSanitizer: heap-use-after-free /var/tmp/mruby-204b81c56/src/gc.c:509:8 in mrb_obj_alloc
Shadow bytes around the buggy address:
  0x507000ca8700: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
  0x507000ca8780: fd fd fd fd fd fd fa fa fa fa 00 00 00 00 00 00
  0x507000ca8800: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
  0x507000ca8880: 00 00 fa fa fa fa 00 00 00 00 00 00 00 00 00 00
  0x507000ca8900: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 fa fa
=>0x507000ca8980: fa fa fd fd fd fd[fd]fd fd fd fd fd fa fa fa fa
  0x507000ca8a00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x507000ca8a80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x507000ca8b00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x507000ca8b80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x507000ca8c00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==69524==ABORTING

@dearblue dearblue requested a review from matz as a code owner August 24, 2024 08:19
@github-actions github-actions Bot added the core label Aug 24, 2024
@matz

matz commented Aug 24, 2024

Copy link
Copy Markdown
Member

I don't think the function name gc_protect_order does not describe the role of the function. It ｓeems that the goal is to reserve space in the arena in advance, unless MRB_GC_FIXED_ARENA is defined. Maybe we can define the function gc_arena_keep or gc_arena_ensure or something, then call it from mrb_obj_alloc where now we call gc_protect_order (unless MRB_GC_FIXED_ARENA is defined).

@dearblue
Fix use-after-free in mrb_obj_alloc()
1c5839f 
When GC occurs during the expansion of the GC arena by `gc_protect()` in `mrb_obj_alloc()`, the object page just allocated by `add_heap()` is released.
Therefore, as soon as control returns from `gc_protect()`, there is a possibility of illegal writing or reading to the address just released.

This issue was discovered during the investigation of mruby#6326.
@dearblue

dearblue commented Aug 25, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for the review.
I changed it to gc_arena_keep.
And in gc_protect(), I replaced gc_arena_keep() with mrb_assert().

「order」は「注文」の意味で使ってました。

dearblue added a commit to dearblue/mruby that referenced this pull request Aug 25, 2024
@dearblue
Restore the GC arena with tests.
79d7ac5 
This was the unintentional catalyst for finding the problem in mruby#6329, but it is preferable to add a test if necessary.
@dearblue dearblue mentioned this pull request Aug 25, 2024
Merged
@matz matz merged commit b9c9367 into mruby:master Aug 25, 2024
matz added a commit that referenced this pull request Aug 25, 2024
@matz
gc.c (gc_arena_keep): reorganized for MRB_GC_FIXED_ARENA; ref #6329
33dd623
matz added a commit that referenced this pull request Sep 3, 2024
@matz
gc.c (gc_arena_keep): revert 2 commits regarding arena allocation; #6329
4062069 


We assumed there's no need for gc_arena_keep() when MRB_GC_FIXED_ARENA
is set.  But it turned out that gc_protect() still can cause use-after-free
with fixed arena.

Revert "gc.c (gc_protect): should not call gc_arena_keep twice from allocation"

This reverts commit 28ece4e.

Revert "gc.c (gc_arena_keep): reorganized for MRB_GC_FIXED_ARENA; ref #6329"

This reverts commit 33dd623.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matz matz Awaiting requested review from matz matz is a code owner

Assignees

No one assigned

Labels

core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dearblue @matz