Merged PR 6872685: Ignore relocations in data section. Do not optimize Debug mode binaries.

It is possible for relocations to occur within the .data section, but that section is excluded from our FIPS boundary and therefore also from the writeable segment copy that the integrity verification code uses to reverse the module's relocations. However, the code didn't account for this, which resulted in reading and writing off the end of the buffer when relocations existed in .data, causing heap corruption. The fix is to just ignore these relocations since they're outside the FIPS boundary anyway.

Also changed CMakeFiles to set -O0 on Debug builds so that temporary values aren't optimized out. This might make the tests take longer to run, but it significantly improves debuggability.

CMakeLists.txt

@@ -85,6 +85,13 @@ else()
     # using GCC-specific pragmas for the loops of interest)
     add_compile_options(-fno-unroll-loops)
 
+    # Do not optimize Debug builds
+    if (CMAKE_BUILD_TYPE MATCHES Debug)
+        add_compile_options(-O0)
+    else()
+        add_compile_options(-O3)
+    endif()
+
     # In Sanitize version, enable sanitizers
     if (CMAKE_BUILD_TYPE MATCHES Sanitize)
         add_compile_options(-fsanitize=address)

cmake-toolchain/LinuxUserMode-AMD64.cmake

@@ -10,7 +10,6 @@ set(SYMCRYPT_TARGET_ENV LinuxUserMode)
 
 # Define _AMD64_ to set up the correct SymCrypt macros, e.g. SYMCRYPT_CPU_AMD64
 add_compile_options(-D_AMD64_)
-add_compile_options(-O3)
 
 # Enable FIPS build
 add_compile_options(-DSYMCRYPT_DO_FIPS_SELFTESTS=1)

cmake-toolchain/LinuxUserMode-ARM64.cmake

@@ -33,7 +33,6 @@ set(SYMCRYPT_TARGET_ENV LinuxUserMode)
 
 # Define _ARM64_ to set up the correct SymCrypt macros, e.g. SYMCRYPT_CPU_ARM64
 add_compile_options(-D_ARM64_)
-add_compile_options(-O3)
 
 # Enable FIPS build
 add_compile_options(-DSYMCRYPT_DO_FIPS_SELFTESTS=1)

module/linux_common/integrity.c

@@ -209,22 +209,32 @@ size_t SymCryptModuleProcessSectionWithRelocations(
         const Elf64_Rela* rela = relaInfo->rela + i;
 
         // Find the relocation within the section. Note that for a shared object module,
-        // rela->r_offset is actually a virtual address
-        Elf64_Xword* target = (Elf64_Xword*) ( segmentCopy +
-            (Elf64_Off) rela->r_offset - (Elf64_Off) programHeader->p_vaddr );
+        // rela->r_offset is actually a virtual address. Relocations can occur within the .data
+        // section, which is outside our FIPS boundary, so any such relocations can be ignored.
+        Elf64_Off offsetInBuffer = (Elf64_Off) rela->r_offset - (Elf64_Off) programHeader->p_vaddr;
+        if( offsetInBuffer > hashableSectionSize )
+        {
+            continue;
+        }
+
+        Elf64_Xword* target = (Elf64_Xword*) ( segmentCopy + offsetInBuffer);
         
         SymCryptModuleUndoRelocation( module_base, target, rela );
     }
 
-    // Process the GOT entries
+    // Process the GOT entries from the .rela.plt section. Same as process above, just
+    // with a different table.
     for( size_t i = 0; i < relaInfo->pltRelaEntryCount; ++i)
     {
         const Elf64_Rela* rela = relaInfo->pltRela + i;
 
-        // Find the relocation within the section. Note that for a shared object module,
-        // rela->r_offset is actually a virtual address
-        Elf64_Xword* target = (Elf64_Xword*) ( segmentCopy +
-            (Elf64_Off) rela->r_offset - (Elf64_Off) programHeader->p_vaddr );
+        Elf64_Off offsetInBuffer = (Elf64_Off) rela->r_offset - (Elf64_Off) programHeader->p_vaddr;
+        if( offsetInBuffer > hashableSectionSize )
+        {
+            continue;
+        }
+
+        Elf64_Xword* target = (Elf64_Xword*) ( segmentCopy + offsetInBuffer);
         
         SymCryptModuleUndoRelocation( module_base, target, rela );
     }