diff --git a/fvm/src/call_manager/default.rs b/fvm/src/call_manager/default.rs index 1c1d0f76dc..8acb7628ce 100644 --- a/fvm/src/call_manager/default.rs +++ b/fvm/src/call_manager/default.rs @@ -14,6 +14,7 @@ use fvm_shared::event::StampedEvent; use fvm_shared::sys::BlockId; use fvm_shared::{ActorID, METHOD_SEND}; use num_traits::Zero; +use std::collections::HashMap; use super::state_access_tracker::{ActorAccessState, StateAccessTracker}; use super::{Backtrace, CallManager, Entrypoint, InvocationResult, NO_DATA_BLOCK_ID}; @@ -75,6 +76,7 @@ pub struct InnerDefaultCallManager { limits: M::Limiter, /// Accumulator for events emitted in this call stack. events: EventsAccumulator, + code_upgrade_by_actor: HashMap, } #[doc(hidden)] @@ -159,6 +161,7 @@ where limits, events: Default::default(), state_access_tracker, + code_upgrade_by_actor: HashMap::new(), }))) } @@ -327,6 +330,10 @@ where self.nonce } + fn get_actor_upgrade_stack(&mut self) -> &mut HashMap { + &mut self.code_upgrade_by_actor + } + fn next_actor_address(&self) -> Address { // Base the next address on the address specified as the message origin. This lets us use, // e.g., an f2 address even if we can't look it up anywhere. diff --git a/fvm/src/call_manager/mod.rs b/fvm/src/call_manager/mod.rs index 2e5b2d7fda..dd91358529 100644 --- a/fvm/src/call_manager/mod.rs +++ b/fvm/src/call_manager/mod.rs @@ -7,6 +7,7 @@ use fvm_shared::econ::TokenAmount; use fvm_shared::error::ExitCode; use fvm_shared::upgrade::UpgradeInfo; use fvm_shared::{ActorID, MethodNum}; +use std::collections::HashMap; use crate::engine::Engine; use crate::gas::{Gas, GasCharge, GasTimer, GasTracker, PriceList}; @@ -119,6 +120,8 @@ pub trait CallManager: 'static { delegated_address: Option
, ) -> Result<()>; + fn get_actor_upgrade_stack(&mut self) -> &mut HashMap; + /// Resolve an address into an actor ID, charging gas as appropriate. fn resolve_address(&self, address: &Address) -> Result>; diff --git a/fvm/src/kernel/default.rs b/fvm/src/kernel/default.rs index 72c024d807..aad8c63713 100644 --- a/fvm/src/kernel/default.rs +++ b/fvm/src/kernel/default.rs @@ -873,6 +873,14 @@ where .create_actor(code_id, actor_id, delegated_address) } + fn actor_on_upgrade_stack(&mut self) -> bool { + self.call_manager + .get_actor_upgrade_stack() + .get(&self.actor_id) + .map(|count| *count > 0) + .unwrap_or(false) + } + fn upgrade_actor( &mut self, new_code_cid: Cid, @@ -896,6 +904,12 @@ where return Err(syscall_error!(LimitExceeded; "cannot store return block").into()); } + self.call_manager + .get_actor_upgrade_stack() + .entry(self.actor_id) + .and_modify(|count| *count += 1) + .or_insert(1); + let result = self.call_manager.with_transaction(|cm| { let origin = cm.origin(); diff --git a/fvm/src/kernel/mod.rs b/fvm/src/kernel/mod.rs index a473380037..00e2b6156f 100644 --- a/fvm/src/kernel/mod.rs +++ b/fvm/src/kernel/mod.rs @@ -215,6 +215,8 @@ pub trait ActorOps { params_id: BlockId, ) -> Result; + fn actor_on_upgrade_stack(&mut self) -> bool; + /// Installs actor code pointed by cid #[cfg(feature = "m2-native")] fn install_actor(&mut self, code_cid: Cid) -> Result<()>; diff --git a/fvm/src/syscalls/actor.rs b/fvm/src/syscalls/actor.rs index 9aeba30388..b4ca3c69a3 100644 --- a/fvm/src/syscalls/actor.rs +++ b/fvm/src/syscalls/actor.rs @@ -121,6 +121,14 @@ pub fn upgrade_actor( Err(err) => return err.into(), }; + // Check if this actor is already being upgraded. + if context.kernel.actor_on_upgrade_stack() { + return ControlFlow::Error(syscall_error!( + Forbidden; + "re-entrant upgrade detected" + )); + }; + match context.kernel.upgrade_actor::(cid, params_id) { Ok(SendResult { block_id, diff --git a/fvm/tests/dummy.rs b/fvm/tests/dummy.rs index 677d9a0b5c..6e38f9a60e 100644 --- a/fvm/tests/dummy.rs +++ b/fvm/tests/dummy.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0, MIT use std::borrow::Borrow; use std::cell::RefCell; +use std::collections::HashMap; use std::rc::Rc; use anyhow::Context; @@ -358,6 +359,10 @@ impl CallManager for DummyCallManager { todo!() } + fn get_actor_upgrade_stack(&mut self) -> &mut HashMap { + todo!() + } + fn invocation_count(&self) -> u64 { todo!() } diff --git a/testing/conformance/src/vm.rs b/testing/conformance/src/vm.rs index 549cbd2515..57410697c1 100644 --- a/testing/conformance/src/vm.rs +++ b/testing/conformance/src/vm.rs @@ -302,6 +302,10 @@ where fn upgrade_actor(&mut self, new_code_cid: Cid, params_id: BlockId) -> Result { self.0.upgrade_actor::(new_code_cid, params_id) } + + fn actor_on_upgrade_stack(&mut self) -> bool { + self.0.actor_on_upgrade_stack() + } } impl IpldBlockOps for TestKernel diff --git a/testing/integration/tests/main.rs b/testing/integration/tests/main.rs index b40f169db5..22b3a10455 100644 --- a/testing/integration/tests/main.rs +++ b/testing/integration/tests/main.rs @@ -1188,9 +1188,6 @@ fn upgrade_actor_test() { .execute_message(message, ApplyKind::Explicit, 100) .unwrap(); - let val: i64 = res.msg_receipt.return_data.deserialize().unwrap(); - assert_eq!(val, 444); - assert!( res.msg_receipt.exit_code.is_success(), "{:?}", diff --git a/testing/test_actors/actors/fil-upgrade-actor/src/actor.rs b/testing/test_actors/actors/fil-upgrade-actor/src/actor.rs index 525d25ec7b..dbb80a0b9a 100644 --- a/testing/test_actors/actors/fil-upgrade-actor/src/actor.rs +++ b/testing/test_actors/actors/fil-upgrade-actor/src/actor.rs @@ -4,6 +4,7 @@ use fvm_ipld_encoding::ipld_block::IpldBlock; use fvm_ipld_encoding::{to_vec, CBOR}; use fvm_sdk as sdk; use fvm_shared::address::Address; +use fvm_shared::error::ErrorNumber; use fvm_shared::upgrade::UpgradeInfo; use serde_tuple::*; #[derive(Serialize_tuple, Deserialize_tuple, PartialEq, Eq, Clone, Debug)] @@ -48,8 +49,9 @@ pub fn upgrade(params_id: u32, upgrade_info_id: u32) -> u32 { sdk::debug::log("[upgrade] params:3, calling upgrade within an upgrade".to_string()); let new_code_cid = sdk::actor::get_actor_code_cid(&Address::new_id(10000)).unwrap(); let params = IpldBlock::serialize_cbor(&SomeStruct { value: 4 }).unwrap(); - let _ = sdk::actor::upgrade_actor(new_code_cid, params); - unreachable!("we should never return from a successful upgrade"); + let res = sdk::actor::upgrade_actor(new_code_cid, params); + assert_eq!(res, Err(ErrorNumber::Forbidden)); + 0 } 4 => { let block_id = sdk::ipld::put_block(CBOR, &to_vec(&444).unwrap()).unwrap(); @@ -88,7 +90,7 @@ pub fn invoke(_: u32) -> u32 { "invalid exit code returned from upgrade_actor" ); } - // test recursive update + // test that calling recursive update on the same actor fails 3 => { let new_code_cid = sdk::actor::get_actor_code_cid(&Address::new_id(10000)).unwrap(); let params = IpldBlock::serialize_cbor(&SomeStruct { value: 3 }).unwrap();