[10/10] ELF/Aarch64: Add overflow checks for relocation writes

[zatrazz]

Summary

Hi ruiu, shankarke,

This is re-post of a previous attempt. This patch adds support for overflow checking when processing
R_AARCH64_CALL26, R_AARCH64_JUMP26, R_AARCH64_CONDBR19,
R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21, and R_AARCH64_TLSLE_ADD_TPREL_HI12
relocations.

[ruiu]

lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp

161

Does the result of S+A-P always fit within int32_t? If not, upper bits are cleared by converting an int64_t result into int32_t, which would result in false test pass in the following isInt<27>() check. Maybe we want to always use int64_t types here to avoid confusion and possible errors.

180

The same comment applies here.

356

Ditto

[zatrazz]

Hi,

Thanks for the review.

[ruiu]

LGTM assuming you will change int32 to int64.

lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp

static void relocR_AARCH64_ADD_ABS_LO12_NC(uint8_t *location, uint64_t P,
   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: " << Twine::utohexstr(S);
         llvm::dbgs() << " A: " << Twine::utohexstr(A);
         llvm::dbgs() << " P: " << Twine::utohexstr(P);
         llvm::dbgs() << " result: " << Twine::utohexstr(result) << "\n");
   write32le(location, result | read32le(location));
 }
 
-static void relocJump26(uint8_t *location, uint64_t P, uint64_t S, int64_t A) {
+/// \brief R_AARCH64_CALL26 and R_AARCH64_JUMP26
+static std::error_code relocJump26(uint8_t *location, uint64_t P, uint64_t S,
+                                   int64_t A) {
   int32_t result = (int32_t)((S + A) - P);
+  if (!isInt<27>(result))
+    return make_out_of_range_reloc_error();
   result &= 0x0FFFFFFC;
   result >>= 2;
   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: " << Twine::utohexstr(S);
         llvm::dbgs() << " A: " << Twine::utohexstr(A);
         llvm::dbgs() << " P: " << Twine::utohexstr(P);
         llvm::dbgs() << " result: " << Twine::utohexstr(result) << "\n");
   write32le(location, result | read32le(location));
+  return std::error_code();
 }
 
 /// \brief R_AARCH64_CONDBR19
-static void relocR_AARCH64_CONDBR19(uint8_t *location, uint64_t P, uint64_t S,
-                                    int64_t A) {
+static std::error_code relocR_AARCH64_CONDBR19(uint8_t *location, uint64_t P,
+                                               uint64_t S, int64_t A) {
   int32_t result = (int32_t)((S + A) - P);
+  if (!isInt<20>(result))
+    return make_out_of_range_reloc_error();
   result &= 0x01FFFFC;
   result <<= 3;
   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: " << Twine::utohexstr(S);
         llvm::dbgs() << " A: " << Twine::utohexstr(A);
         llvm::dbgs() << " P: " << Twine::utohexstr(P);
         llvm::dbgs() << " result: " << Twine::utohexstr(result) << "\n");
   write32le(location, result | read32le(location));
+  return std::error_code();
 }
 
 /// \brief R_AARCH64_LDST8_ABS_LO12_NC - S + A
 static void relocR_AARCH64_LDST8_ABS_LO12_NC(uint8_t *location, uint64_t P,
                                              uint64_t S, int64_t A) {
   int32_t result = (int32_t)((S + A) & 0xFFF);
   result <<= 10;
   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " P: " << Twine::utohexstr(P);
         llvm::dbgs() << " result: " << Twine::utohexstr(result) << "\n");
   result &= 0xFFF;
   result <<= 10;
   write32le(location, result | read32le(location));
 }
 
 // R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21
-static void relocR_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21(uint8_t *location,
-                                                     uint64_t P, uint64_t S,
+static std::error_code relocR_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21(uint8_t *location,
+                                                                uint64_t P,
+                                                                uint64_t S,
                                                                 int64_t A) {
   int64_t result = PAGE(S + A) - PAGE(P);
+  if (!isInt<32>(result))
+    return make_out_of_range_reloc_error();
   result >>= 12;
   uint32_t immlo = result & 0x3;
   uint32_t immhi = result & 0x1FFFFC;
   immlo = immlo << 29;
   immhi = immhi << 3;
   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: " << Twine::utohexstr(S);
         llvm::dbgs() << " A: " << Twine::utohexstr(A);
         llvm::dbgs() << " P: " << Twine::utohexstr(P);
         llvm::dbgs() << " immhi: " << Twine::utohexstr(immhi);
         llvm::dbgs() << " immlo: " << Twine::utohexstr(immlo);
         llvm::dbgs() << " result: " << Twine::utohexstr(result) << "\n");
   write32le(location, immlo | immhi | read32le(location));
+  return std::error_code();
 }
 
 // R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC
 static void relocR_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC(uint8_t *location,
                                                        uint64_t P, uint64_t S,
                                                        int64_t A) {
   int32_t result = S + A;
   result &= 0xFF8;
   result <<= 7;
   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: " << Twine::utohexstr(S);
         llvm::dbgs() << " A: " << Twine::utohexstr(A);
         llvm::dbgs() << " P: " << Twine::utohexstr(P);
         llvm::dbgs() << " result: " << Twine::utohexstr(result) << "\n");
   write32le(location, result | read32le(location));
 }
 
 /// \brief R_AARCH64_TLSLE_ADD_TPREL_HI12
