Skip to content

Commit

Permalink
syscall, cpi, vm: fix syscall cpi mismatches
Browse files Browse the repository at this point in the history
  • Loading branch information
@mjain-jump
mjain-jump committed Nov 14, 2024
1 parent e73ad30 commit 12a83f1
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 18 deletions.
54 changes: 51 additions & 3 deletions contrib/test/test-vectors-fixtures/cpi-fixtures/cpi.list
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,54 @@
dump/test-vectors/cpi/fixtures/callee_not_executable_log.fix
dump/test-vectors/cpi/fixtures/cpi_prepare_missing_acct.fix
dump/test-vectors/cpi/fixtures/02a029a5928a7417d7c87b19244dafce98b3d432_3159772.fix
dump/test-vectors/cpi/fixtures/0eee9e82a7855e4ac872817c1e366afd36d63508_3166008.fix
dump/test-vectors/cpi/fixtures/109d2e9e3d3a595e10f1a05491eec269a8f0ff5f_3159240.fix
dump/test-vectors/cpi/fixtures/13b76b5d5e7ea1cc1c53445a796ad6eb07af1f2d_3162592.fix
dump/test-vectors/cpi/fixtures/14becdf2902070b25122a29ea3f42becb768c1fb_3167277.fix
dump/test-vectors/cpi/fixtures/154cc1fe7a191d7cd9761cdf93a3f1b92148de49_3164049.fix
dump/test-vectors/cpi/fixtures/18387a7e499df53e20db216f4520bca9ce0727e7_1582213.fix
dump/test-vectors/cpi/fixtures/dead_non_executable_callee.fix
dump/test-vectors/cpi/fixtures/18bd882dd6084b4dbd990f3ff3897e526d9d22ff_3168122.fix
dump/test-vectors/cpi/fixtures/225318f0bc1ca24069b8aec607d22aefe2a04b60_3163825.fix
dump/test-vectors/cpi/fixtures/2570ffd252b282ea2cab9ad85e5f03374627a8ff_3166564.fix
dump/test-vectors/cpi/fixtures/2edd851097c3dcddda6e62c2ebfee38dfd40ce40_3168367.fix
dump/test-vectors/cpi/fixtures/402c9acfce58703464db006d2de77d2194789f3c_3169698.fix
dump/test-vectors/cpi/fixtures/42fde53aca766f91549e73d30e5e7fe657823567_3160396.fix
dump/test-vectors/cpi/fixtures/527b50b91586014737647ed7e76eddc9b4f05fec_3169950.fix
dump/test-vectors/cpi/fixtures/5a9acb52804b5334432fc7545781ed8808f444e8_3171140.fix
dump/test-vectors/cpi/fixtures/5ab692110fc354ef1b7980daf6a0198276ae77f9_3160690.fix
dump/test-vectors/cpi/fixtures/5b3a7a9ed4b766c410d027372cd268ed9e07cc7c_3162136.fix
dump/test-vectors/cpi/fixtures/611804da4a1f39f34976a6993d72e15996f47041_3167482.fix
dump/test-vectors/cpi/fixtures/62b47b37e0e462bd2b22c8862c6290971834016d_3162339.fix
dump/test-vectors/cpi/fixtures/65229a93efeb6ab8264c2b7fbff4112b5d08b8f8_3164849.fix
dump/test-vectors/cpi/fixtures/6d086f4ed3d6ca21f2f3a286fcb85964ddcbaeba_3159506.fix
dump/test-vectors/cpi/fixtures/6ea248057266261ee27aafa9bfe93a9e7344b5ec_3169079.fix
dump/test-vectors/cpi/fixtures/70eee9b08f2c12b5debb200c147aeafad39690f4_3171382.fix
dump/test-vectors/cpi/fixtures/79339ec1f28b1450d756743ef4f7545605025c7c_3167684.fix
dump/test-vectors/cpi/fixtures/850b8f0286d8f4793a4f447c917164247e74a803_302825.fix
dump/test-vectors/cpi/fixtures/86aac5b9f4512a558c50c4138475ecc4e14200ce_3161668.fix
dump/test-vectors/cpi/fixtures/8fc1d3d65cfc083213f3d2f04657a62d2a521111_3165271.fix
dump/test-vectors/cpi/fixtures/909a6d936a68b2d0325ae38f5d1ec447bd854d32_3166348.fix
dump/test-vectors/cpi/fixtures/94dbe9c0238593cdaafe078787e4ac2943075192_3165485.fix
dump/test-vectors/cpi/fixtures/94f3a4a6e0b0d924ea73faa81986774e587fba9f_3169270.fix
dump/test-vectors/cpi/fixtures/9bf0d17e5fb024aa35abdab614431786ed3ff41c_3167917.fix
dump/test-vectors/cpi/fixtures/9d70a0beb4baff67ecf28b37326b3a526e753a07_3166792.fix
dump/test-vectors/cpi/fixtures/9eb285ddad725436f9bfdfce14ea8ef7fbe02708_3161907.fix
dump/test-vectors/cpi/fixtures/ac9c7abc030b59579e9922bd0e6a05f9281cddce_3161422.fix
dump/test-vectors/cpi/fixtures/b06b5c4824c38f9bd935756a2d383c01d4dad885_228827.fix
dump/test-vectors/cpi/fixtures/b1f4544dffbd31e3d8f1ba49dcef6eaae3ae0ff8_3163079.fix
dump/test-vectors/cpi/fixtures/b8ec07e6641610dabbce7a6aaf7bcf8786bd94d6_3161187.fix
dump/test-vectors/cpi/fixtures/b90a28ed3d250e217ddb0fbe843c33f45d342206_3169513.fix
dump/test-vectors/cpi/fixtures/c13608fe24f860b05d681e387dcf84cb4e91fffd_3160153.fix
dump/test-vectors/cpi/fixtures/c54749ed71af918df4f13db4e25ffd5969a95f55_3164594.fix
dump/test-vectors/cpi/fixtures/c586287a5fffaabb267f7b347f2e181837b0034a_3168837.fix
dump/test-vectors/cpi/fixtures/c7fbc153c56338651a1ac0a7077be9ee12031b48_3162826.fix
dump/test-vectors/cpi/fixtures/callee_not_executable_log.fix
dump/test-vectors/cpi/fixtures/cc376ae05f2d05d628531614e9c29bcc5e76ddd9_3167036.fix
dump/test-vectors/cpi/fixtures/ce27c22339ab86527a59723be3fcaa7481680214_3171610.fix
dump/test-vectors/cpi/fixtures/cpi_callee_prog_in_txn_check.fix
dump/test-vectors/cpi/fixtures/cpi_prepare_missing_acct.fix
dump/test-vectors/cpi/fixtures/d3ae40413bfbdf94985163ed51ef1659823578c2_3158962.fix
dump/test-vectors/cpi/fixtures/d7db876ed147026331555538c98e9d7be44d756b_3160979.fix
dump/test-vectors/cpi/fixtures/d7fd0177af6c324fbb3282e23bb8e5b3dfa9362f_3164349.fix
dump/test-vectors/cpi/fixtures/dead_non_executable_callee.fix
dump/test-vectors/cpi/fixtures/f6c8cf465b4edc04fbad54c3d55c78f39cd67059_3168608.fix
dump/test-vectors/cpi/fixtures/fc3571eee4259ca8587c388c4f2a71df4ec5ca02_3165792.fix
dump/test-vectors/cpi/fixtures/ff6c6c34d1e3065cb0573fa26a478d6497b70664_3165041.fix
41 changes: 26 additions & 15 deletions src/flamenco/vm/syscall/fd_vm_syscall_cpi_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ VM_SYCALL_CPI_UPDATE_CALLEE_ACC_FUNC( fd_vm_t * vm,
}
}

