diff options
author | Alan Wu <XrXr@users.noreply.github.com> | 2023-12-06 21:00:37 -0500 |
---|---|---|
committer | Alan Wu <XrXr@users.noreply.github.com> | 2023-12-07 11:16:12 -0500 |
commit | 195dbf241f800556731c190b0e4368224afe2e11 (patch) | |
tree | 98e700441ea328a72751973884fbc4f6900f8994 | |
parent | 050806f4252dce2698260d905f7c86c621c1ec63 (diff) | |
download | ruby-195dbf241f800556731c190b0e4368224afe2e11.tar.gz |
Fix potential compaction issue in env_copy()
`src_ep[VM_ENV_DATA_INDEX_ME_CREF]` was read out and held without
marking across the allocation in vm_env_new(). In case vm_env_new() ran
compaction, an invalid reference could have been written into
`copied_env`.
It might've been hard to actually produce a crash with this issue due to
the pinning marking of the field in rb_execution_context_mark().
-rw-r--r-- | vm.c | 9 |
1 files changed, 5 insertions, 4 deletions
@@ -1218,14 +1218,15 @@ env_copy(const VALUE *src_ep, VALUE read_only_variables) VALUE *env_body = ZALLOC_N(VALUE, src_env->env_size); // fill with Qfalse VALUE *ep = &env_body[src_env->env_size - 2]; - ep[VM_ENV_DATA_INDEX_ME_CREF] = src_ep[VM_ENV_DATA_INDEX_ME_CREF]; - ep[VM_ENV_DATA_INDEX_FLAGS] = src_ep[VM_ENV_DATA_INDEX_FLAGS] | VM_ENV_FLAG_ISOLATED; + const rb_env_t *copied_env = vm_env_new(ep, env_body, src_env->env_size, src_env->iseq); + + // Copy after allocations above, since they can move objects in src_ep. + RB_OBJ_WRITE(copied_env, &ep[VM_ENV_DATA_INDEX_ME_CREF], src_ep[VM_ENV_DATA_INDEX_ME_CREF]); + ep[VM_ENV_DATA_INDEX_FLAGS] = src_ep[VM_ENV_DATA_INDEX_FLAGS] | VM_ENV_FLAG_ISOLATED; if (!VM_ENV_LOCAL_P(src_ep)) { VM_ENV_FLAGS_SET(ep, VM_ENV_FLAG_LOCAL); } - const rb_env_t *copied_env = vm_env_new(ep, env_body, src_env->env_size, src_env->iseq); - if (read_only_variables) { for (int i=RARRAY_LENINT(read_only_variables)-1; i>=0; i--) { ID id = NUM2ID(RARRAY_AREF(read_only_variables, i)); |