Fixed issue where a CLC without global permissions removed load global
permission from the sealed loaded capability.

PiperOrigin-RevId: 707934877
Change-Id: Iea1f3eb08824c176852cdaae27aa46ded33a876e

cheriot/riscv_cheriot_instructions.cc

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();
       }
     }

cheriot/test/riscv_cheriot_instructions_test.cc

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();