if( !FD_FEATURE_ACTIVE( vm->instr_ctx->slot_ctx, bpf_account_data_direct_mapping ) ) {
if( !vm->direct_mapping ) {
/* Get the account data */
/* Update the account data, if the account data can be changed */
/* FIXME: double-check these permissions, especially the callee_acc_idx */
Expand Down Expand Up @@ -380,7 +380,7 @@ VM_SYSCALL_CPI_UPDATE_CALLER_ACC_FUNC( fd_vm_t * vm,
uchar instr_acc_idx,
fd_pubkey_t const * pubkey ) {

if( !FD_FEATURE_ACTIVE( vm->instr_ctx->slot_ctx, bpf_account_data_direct_mapping ) ) {
if( !vm->direct_mapping ) {
/* Look up the borrowed account from the instruction context, which will contain
the callee's changes. */
fd_borrowed_account_t * callee_acc_rec = NULL;
Expand Down Expand Up @@ -585,8 +585,21 @@ VM_SYSCALL_CPI_ENTRYPOINT( void * _vm,
/* Translate instruction ********************************************/
/* translate_instruction is the first thing that agave does
https://github.com/anza-xyz/agave/blob/838c1952595809a31520ff1603a13f2c9123aa51/programs/bpf_loader/src/syscalls/cpi.rs#L1089 */

/* Translating the CPI instruction
https://github.com/anza-xyz/agave/blob/838c1952595809a31520ff1603a13f2c9123aa51/programs/bpf_loader/src/syscalls/cpi.rs#L420-L424 */
VM_SYSCALL_CPI_INSTR_T const * cpi_instruction =
FD_VM_MEM_HADDR_LD( vm, instruction_va, VM_SYSCALL_CPI_INSTR_ALIGN, VM_SYSCALL_CPI_INSTR_SIZE );

/* Translate the program ID */
fd_pubkey_t const * program_id = (fd_pubkey_t *)VM_SYSCALL_CPI_INSTR_PROGRAM_ID( vm, cpi_instruction );

/* Translate CPI account metas *************************************************/
VM_SYSCALL_CPI_ACC_META_T const * cpi_account_metas =
FD_VM_MEM_SLICE_HADDR_LD( vm, VM_SYSCALL_CPI_INSTR_ACCS_ADDR( cpi_instruction ),
VM_SYSCALL_CPI_ACC_META_ALIGN,
fd_ulong_sat_mul( VM_SYSCALL_CPI_INSTR_ACCS_LEN( cpi_instruction ), VM_SYSCALL_CPI_ACC_META_SIZE ) );

/* Agave consumes CU in translate_instruction
https://github.com/anza-xyz/agave/blob/838c1952595809a31520ff1603a13f2c9123aa51/programs/bpf_loader/src/syscalls/cpi.rs#L445 */
if( FD_FEATURE_ACTIVE( vm->instr_ctx->slot_ctx, loosen_cpi_size_restriction ) ) {
Expand All @@ -612,12 +625,6 @@ VM_SYSCALL_CPI_ENTRYPOINT( void * _vm,
}
}

/* Translate CPI account metas *************************************************/
VM_SYSCALL_CPI_ACC_META_T const * cpi_account_metas =
FD_VM_MEM_SLICE_HADDR_LD( vm, VM_SYSCALL_CPI_INSTR_ACCS_ADDR( cpi_instruction ),
VM_SYSCALL_CPI_ACC_META_ALIGN,
fd_ulong_sat_mul( VM_SYSCALL_CPI_INSTR_ACCS_LEN( cpi_instruction ), VM_SYSCALL_CPI_ACC_META_SIZE ) );

/* Translate instruction data *************************************************/

uchar const * data = FD_VM_MEM_SLICE_HADDR_LD(
Expand All @@ -627,7 +634,6 @@ VM_SYSCALL_CPI_ENTRYPOINT( void * _vm,

/* Authorized program check *************************************************/

fd_pubkey_t const * program_id = (fd_pubkey_t *)VM_SYSCALL_CPI_INSTR_PROGRAM_ID( vm, cpi_instruction );
if( FD_UNLIKELY( fd_vm_syscall_cpi_check_authorized_program( program_id, vm->instr_ctx->slot_ctx, data, VM_SYSCALL_CPI_INSTR_DATA_LEN( cpi_instruction ) ) ) ) {
return FD_VM_ERR_PERM;
}
Expand Down Expand Up @@ -660,13 +666,18 @@ VM_SYSCALL_CPI_ENTRYPOINT( void * _vm,
if( FD_UNLIKELY( err ) ) return err;

/* Translate account infos ******************************************/
/* Direct mapping check
https://github.com/anza-xyz/agave/blob/v2.1.0/programs/bpf_loader/src/syscalls/cpi.rs#L805-L814 */
ulong acc_info_total_sz = fd_ulong_sat_mul( acct_info_cnt, VM_SYSCALL_CPI_ACC_INFO_SIZE );
if( FD_UNLIKELY( vm->direct_mapping && fd_ulong_sat_add( acct_infos_va, acc_info_total_sz ) >= FD_VM_MEM_MAP_INPUT_REGION_START ) ) {
FD_VM_ERR_FOR_LOG_SYSCALL( vm, FD_VM_ERR_SYSCALL_INVALID_POINTER );
return FD_VM_ERR_SYSCALL_INVALID_POINTER;
}

/* This is the equivalent of translate_slice in translate_account_infos:
https://github.com/anza-xyz/agave/blob/838c1952595809a31520ff1603a13f2c9123aa51/programs/bpf_loader/src/syscalls/cpi.rs#L816 */
VM_SYSCALL_CPI_ACC_INFO_T * acc_infos =
FD_VM_MEM_SLICE_HADDR_ST( vm,
acct_infos_va,
VM_SYSCALL_CPI_ACC_INFO_ALIGN,
fd_ulong_sat_mul( acct_info_cnt, VM_SYSCALL_CPI_ACC_INFO_SIZE ) );
VM_SYSCALL_CPI_ACC_INFO_T * acc_infos = FD_VM_MEM_SLICE_HADDR_ST( vm, acct_infos_va, VM_SYSCALL_CPI_ACC_INFO_ALIGN, acc_info_total_sz );

/* Right after translating, Agave checks the number of account infos:
https://github.com/anza-xyz/agave/blob/838c1952595809a31520ff1603a13f2c9123aa51/programs/bpf_loader/src/syscalls/cpi.rs#L822 */
if( FD_FEATURE_ACTIVE( vm->instr_ctx->slot_ctx, loosen_cpi_size_restriction ) ) {
Expand Down Expand Up @@ -723,7 +734,7 @@ VM_SYSCALL_CPI_ENTRYPOINT( void * _vm,
/* Update all account permissions before updating the account data updates.
We have inlined the anza function update_caller_account_perms here.
TODO: consider factoring this out */
if( FD_FEATURE_ACTIVE( vm->instr_ctx->slot_ctx, bpf_account_data_direct_mapping ) ) {
if( vm->direct_mapping ) {
for( ulong i=0UL; i<vm->instr_ctx->instr->acct_cnt; i++ ) {
/* https://github.com/firedancer-io/solana/blob/508f325e19c0fd8e16683ea047d7c1a85f127e74/programs/bpf_loader/src/syscalls/cpi.rs#L939-L943 */
/* Anza only even attemps to update the account permissions if it is a
Expand Down

0 comments on commit 12a83f1

Please sign in to comment.