Skip to content

Commit

Permalink
Fix potential compaction issue in env_copy()
Browse files Browse the repository at this point in the history
`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().
  • Loading branch information
XrXr committed Dec 7, 2023
1 parent 050806f commit 195dbf2
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions vm.c
Expand Up @@ -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));
Expand Down

0 comments on commit 195dbf2

Please sign in to comment.