Fixed issue where a CLC without global permissions removed load global
permission from the sealed loaded capability.
PiperOrigin-RevId: 707934877
Change-Id: Iea1f3eb08824c176852cdaae27aa46ded33a876e
diff --git a/cheriot/riscv_cheriot_instructions.cc b/cheriot/riscv_cheriot_instructions.cc
index d1b1637..5d4c3ef 100644
--- a/cheriot/riscv_cheriot_instructions.cc
+++ b/cheriot/riscv_cheriot_instructions.cc
@@ -401,23 +401,24 @@
auto *cd = GetCapDest(instruction, 0);
cd->Expand(context->db->Get<uint32_t>(0), context->db->Get<uint32_t>(1),
context->tag_db->Get<uint8_t>(0));
+ // If the source capability did not have load/store capability, invalidate.
+ if ((context->permissions & CapReg::kPermitLoadStoreCapability) == 0) {
+ cd->Invalidate();
+ }
if (cd->tag()) {
if ((context->permissions & CapReg::kPermitLoadGlobal) == 0) {
- cd->ClearPermissions(CapReg::kPermitGlobal | CapReg::kPermitLoadGlobal);
+ cd->ClearPermissions(CapReg::kPermitGlobal);
+ if (!cd->IsSealed()) cd->ClearPermissions(CapReg::kPermitLoadGlobal);
}
if (!cd->IsSealed() &&
((context->permissions & CapReg::kPermitLoadMutable) == 0)) {
cd->ClearPermissions(CapReg::kPermitStore | CapReg::kPermitLoadMutable);
}
- // If the source capability did not have load/store capability, invalidate.
- if ((context->permissions & CapReg::kPermitLoadStoreCapability) == 0) {
- cd->Invalidate();
- }
// If it's not a sealing cap, check for revocation.
- if (cd->tag() &&
- ((cd->permissions() & (CapReg::kPermitSeal | CapReg::kPermitUnseal |
- CapReg::kUserPerm0)) == 0)) {
- if (state->MustRevoke(cd->base())) {
+ if ((cd->permissions() & (CapReg::kPermitSeal | CapReg::kPermitUnseal |
+ CapReg::kUserPerm0)) == 0) {
+ auto granule_addr = cd->base() & ~((1ULL << CapReg::kGranuleShift) - 1);
+ if (state->MustRevoke(granule_addr)) {
cd->Invalidate();
}
}
diff --git a/cheriot/test/riscv_cheriot_instructions_test.cc b/cheriot/test/riscv_cheriot_instructions_test.cc
index 2a10c91..bf66129 100644
--- a/cheriot/test/riscv_cheriot_instructions_test.cc
+++ b/cheriot/test/riscv_cheriot_instructions_test.cc
@@ -90,6 +90,7 @@
constexpr char kC2[] = "c12";
constexpr char kC3[] = "c13";
constexpr char kC4[] = "c14";
+constexpr char kC5[] = "c15";
// Register number definitions.
constexpr int kC1Num = 11;
constexpr int kPccNum = 0b1'00000;
@@ -117,6 +118,7 @@
{kC2, &c2_reg_},
{kC3, &c3_reg_},
{kC4, &c4_reg_},
+ {kC5, &c5_reg_},
{kCra, &cra_reg_}}) {
*cap_reg_ptr = state_->GetRegister<CheriotRegister>(reg_name).first;
}
@@ -219,6 +221,7 @@
CheriotRegister *c2_reg() { return c2_reg_; }
CheriotRegister *c3_reg() { return c3_reg_; }
CheriotRegister *c4_reg() { return c4_reg_; }
+ CheriotRegister *c5_reg() { return c5_reg_; }
CheriotRegister *cra_reg() { return cra_reg_; }
absl::BitGen &bitgen() { return bitgen_; }
bool trap_taken() { return trap_taken_; }
@@ -236,6 +239,7 @@
CheriotRegister *c2_reg_;
CheriotRegister *c3_reg_;
CheriotRegister *c4_reg_;
+ CheriotRegister *c5_reg_;
CheriotRegister *cra_reg_;
absl::BitGen bitgen_;
bool trap_taken_ = false;
@@ -935,8 +939,9 @@
EXPECT_TRUE(*c3_reg() == *state()->memory_root());
}
-// Load without global flag should clear global flag of loaded capability.
-TEST_F(RiscVCheriotInstructionsTest, CLcNoLoadGlobal) {
+// Load unsealed without global flag should clear global and load global flags
+// of loaded capability.
+TEST_F(RiscVCheriotInstructionsTest, CLcNoLoadGlobalUnsealed) {
c4_reg()->ResetMemoryRoot();
c4_reg()->ClearPermissions(PB::kPermitGlobal | PB::kPermitLoadGlobal);
SetUpForLoadCapabilityTest(kMemAddress + 0x200, state()->memory_root());
@@ -952,6 +957,29 @@
EXPECT_TRUE(*c3_reg() == *c4_reg());
}
+// Load sealed without global flag should clear global flag of loaded
+// capability.
+TEST_F(RiscVCheriotInstructionsTest, CLcNoLoadGlobalSealed) {
+ c4_reg()->ResetMemoryRoot();
+ c4_reg()->ClearPermissions(PB::kPermitGlobal);
+ c4_reg()->set_address(0xdeadbeef);
+ CHECK_OK(c4_reg()->Seal(*state()->sealing_root(), kDataSeal10));
+ c5_reg()->ResetMemoryRoot();
+ CHECK_OK(c5_reg()->Seal(*state()->sealing_root(), kDataSeal10));
+ SetUpForLoadCapabilityTest(kMemAddress + 0x200, c5_reg());
+ AppendCapabilityOperands(inst(), {kC1, kC2}, {});
+ AppendCapabilityOperands(inst()->child(), {}, {kC3});
+ c1_reg()->ResetMemoryRoot();
+ c1_reg()->ClearPermissions(PB::kPermitLoadGlobal);
+ c1_reg()->set_address(kMemAddress);
+ c2_reg()->set_address(0x200);
+ inst()->Execute(nullptr);
+ EXPECT_FALSE(trap_taken());
+ EXPECT_FALSE(*c3_reg() == *state()->memory_root());
+ EXPECT_TRUE(*c3_reg() == *c4_reg()) << "c3_reg(): " << c3_reg()->AsString()
+ << "\nc4_reg(): " << c4_reg()->AsString();
+}
+
// Load without mutable flag should clear mutable and store permissions of
// unsealed capability.
TEST_F(RiscVCheriotInstructionsTest, CLcNoLoadMutableUnsealed) {
@@ -997,13 +1025,9 @@
c2_reg()->set_address(0x200);
inst()->Execute(nullptr);
EXPECT_FALSE(trap_taken());
- // Result should be equal to memory root, but without the valid tag and some
- // permissions removed.
+ // Result should be equal to memory root, but without the valid tag.
c4_reg()->ResetMemoryRoot();
c4_reg()->set_address(0xdeadbeef);
- c4_reg()->ClearPermissions(
- PB::kPermitGlobal | PB::kPermitLoadGlobal | PB::kPermitLoadMutable |
- PB::kPermitStoreLocalCapability | PB::kPermitStore);
c4_reg()->Invalidate();
EXPECT_TRUE(*c3_reg() == *c4_reg()) << "c3_reg(): " << c3_reg()->AsString()
<< "\nc4_reg(): " << c4_reg()->AsString();