-static void relocR_AARCH64_TLSLE_ADD_TPREL_HI12(uint8_t *location, uint64_t P,
-                                                uint64_t S, int64_t A) {
+static std::error_code relocR_AARCH64_TLSLE_ADD_TPREL_HI12(uint8_t *location,
+                                                           uint64_t P,
+                                                           uint64_t S,
+                                                           int64_t A) {
   int32_t result = S + A;
+  if (!isUInt<24>(result))
+    return make_out_of_range_reloc_error();
   result &= 0x0FFF000;
   result >>= 2;
   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: " << Twine::utohexstr(S);
         llvm::dbgs() << " A: " << Twine::utohexstr(A);
         llvm::dbgs() << " P: " << Twine::utohexstr(P);
         llvm::dbgs() << " result: " << Twine::utohexstr(result) << "\n");
   write32le(location, result | read32le(location));
+  return std::error_code();
 }
 
 /// \brief R_AARCH64_TLSLE_ADD_TPREL_LO12_NC
 static void relocR_AARCH64_TLSLE_ADD_TPREL_LO12_NC(uint8_t *location,
                                                    uint64_t P, uint64_t S,
                                                    int64_t A) {
   int32_t result = S + A;
   result &= 0x0FFF;
case R_AARCH64_ADR_PREL_PG_HI21:
     return relocR_AARCH64_ADR_PREL_PG_HI21(loc, reloc, target, addend);
   case R_AARCH64_ADR_PREL_LO21:
     return relocR_AARCH64_ADR_PREL_LO21(loc, reloc, target, addend);
   case R_AARCH64_ADD_ABS_LO12_NC:
     relocR_AARCH64_ADD_ABS_LO12_NC(loc, reloc, target, addend);
     break;
   case R_AARCH64_CALL26:
   case R_AARCH64_JUMP26:
-    relocJump26(loc, reloc, target, addend);
-    break;
+    return relocJump26(loc, reloc, target, addend);
   case R_AARCH64_CONDBR19:
-    relocR_AARCH64_CONDBR19(loc, reloc, target, addend);
-    break;
+    return relocR_AARCH64_CONDBR19(loc, reloc, target, addend);
   case R_AARCH64_ADR_GOT_PAGE:
     return relocR_AARCH64_ADR_GOT_PAGE(loc, reloc, target, addend);
   case R_AARCH64_LD64_GOT_LO12_NC:
     return relocR_AARCH64_LD64_GOT_LO12_NC(loc, reloc, target, addend);
   case R_AARCH64_LDST8_ABS_LO12_NC:
     relocR_AARCH64_LDST8_ABS_LO12_NC(loc, reloc, target, addend);
     break;
   case R_AARCH64_LDST16_ABS_LO12_NC:
     relocR_AARCH64_LDST16_ABS_LO12_NC(loc, reloc, target, addend);
     break;
   case R_AARCH64_LDST32_ABS_LO12_NC:
     relocR_AARCH64_LDST32_ABS_LO12_NC(loc, reloc, target, addend);
     break;
   case R_AARCH64_LDST64_ABS_LO12_NC:
     relocR_AARCH64_LDST64_ABS_LO12_NC(loc, reloc, target, addend);
     break;
   case R_AARCH64_LDST128_ABS_LO12_NC:
     relocR_AARCH64_LDST128_ABS_LO12_NC(loc, reloc, target, addend);
     break;
   case ADD_AARCH64_GOTRELINDEX:
     relocADD_AARCH64_GOTRELINDEX(loc, reloc, target, addend);
     break;
   case R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21:
-    relocR_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21(loc, reloc, target, addend);
-    break;
+    return relocR_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21(loc, reloc, target, addend);
   case R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC:
     relocR_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC(loc, reloc, target, addend);
     break;
   case R_AARCH64_TLSLE_ADD_TPREL_HI12:
-    relocR_AARCH64_TLSLE_ADD_TPREL_HI12(loc, reloc, target, addend);
-    break;
+    return relocR_AARCH64_TLSLE_ADD_TPREL_HI12(loc, reloc, target, addend);
   case R_AARCH64_TLSLE_ADD_TPREL_LO12_NC:
     relocR_AARCH64_TLSLE_ADD_TPREL_LO12_NC(loc, reloc, target, addend);
     break;
   default:
     return make_unhandled_reloc_error();
   }
   return std::error_code();
 }