From 8f29357959385e4191b23b391471bbd95e21c9e5 Mon Sep 17 00:00:00 2001 From: Richard Patel Date: Fri, 2 Aug 2024 17:30:22 +0000 Subject: [PATCH] Hardcode reject_callx_r10 feature The feature reject_callx_r10 is enabled on all clusters and thus should be hardcoded --- src/app/fddev/configure/genesis.c | 2 +- .../runtime/tests/fd_exec_sol_compat.c | 6 +++--- .../runtime/tests/fd_vm_validate_test.c | 19 +++++-------------- src/flamenco/vm/fd_vm.c | 10 +++++----- src/flamenco/vm/instr_test/jump.instr | 14 ++------------ src/flamenco/vm/test_vm_instr.c | 10 ++-------- src/flamenco/vm/test_vm_util.c | 11 ++++------- src/flamenco/vm/test_vm_util.h | 3 +-- 8 files changed, 23 insertions(+), 52 deletions(-) diff --git a/src/app/fddev/configure/genesis.c b/src/app/fddev/configure/genesis.c index 06bc7a4270..e0f10e88cb 100644 --- a/src/app/fddev/configure/genesis.c +++ b/src/app/fddev/configure/genesis.c @@ -30,7 +30,7 @@ default_enable_features( fd_features_t * features ) { features->incremental_snapshot_only_incremental_hash_calculation = 0UL; features->timely_vote_credits = 0UL; features->apply_cost_tracker_during_replay = 0UL; - features->reject_callx_r10 = 0UL; + features->reject_callx_r10 = 1UL; features->update_hashes_per_tick = 0UL; features->enable_partitioned_epoch_reward = 0UL; features->pico_inflation = 0UL; diff --git a/src/flamenco/runtime/tests/fd_exec_sol_compat.c b/src/flamenco/runtime/tests/fd_exec_sol_compat.c index a0abf9bf47..32b29ce990 100644 --- a/src/flamenco/runtime/tests/fd_exec_sol_compat.c +++ b/src/flamenco/runtime/tests/fd_exec_sol_compat.c @@ -30,6 +30,7 @@ static ulong cleaned_up_features[] = 0x8f688d4e3ab17a60, // enable_early_verification_of_account_modifications 0x50a615bae8ca3874, // native_programs_consume_cu 0x65b79c7f3e7441b3, // require_custodian_for_locked_stake_authorize + 0x7e787d5c6d662d23, // reject_callx_r10 }; static ulong supported_features[] = @@ -47,7 +48,6 @@ static ulong supported_features[] = 0x8a8eb9085ca2bb0b, // commission_updates_only_allowed_in_first_half_of_epoch 0x7bc99a080444c8d9, // allow_votes_to_directly_update_vote_state 0x2ca5833736ba5c69, // compact_vote_state_updates - 0x7e787d5c6d662d23, // reject_callx_r10 0x0b9047b5bb9ef961, // move_stake_and_move_lamports_ixs }; @@ -347,7 +347,7 @@ sol_compat_syscall_fixture( fd_exec_instr_test_runner_t * runner, return ok; } -int +int sol_compat_validate_vm_fixture( fd_exec_instr_test_runner_t * runner, uchar const * in, ulong in_sz ) { @@ -362,7 +362,7 @@ sol_compat_validate_vm_fixture( fd_exec_instr_test_runner_t * runner, void * output = NULL; sol_compat_execute_wrapper( runner, &fixture->input, - &output, + &output, (exec_test_run_fn_t *)fd_exec_vm_validate_test_run ); // Compare effects int ok = sol_compat_cmp_binary_strict( output, &fixture->output, &fd_exec_test_validate_vm_effects_t_msg ); diff --git a/src/flamenco/runtime/tests/fd_vm_validate_test.c b/src/flamenco/runtime/tests/fd_vm_validate_test.c index ed8bef58ec..1ece9cc4ac 100644 --- a/src/flamenco/runtime/tests/fd_vm_validate_test.c +++ b/src/flamenco/runtime/tests/fd_vm_validate_test.c @@ -15,16 +15,7 @@ fd_exec_vm_validate_test_run( fd_exec_instr_test_runner_t * runner, return 0UL; } - int rej_callx_r10 = 0; - if( input->has_features ) { - for( ulong i=0UL; i < input->features.features_count; i++ ) { - if( input->features.features[i] == TEST_VM_REJECT_CALLX_R10_FEATURE_PREFIX ) { - rej_callx_r10 = 1; - break; - } - } - } - fd_exec_instr_ctx_t * ctx = test_vm_minimal_exec_instr_ctx( fd_libc_alloc_virtual(), rej_callx_r10 ); + fd_exec_instr_ctx_t * ctx = test_vm_minimal_exec_instr_ctx( fd_libc_alloc_virtual() ); FD_TEST( output_bufsz >= sizeof(fd_exec_test_validate_vm_effects_t) ); @@ -48,7 +39,7 @@ fd_exec_vm_validate_test_run( fd_exec_instr_test_runner_t * runner, rodata_sz = vm_ctx->rodata->size; } - ulong * text = (ulong *) (rodata + vm_ctx->rodata_text_section_offset); + ulong * text = (ulong *) (rodata + vm_ctx->rodata_text_section_offset); ulong text_cnt = vm_ctx->rodata_text_section_length / 8UL; fd_vm_t * vm = fd_vm_join( fd_vm_new( fd_valloc_malloc( valloc, fd_vm_align(), fd_vm_footprint() ) ) ); @@ -80,13 +71,13 @@ fd_exec_vm_validate_test_run( fd_exec_instr_test_runner_t * runner, fd_valloc_free( valloc, fd_vm_delete( fd_vm_leave( vm ) ) ); } while(0); - + /* Run vm validate and capture result */ - + effects->success = (effects->result == FD_VM_SUCCESS); *output = effects; - + test_vm_exec_instr_ctx_delete( ctx ); return sizeof (fd_exec_test_validate_vm_effects_t); } diff --git a/src/flamenco/vm/fd_vm.c b/src/flamenco/vm/fd_vm.c index 9d04e44df3..9c39cac9d7 100644 --- a/src/flamenco/vm/fd_vm.c +++ b/src/flamenco/vm/fd_vm.c @@ -159,12 +159,12 @@ fd_vm_validate( fd_vm_t const * vm ) { /* FIXME: These checks are not necessary assuming fd_vm_t is populated by metadata generated in fd_sbpf_elf_peek (which performs these checks). But there is no guarantee, and - this non-guarantee is (rightfully) exploited by the fuzz harnesses. + this non-guarantee is (rightfully) exploited by the fuzz harnesses. Agave doesn't perform these checks explicitly due to Rust's guarantees */ - if( FD_UNLIKELY( vm->text_sz / 8UL != vm->text_cnt || + if( FD_UNLIKELY( vm->text_sz / 8UL != vm->text_cnt || (const uchar *) vm->text < vm->rodata || (const uchar *) vm->text > (const uchar *) vm->text + vm->text_sz || /* Overflow chk */ - (const uchar *) vm->text + vm->text_sz > vm->rodata + vm->rodata_sz ) ) + (const uchar *) vm->text + vm->text_sz > vm->rodata + vm->rodata_sz ) ) return FD_VM_ERR_BAD_TEXT; if( FD_UNLIKELY( !fd_ulong_is_aligned( vm->text_sz, 8UL ) ) ) /* https://github.com/solana-labs/rbpf/blob/v0.8.0/src/verifier.rs#L109 */ @@ -209,7 +209,7 @@ fd_vm_validate( fd_vm_t const * vm ) { /* FIXME: SET A BIT MAP HERE OF ADDL_IMM TO DENOTE * AS FORBIDDEN BRANCH TARGETS OF CALL_REG?? */ - + i++; /* Skip the addl imm */ break; } @@ -232,7 +232,7 @@ fd_vm_validate( fd_vm_t const * vm ) { case FD_CHECK_CALLX: { /* The register number to read is stored in the immediate. https://github.com/solana-labs/rbpf/blob/v0.8.1/src/verifier.rs#L218 */ - if( FD_UNLIKELY( instr.imm > ( FD_FEATURE_ACTIVE( vm->instr_ctx->slot_ctx, reject_callx_r10 ) ? 9 : 10 ) ) ) { + if( FD_UNLIKELY( instr.imm > 9 ) ) { return FD_VM_ERR_INVALID_REG; } break; diff --git a/src/flamenco/vm/instr_test/jump.instr b/src/flamenco/vm/instr_test/jump.instr index e9fc023095..aa0021dafb 100644 --- a/src/flamenco/vm/instr_test/jump.instr +++ b/src/flamenco/vm/instr_test/jump.instr @@ -364,19 +364,9 @@ $ op=dd dst=a src=1 off=0000 : vfy $ op=dd dst=9 src=b off=0000 : vfy # call_reg reg[imm] (these should fail during exec by default) -$ op=8d dst=9 src=9 imm=0 : err -$ op=8d dst=9 src=9 imm=1 : err -$ op=8d dst=9 src=9 imm=9 : err -$ op=8d dst=9 src=9 imm=b : vfy -$ op=8d dst=9 src=9 imm=ffffffff : vfy -$ op=8d dst=9 src=9 imm=a : err - -$ reject_callx_r10=1 - op=8d dst=9 src=9 imm=a : vfy $ op=8d dst=9 src=9 imm=0 : err $ op=8d dst=9 src=9 imm=1 : err $ op=8d dst=9 src=9 imm=9 : err $ op=8d dst=9 src=9 imm=b : vfy -$ op=8d dst=9 src=9 imm=fffffffff : vfy - -$ reject_callx_r10=0 +$ op=8d dst=9 src=9 imm=ffffffff : vfy +$ op=8d dst=9 src=9 imm=a : vfy diff --git a/src/flamenco/vm/test_vm_instr.c b/src/flamenco/vm/test_vm_instr.c index 8f10e9b871..4a411a135c 100644 --- a/src/flamenco/vm/test_vm_instr.c +++ b/src/flamenco/vm/test_vm_instr.c @@ -45,7 +45,6 @@ struct test_input { ushort off; ulong imm; ulong reg[REG_CNT]; - bool reject_callx_r10; uint region_boundary[16]; /* This can be changed */ uint region_boundary_cnt; }; @@ -326,11 +325,6 @@ parse_token( test_parser_t * p, ulong * out = p->state == PARSE_STATE_ASSERT ? p->effects.reg : p->input.reg; out[ reg ] = parse_hex_int( p ); - } else if( 0==strncmp( word, "reject_callx_r10", word_len ) ) { - - parse_assign_sep( p ); - p->input.reject_callx_r10 = parse_hex_int( p ); - } else { FD_LOG_ERR(( "Unexpected token '%.*s' at %s(%lu)", (int)word_len, word, p->path, p->line )); @@ -403,7 +397,7 @@ run_input( test_input_t const * input, /* Turn input into a single memory region */ fd_vm_input_region_t input_region[32]; - + if( !input->region_boundary_cnt ) { input_region[0] = (fd_vm_input_region_t){ .vaddr_offset = 0UL, @@ -439,7 +433,7 @@ run_input( test_input_t const * input, fd_sbpf_syscalls_new( aligned_alloc( fd_sbpf_syscalls_align(), fd_sbpf_syscalls_footprint() ) ) ); - fd_exec_instr_ctx_t * instr_ctx = test_vm_minimal_exec_instr_ctx( fd_libc_alloc_virtual(), input->reject_callx_r10 ); + fd_exec_instr_ctx_t * instr_ctx = test_vm_minimal_exec_instr_ctx( fd_libc_alloc_virtual() ); int vm_ok = !!fd_vm_init( /* vm */ vm, diff --git a/src/flamenco/vm/test_vm_util.c b/src/flamenco/vm/test_vm_util.c index 72e2a326d6..279ffa0814 100644 --- a/src/flamenco/vm/test_vm_util.c +++ b/src/flamenco/vm/test_vm_util.c @@ -5,9 +5,8 @@ /* Generates a minimal instruction context to supply to fd_vm_t. For now, we just need to setup feature flags. */ fd_exec_instr_ctx_t * -test_vm_minimal_exec_instr_ctx( - fd_valloc_t valloc, - bool reject_callx_r10 ) { +test_vm_minimal_exec_instr_ctx( + fd_valloc_t valloc ) { void * _ctx = fd_exec_instr_ctx_new( fd_valloc_malloc( valloc, FD_EXEC_INSTR_CTX_ALIGN, FD_EXEC_INSTR_CTX_FOOTPRINT ) ); @@ -39,9 +38,7 @@ test_vm_minimal_exec_instr_ctx( /* Setup feature flags */ fd_features_disable_all( &epoch_ctx->features ); - if ( reject_callx_r10 ) { - fd_features_set( &epoch_ctx->features, fd_feature_id_query(TEST_VM_REJECT_CALLX_R10_FEATURE_PREFIX), 0UL ); - } + fd_features_set( &epoch_ctx->features, fd_feature_id_query(TEST_VM_REJECT_CALLX_R10_FEATURE_PREFIX), 0UL ); return ctx; } @@ -49,7 +46,7 @@ test_vm_minimal_exec_instr_ctx( void test_vm_exec_instr_ctx_delete( fd_exec_instr_ctx_t * ctx ) { - + fd_valloc_t valloc = ctx->valloc; fd_exec_slot_ctx_t * slot_ctx = ctx->slot_ctx; fd_exec_epoch_ctx_t * epoch_ctx = slot_ctx->epoch_ctx; diff --git a/src/flamenco/vm/test_vm_util.h b/src/flamenco/vm/test_vm_util.h index 226f4ae830..ceae24c384 100644 --- a/src/flamenco/vm/test_vm_util.h +++ b/src/flamenco/vm/test_vm_util.h @@ -9,8 +9,7 @@ fd_exec_instr_ctx_t * test_vm_minimal_exec_instr_ctx( - fd_valloc_t valloc, - bool reject_callx_r10 + fd_valloc_t valloc ); void