From 299455be9966c0a31dabe00014a4b8fae5093a7d Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 14 Jan 2025 17:54:19 -0800 Subject: [PATCH] merge revision(s) 92dd9734a967c20e628c8f77c5ce700058dcd58c: [Backport #20950] Fix use-after-free in ep in Proc#dup for ifunc procs [Bug #20950] ifunc proc has the ep allocated in the cfunc_proc_t which is the data of the TypedData object. If an ifunc proc is duplicated, the ep points to the ep of the source object. If the source object is freed, then the ep of the duplicated object now points to a freed memory region. If we try to use the ep we could crash. For example, the following script crashes: p = { a: 1 }.to_proc 100.times do p = p.dup GC.start p.call rescue ArgumentError end This commit changes ifunc proc to also duplicate the ep when it is duplicated. --- internal/proc.h | 1 + proc.c | 44 ++++++++++++++++++++++++++++++++++++++---- test/ruby/test_proc.rb | 12 ++++++++++++ version.h | 2 +- vm.c | 11 ++++++++++- 5 files changed, 64 insertions(+), 6 deletions(-) diff --git a/internal/proc.h b/internal/proc.h index c75f15b283..c6a9e38bfa 100644 --- a/internal/proc.h +++ b/internal/proc.h @@ -23,6 +23,7 @@ VALUE rb_block_to_s(VALUE self, const struct rb_block *block, const char *additi VALUE rb_callable_receiver(VALUE); VALUE rb_func_proc_new(rb_block_call_func_t func, VALUE val); +VALUE rb_func_proc_dup(VALUE src_obj); VALUE rb_func_lambda_new(rb_block_call_func_t func, VALUE val, int min_argc, int max_argc); VALUE rb_iseq_location(const struct rb_iseq_struct *iseq); VALUE rb_sym_to_proc(VALUE sym); diff --git a/proc.c b/proc.c index 9405a1a09f..a3fdb1783c 100644 --- a/proc.c +++ b/proc.c @@ -680,6 +680,29 @@ cfunc_proc_new(VALUE klass, VALUE ifunc) return procval; } +VALUE +rb_func_proc_dup(VALUE src_obj) +{ + RUBY_ASSERT(rb_typeddata_is_instance_of(src_obj, &proc_data_type)); + + rb_proc_t *src_proc; + GetProcPtr(src_obj, src_proc); + RUBY_ASSERT(vm_block_type(&src_proc->block) == block_type_ifunc); + + cfunc_proc_t *proc; + VALUE proc_obj = TypedData_Make_Struct(rb_obj_class(src_obj), cfunc_proc_t, &proc_data_type, proc); + + memcpy(&proc->basic, src_proc, sizeof(rb_proc_t)); + + VALUE *ep = *(VALUE **)&proc->basic.block.as.captured.ep = proc->env + VM_ENV_DATA_SIZE - 1; + ep[VM_ENV_DATA_INDEX_FLAGS] = src_proc->block.as.captured.ep[VM_ENV_DATA_INDEX_FLAGS]; + ep[VM_ENV_DATA_INDEX_ME_CREF] = src_proc->block.as.captured.ep[VM_ENV_DATA_INDEX_ME_CREF]; + ep[VM_ENV_DATA_INDEX_SPECVAL] = src_proc->block.as.captured.ep[VM_ENV_DATA_INDEX_SPECVAL]; + ep[VM_ENV_DATA_INDEX_ENV] = src_proc->block.as.captured.ep[VM_ENV_DATA_INDEX_ENV]; + + return proc_obj; +} + static VALUE sym_proc_new(VALUE klass, VALUE sym) { @@ -1300,12 +1323,17 @@ proc_eq(VALUE self, VALUE other) } break; case block_type_ifunc: - if (self_block->as.captured.ep != \ - other_block->as.captured.ep || - self_block->as.captured.code.ifunc != \ + if (self_block->as.captured.code.ifunc != \ other_block->as.captured.code.ifunc) { return Qfalse; } + + if (memcmp( + ((cfunc_proc_t *)self_proc)->env, + ((cfunc_proc_t *)other_proc)->env, + sizeof(((cfunc_proc_t *)self_proc)->env))) { + return Qfalse; + } break; case block_type_proc: if (self_block->as.proc != other_block->as.proc) { @@ -1434,6 +1462,7 @@ rb_hash_proc(st_index_t hash, VALUE prc) break; case block_type_ifunc: hash = rb_st_hash_uint(hash, (st_index_t)proc->block.as.captured.code.ifunc->func); + hash = rb_st_hash_uint(hash, (st_index_t)proc->block.as.captured.code.ifunc->data); break; case block_type_symbol: hash = rb_st_hash_uint(hash, rb_any_hash(proc->block.as.symbol)); @@ -1445,7 +1474,14 @@ rb_hash_proc(st_index_t hash, VALUE prc) rb_bug("rb_hash_proc: unknown block type %d", vm_block_type(&proc->block)); } - return rb_hash_uint(hash, (st_index_t)proc->block.as.captured.ep); + /* ifunc procs have their own allocated ep. If an ifunc is duplicated, they + * will point to different ep but they should return the same hash code, so + * we cannot include the ep in the hash. */ + if (vm_block_type(&proc->block) != block_type_ifunc) { + hash = rb_hash_uint(hash, (st_index_t)proc->block.as.captured.ep); + } + + return hash; } diff --git a/test/ruby/test_proc.rb b/test/ruby/test_proc.rb index 19fc89357a..511826c636 100644 --- a/test/ruby/test_proc.rb +++ b/test/ruby/test_proc.rb @@ -410,6 +410,18 @@ class TestProc < Test::Unit::TestCase assert_throw(:initialize_dup) {c1.new{}.dup} end + def test_dup_ifunc_proc_bug_20950 + assert_normal_exit(<<~RUBY, "[Bug #20950]") + p = { a: 1 }.to_proc + 100.times do + p = p.dup + GC.start + p.call + rescue ArgumentError + end + RUBY + end + def test_clone_subclass c1 = Class.new(Proc) assert_equal c1, c1.new{}.clone.class, '[Bug #17545]' diff --git a/version.h b/version.h index eeffafbab6..3de17eb647 100644 --- a/version.h +++ b/version.h @@ -11,7 +11,7 @@ # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 6 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 117 +#define RUBY_PATCHLEVEL 118 #include "ruby/version.h" #include "ruby/internal/abi.h" diff --git a/vm.c b/vm.c index b9e9b711e3..a0fe0104b0 100644 --- a/vm.c +++ b/vm.c @@ -1158,7 +1158,16 @@ rb_proc_dup(VALUE self) rb_proc_t *src; GetProcPtr(self, src); - procval = proc_create(rb_obj_class(self), &src->block, src->is_from_method, src->is_lambda); + + switch (vm_block_type(&src->block)) { + case block_type_ifunc: + procval = rb_func_proc_dup(self); + break; + default: + procval = proc_create(rb_obj_class(self), &src->block, src->is_from_method, src->is_lambda); + break; + } + if (RB_OBJ_SHAREABLE_P(self)) FL_SET_RAW(procval, RUBY_FL_SHAREABLE); RB_GC_GUARD(self); /* for: body = rb_proc_dup(body) */ return procval;