From f3abe5ba645839fb2a686aee18d3466b59256af0 Mon Sep 17 00:00:00 2001 From: "NARUSE, Yui" Date: Fri, 17 Mar 2023 13:40:04 +0900 Subject: merge revision(s) 0700d0fd1c77b4fddf803dea3c10be654df600ff,62c2082f1f726cb90d8c332fbedbecf41d5d82ec: [Backport #19469] Fix indentation in vm_setivar_default --- vm_insnhelper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) [Bug #19469] Fix crash when resizing generic iv list The following script can sometimes trigger a crash: ```ruby GC.stress = true class Array def foo(bool) if bool @a = 1 @b = 2 @c = 1 else @c = 1 end end end obj = [] obj.foo(true) obj2 = [] obj2.foo(false) obj3 = [] obj3.foo(true) ``` This is because vm_setivar_default calls rb_ensure_generic_iv_list_size to resize the iv list. However, the call to gen_ivtbl_resize reallocs the iv list, and then inserts into the generic iv table. If the st_insert triggers a GC then the old iv list will be read during marking, causing a use-after-free bug. Co-Authored-By: Jemma Issroff --- internal/variable.h | 2 +- variable.c | 23 ++++++++++++++++++----- vm_insnhelper.c | 4 ++-- 3 files changed, 21 insertions(+), 8 deletions(-) --- internal/variable.h | 2 +- variable.c | 23 ++++++++++++++++++----- version.h | 2 +- vm_insnhelper.c | 6 +++--- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/internal/variable.h b/internal/variable.h index 3933279633..6dec6a6759 100644 --- a/internal/variable.h +++ b/internal/variable.h @@ -57,7 +57,7 @@ VALUE rb_gvar_defined(ID); void rb_const_warn_if_deprecated(const rb_const_entry_t *, VALUE, ID); rb_shape_t * rb_grow_iv_list(VALUE obj); void rb_ensure_iv_list_size(VALUE obj, uint32_t len, uint32_t newsize); -struct gen_ivtbl * rb_ensure_generic_iv_list_size(VALUE obj, uint32_t newsize); +struct gen_ivtbl *rb_ensure_generic_iv_list_size(VALUE obj, rb_shape_t *shape, uint32_t newsize); attr_index_t rb_obj_ivar_set(VALUE obj, ID id, VALUE val); MJIT_SYMBOL_EXPORT_END diff --git a/variable.c b/variable.c index 0fd4a80887..c409209117 100644 --- a/variable.c +++ b/variable.c @@ -66,7 +66,10 @@ static st_table *generic_iv_tbl_; struct ivar_update { struct gen_ivtbl *ivtbl; uint32_t iv_index; - rb_shape_t* shape; + uint32_t max_index; +#if !SHAPE_IN_BASIC_FLAGS + rb_shape_t *shape; +#endif }; void @@ -1018,7 +1021,7 @@ generic_ivar_update(st_data_t *k, st_data_t *v, st_data_t u, int existing) } } FL_SET((VALUE)*k, FL_EXIVAR); - ivtbl = gen_ivtbl_resize(ivtbl, ivup->shape->next_iv_index); + ivtbl = gen_ivtbl_resize(ivtbl, ivup->max_index); // Reinsert in to the hash table because ivtbl might be a newly resized chunk of memory *v = (st_data_t)ivtbl; ivup->ivtbl = ivtbl; @@ -1281,7 +1284,10 @@ generic_ivar_set(VALUE obj, ID id, VALUE val) RUBY_ASSERT(index == (shape->next_iv_index - 1)); } + ivup.max_index = shape->next_iv_index; +#if !SHAPE_IN_BASIC_FLAGS ivup.shape = shape; +#endif RB_VM_LOCK_ENTER(); { @@ -1382,15 +1388,22 @@ rb_ensure_iv_list_size(VALUE obj, uint32_t current_capacity, uint32_t new_capaci } struct gen_ivtbl * -rb_ensure_generic_iv_list_size(VALUE obj, uint32_t newsize) +rb_ensure_generic_iv_list_size(VALUE obj, rb_shape_t *shape, uint32_t newsize) { struct gen_ivtbl * ivtbl = 0; RB_VM_LOCK_ENTER(); { if (UNLIKELY(!gen_ivtbl_get_unlocked(obj, 0, &ivtbl) || newsize > ivtbl->numiv)) { - ivtbl = gen_ivtbl_resize(ivtbl, newsize); - st_insert(generic_ivtbl_no_ractor_check(obj), (st_data_t)obj, (st_data_t)ivtbl); + struct ivar_update ivup = { + .iv_index = newsize - 1, + .max_index = newsize, +#if !SHAPE_IN_BASIC_FLAGS + .shape = shape +#endif + }; + st_update(generic_ivtbl_no_ractor_check(obj), (st_data_t)obj, generic_ivar_update, (st_data_t)&ivup); + ivtbl = ivup.ivtbl; FL_SET_RAW(obj, FL_EXIVAR); } } diff --git a/version.h b/version.h index 5dac221817..a148cc6c3a 100644 --- a/version.h +++ b/version.h @@ -11,7 +11,7 @@ # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 1 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 41 +#define RUBY_PATCHLEVEL 42 #include "ruby/version.h" #include "ruby/internal/abi.h" diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 8b1542bd1a..fb77b1f1cf 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1403,11 +1403,11 @@ vm_setivar_default(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_i shape_id_t source_shape_id = dest_shape->parent_id; if (shape_id == source_shape_id && dest_shape->edge_name == id && dest_shape->type == SHAPE_IVAR) { - ivtbl = rb_ensure_generic_iv_list_size(obj, index + 1); + ivtbl = rb_ensure_generic_iv_list_size(obj, dest_shape, index + 1); #if SHAPE_IN_BASIC_FLAGS - RBASIC_SET_SHAPE_ID(obj, dest_shape_id); + RBASIC_SET_SHAPE_ID(obj, dest_shape_id); #else - ivtbl->shape_id = dest_shape_id; + RUBY_ASSERT(ivtbl->shape_id == dest_shape_id); #endif } else { -- cgit v1.2.1