Merged PR 7775773: Fix integer truncation issue in 32-bit Linux builds

mlindgren · mlindgren · commit 1fbad55b878d · 2022-08-31T00:28:02.000Z
This change fixes an incorrect implementation of the `SYMCRYPT_MUL32x32TO64` when compiling for 32-bit architectures using gcc or clang. Previously the macro cast the input parameters to `unsigned long`, which is 32 bits on some platforms and 64 bits on others. This resulted in the multiplication truncating the top half of the word. The fix is to cast to UINT64 instead.

Also added 32-bit Linux unit tests to the Azure pipeline so it will be built and tested in our CI builds.

Related work items: #32615192
diff --git a/README.md b/README.md
@@ -18,7 +18,7 @@ Like any engineering project, SymCrypt is a compromise between conflicting requi
 - Support FIPS 140-2 certification of products using SymCrypt.
 - Provide high assurance in the proper functionality of the library.
 
-# Clone the Repo
+# Cloning the Repo
 In some of our Linux modules, SymCrypt uses [Jitterentropy](https://github.com/smuellerDD/jitterentropy-library)
 as a source of FIPS-certifiable entropy. To build these modules, you will need to ensure that the
 jitterentropy-library submodule is also cloned. You can do this by running
@@ -30,36 +30,56 @@ publicly, so this submodule will only be cloneable by Microsoft employees with a
 Azure DevOps repository. If you are external to Microsoft, you can ignore this submodule. It is only used in
 the unit tests and does not change the behavior of the SymCrypt product code.
 
-# Build and Test
+# Building
+## Prerequisites
 SymCrypt can be compiled with CMake >= 3.13.0 and Visual Studio 2019 (with Windows 10 SDK version 18362) on Windows
-or gcc 7.4.0 or clang 10.0.0 on Linux. Note that CMake ships with Visual Studio 2019.
+or gcc 7.4.0 or clang 10.0.0 on Linux. Note that CMake ships with Visual Studio 2019; you can use Visual Studio's
+included CMake by setting `$env:PATH="C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\;${env:PATH}"`.
 
-Python3 is also required for translation of SymCryptAsm, and for building the SymCrypt module with integrity check.
+Python 3 is also required for translation of SymCryptAsm, and for building the SymCrypt module with integrity check.
 The integrity check additionally requires pip and pyelftools: `pip3 install -r ./scripts/requirements.txt`
 
-1. Optionally use CMake from Visual Studio `$env:PATH="C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\;${env:PATH}"`
-2. For Microsoft employees building the library internally, to include msbignum and RSA32 implementation benchmarks in the unit tests:
+## Supported Configurations
+SymCrypt has pure C implementations of all supported functionality. These "generic" implementations are designed to
+be portable to various architectures. However, they do not offer optimal performance because they do not take
+advantage of CPU-specific optimizations. To that end, we also have hand-written assembly implementations of
+performance-critical internal functions. Our CMake build scripts do not currently support ASM optimizations on all
+combinations of architectures and platforms; the Build Instructions section below lists some of the currently supported
+combinations, and we're working on adding support for more.
+
+The ability to build SymCrypt on any particular platform or architecture, with or without CPU optimizations, does not
+imply that it has been tested for or is actively supported by Microsoft on that platform/architecture. While we make
+every effort to ensure that SymCrypt is reliable, stable and bug-free on every platform we run on, the code in this
+repository is provided *as is*, without warranty of any kind, express or implied, including but not limited to the
+warranties of merchantability, fitness for a particular purpose and noninfringement (see our [LICENSE](./LICENSE)).
+
+## Build Instructions
+1. For Microsoft employees building the library internally, to include msbignum and RSA32 implementation benchmarks in the unit tests:
     1. Make sure the SymCryptDependencies submodule is initialized by following the steps above (`git submodule update --init`)
-    2. In step 4 below, add the additional cmake argument `-DSYMCRYPT_INTERNAL_BUILD=1`
-3. `mkdir bin; cd bin`
-4. Configure CMake compilation:
-    * For x86 Windows targets: `cmake .. -DCMAKE_TOOLCHAIN_FILE="../cmake-toolchain/WindowsUserMode-X86.cmake" -A Win32`
+    1. In step 4 below, add the additional cmake argument `-DSYMCRYPT_INTERNAL_BUILD=1`
+1. `mkdir bin; cd bin`
+1. Use the appropriate CMake arguments to specify which architecture you want to compile for:
     * For x86-64 Windows targets: `cmake .. -DCMAKE_TOOLCHAIN_FILE="../cmake-toolchain/WindowsUserMode-AMD64.cmake"`
+    * For x86 Windows targets: `cmake .. -DCMAKE_TOOLCHAIN_FILE="../cmake-toolchain/WindowsUserMode-X86.cmake" -A Win32`
     * For x86-64 Linux targets: `cmake .. -DCMAKE_TOOLCHAIN_FILE="../cmake-toolchain/LinuxUserMode-AMD64.cmake"`
+    * For x86 Linux targets **no ASM optimizations**: `cmake .. -DCMAKE_TOOLCHAIN_FILE="../cmake-toolchain/LinuxUserMode-X86.cmake"`
     * For ARM64 Linux targets: `cmake .. -DCMAKE_TOOLCHAIN_FILE="../cmake-toolchain/LinuxUserMode-ARM64.cmake"`
-    * For no CPU optimizations: `cmake ..`
+    * To use the host system architecture **with no ASM optimizations**: `cmake ..`
     * Optionally, for a release build, specify `-DCMAKE_BUILD_TYPE=Release`
-5. `cmake --build .`
+1. `cmake --build .`
     * Optionally, for a release build on Windows, specify `--config Release`
     * Optionally specify `-jN` where N is the number of processes you wish to spawn for the build
 
-If compilation succeeds, the output will be put in the `exe` subdirectory relative to where compilation occurred
-(i.e. `bin/exe` if you followed the instructions above).
+After successful compilation, the generated binaries will be placed in the following directories relative
+to your build directory:
+* `lib/\<arch\>/\<environment\>/` - static libraries
+* `module\<arch\>/\<environment\>/` - shared object libraries (currently only on Linux)
+* `exe/\<arch\>/\<environment\>/` - unit tests
 
-The SymCrypt unit test is in the `unittest` directory. It runs extensive functional tests on the SymCrypt
-library. On Windows it also compares results against on other implementations such as the Windows APIs CNG
-and CAPI, and the older crypto libraries rsa32 and msbignum, if they are available. It also provides
-detailed performance information.
+# Testing
+The SymCrypt unit test runs extensive functional tests on the SymCrypt library. On Windows it also compares results
+against on other implementations such as the Windows APIs CNG and CAPI, and the older crypto libraries rsa32 and
+msbignum, if they are available. It also provides detailed performance information.
 
 # Versioning and Servicing
 As of version 101.0.0, SymCrypt uses the version scheme defined by the
diff --git a/azure-build-template.yml b/azure-build-template.yml
@@ -51,6 +51,9 @@ parameters:
   values:
   - native
   - qemu-aarch64
+- name: testInPrBuild
+  type: boolean
+  default: true
 
 steps:
   - checkout: self  # self represents the repo where the initial Pipelines YAML file was found
@@ -63,12 +66,17 @@ steps:
         python -m pip install --upgrade pip setuptools wheel
         pip install -r $(Build.SourcesDirectory)/scripts/requirements.txt
       displayName: 'Install Python requirements'
-  - ${{ if eq(parameters.emulator, 'qemu-aarch64') }}:
-    - script: |
-        sudo apt-get update
-        sudo apt-get install qemu-user binutils-aarch64-linux-gnu
-        sudo apt-get install gcc-aarch64-linux-gnu g++-aarch64-linux-gnu
-      displayName: 'Install arm64 cross-compilation and emulation tools'
+    - ${{ if eq(parameters.arch, 'X86') }}:
+      - script: |
+          sudo apt-get update
+          sudo apt-get install gcc-multilib g++-multilib
+        displayName: 'Install x86 headers and libraries'
+    - ${{ if eq(parameters.emulator, 'qemu-aarch64') }}:
+      - script: |
+          sudo apt-get update
+          sudo apt-get install qemu-user binutils-aarch64-linux-gnu
+          sudo apt-get install gcc-aarch64-linux-gnu g++-aarch64-linux-gnu
+        displayName: 'Install arm64 cross-compilation and emulation tools'
   # Specify no toolchain file for generic build
   - ${{ if eq(parameters.env, 'Generic') }}:
     - task: CMake@1
@@ -98,14 +106,14 @@ steps:
         workingDirectory: '$(Build.SourcesDirectory)/bin'
         cmakeArgs: '--build . -j --config ${{parameters.buildType}}'
     # Execute unit tests using the inline script
-    - ${{ if ne(parameters.env, 'Generic') }}:
+    - ${{ if eq(parameters.testInPrBuild, true) }}:
       - script: |
           cd bin\exe\${{parameters.arch}}\${{parameters.env}}\${{parameters.buildType}}
           .\symcryptunittest.exe dynamic:symcrypttestmodule.dll
         displayName: 'Execute unit tests with SymCrypt test module'
         name: '${{parameters.env}}UnitTest_${{parameters.buildType}}'
     # Execute Generic unit tests in CI and in PRs to publish
-    - ${{ if eq(parameters.env, 'Generic') }}:
+    - ${{ if eq(parameters.testInPrBuild, false) }}:
       - script: |
           cd bin\exe\%PROCESSOR_ARCHITECTURE%\${{parameters.env}}\${{parameters.buildType}}
           .\symcryptunittest.exe dynamic:symcrypttestmodule.dll
@@ -124,7 +132,7 @@ steps:
         ulimit -c unlimited
         cp $(Build.SourcesDirectory)/.artifactignore $(Agent.WorkFolder)
       displayName: 'Enable core dumps & Place .artifactignore file'
-    - ${{ if ne(parameters.env, 'Generic') }}:
+    - ${{ if eq(parameters.testInPrBuild, true) }}:
       # Execute unit test using the inline script
       - ${{ if eq(parameters.emulator, 'native') }}:
         - script: |
@@ -169,19 +177,19 @@ steps:
           displayName: 'Execute unit tests with generic SymCrypt module'
           name: '${{parameters.env}}UnitTestDynamicGeneric_${{parameters.buildType}}_QEMU_AARCH64'
     # Execute Generic unit tests in CI and in PRs to publish
-    - ${{ if eq(parameters.env, 'Generic') }}:
+    - ${{ if eq(parameters.testInPrBuild, false) }}:
       - script: |
           archName=`uname -m`
           cd bin/exe/${archName}/${{parameters.env}}
-          ./symcryptunittest
+          ./symcryptunittest noperftests
         displayName: 'Execute unit tests'
         name: '${{parameters.env}}UnitTest_${{parameters.buildType}}'
         condition: or(eq(variables['System.PullRequest.TargetBranch'], 'refs/heads/publish'), in(variables['Build.Reason'], 'IndividualCI', 'BatchedCI'))
       # Run unit tests against the generic SymCrypt module
       - script: |
           archName=`uname -m`
           cd bin/exe/${archName}/${{parameters.env}}
-          ./symcryptunittest dynamic:../../../module/${archName}/${{parameters.env}}/generic/libsymcrypt.so
+          ./symcryptunittest dynamic:../../../module/${archName}/${{parameters.env}}/generic/libsymcrypt.so noperftests
         displayName: 'Execute unit tests with generic SymCrypt module'
         name: '${{parameters.env}}UnitTestDynamicGeneric_${{parameters.buildType}}'
         condition: or(eq(variables['System.PullRequest.TargetBranch'], 'refs/heads/publish'), in(variables['Build.Reason'], 'IndividualCI', 'BatchedCI'))
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -32,8 +32,9 @@ pr:
 # The following 4 jobs only run the unit tests on CI and PRs to publish (they are always built)
 # 13. Windows 64b with no CPU optimizations in Release mode
 # 14. Windows 32b with no CPU optimizations in Release mode
-# 15. Linux with no CPU optimizations using GCC in Release mode
-# 16. Linux with no CPU optimizations using clang in Release mode
+# 15. Linux 64b with no CPU optimizations using GCC in Release mode
+# 16. Linux 64b with no CPU optimizations using clang in Release mode
+# 17. Linux 32b with no CPU optimizations using GCC in Release mode
 
 jobs:
 - job: Windows_AMD64_Debug
@@ -106,7 +107,6 @@ jobs:
       cc: gcc
       cxx: g++
       buildType: Debug
-      additionalCMakeArgs:
 
 - job: Linux_AMD64_gcc_Sanitize
   pool:
@@ -120,7 +120,6 @@ jobs:
       cc: gcc
       cxx: g++
       buildType: Sanitize
-      additionalCMakeArgs:
 
 - job: Linux_AMD64_gcc_Release
   pool:
@@ -134,7 +133,6 @@ jobs:
       cc: gcc
       cxx: g++
       buildType: Release
-      additionalCMakeArgs:
 
 - job: Linux_AMD64_clang_Debug
   pool:
@@ -148,7 +146,6 @@ jobs:
       cc: clang
       cxx: clang++
       buildType: Debug
-      additionalCMakeArgs:
 
 - job: Linux_AMD64_clang_Sanitize
   pool:
@@ -162,7 +159,6 @@ jobs:
       cc: clang
       cxx: clang++
       buildType: Sanitize
-      additionalCMakeArgs:
 
 - job: Linux_AMD64_clang_Release
   pool:
@@ -176,7 +172,6 @@ jobs:
       cc: clang
       cxx: clang++
       buildType: Release
-      additionalCMakeArgs:
 
 - job: Linux_QEMU_ARM64_clang_Debug
   pool:
@@ -190,7 +185,6 @@ jobs:
       cc: clang
       cxx: clang++
       buildType: Debug
-      additionalCMakeArgs:
       emulator: qemu-aarch64
 
 - job: Linux_QEMU_ARM64_clang_Release
@@ -205,7 +199,6 @@ jobs:
       cc: clang
       cxx: clang++
       buildType: Release
-      additionalCMakeArgs:
       emulator: qemu-aarch64
 
 - job: Generic_Windows_Win64_Release
@@ -220,7 +213,7 @@ jobs:
       cc: cl
       cxx: cl
       buildType: Release
-      additionalCMakeArgs:
+      testInPrBuild: false
   timeoutInMinutes: 120
 
 - job: Generic_Windows_Win32_Release
@@ -236,9 +229,10 @@ jobs:
       cxx: cl
       buildType: Release
       additionalCMakeArgs: -A Win32
+      testInPrBuild: false
   timeoutInMinutes: 120
 
-- job: Generic_Linux_gcc_Release
+- job: Generic_Linux64_gcc_Release
   pool:
     vmImage: 'ubuntu-20.04'
   steps:
@@ -250,10 +244,10 @@ jobs:
       cc: gcc
       cxx: g++
       buildType: Release
-      additionalCMakeArgs:
+      testInPrBuild: false
   timeoutInMinutes: 120
 
-- job: Generic_Linux_clang_Release
+- job: Generic_Linux64_clang_Release
   pool:
     vmImage: 'ubuntu-20.04'
   steps:
@@ -265,5 +259,20 @@ jobs:
       cc: clang
       cxx: clang++
       buildType: Release
-      additionalCMakeArgs:
+      testInPrBuild: false
   timeoutInMinutes: 120
+
+- job: Generic_Linux32_gcc_Release
+  pool:
+    vmImage: 'ubuntu-20.04'
+  steps:
+  - template: azure-build-template.yml
+    parameters:
+      hostos: Linux
+      env: LinuxUserMode # Not using Generic environment because we have to add -m32 compiler/linker options
+      arch: X86
+      cc: gcc
+      cxx: g++
+      buildType: Release
+      testInPrBuild: false
+  timeoutInMinutes: 120
diff --git a/cmake-toolchain/LinuxUserMode-X86.cmake b/cmake-toolchain/LinuxUserMode-X86.cmake
@@ -0,0 +1,18 @@
+# This toolchain file configures CMake options for Linux User Mode X86 compilation with CPU optimizations.
+# To use the toolchain file, run cmake .. -DCMAKE_TOOLCHAIN_FILE="cmake-toolchain/LinuxUserMode-X86.cmake"
+
+# Set CMake variables that subsequent CMake scripts can check against
+set(CMAKE_SYSTEM_NAME Linux)
+set(CMAKE_SYSTEM_PROCESSOR X86)
+
+# For now this is just used for separating the output directories
+set(SYMCRYPT_TARGET_ENV LinuxUserMode)
+
+add_compile_options("-m32")
+add_link_options("-m32")
+
+# 32-bit ASM needs to be translated from MASM to SymCryptAsm, so we don't support ASM optimizations
+# on 32-bit Linux for now
+add_compile_options("-DSYMCRYPT_IGNORE_PLATFORM")
+
+set(CMAKE_ASM_FLAGS "-x assembler-with-cpp")
diff --git a/lib/env_linuxUserMode.c b/lib/env_linuxUserMode.c
@@ -51,6 +51,13 @@ SymCryptInitEnvLinuxUsermode( UINT32 version )
 // without PLT
 void __stack_chk_fail();
 
+// On X86, __stack_chk_fail_local is used as a wrapper for __stack_chk_fail. The compiler should
+// generate it for us, but for some reason it is not doing so on gcc 9.4.0.
+void __stack_chk_fail_local()
+{
+    __stack_chk_fail();
+}
+
 _Analysis_noreturn_
 VOID
 SYMCRYPT_CALL
diff --git a/lib/sc_lib.h b/lib/sc_lib.h
@@ -1610,7 +1610,7 @@ extern const BYTE SymCryptSha512KATAnswer[64];
 #if SYMCRYPT_MS_VC
 #define SYMCRYPT_MUL32x32TO64( _a, _b )         UInt32x32To64( (_a), (_b) )
 #elif SYMCRYPT_GNUC
-#define SYMCRYPT_MUL32x32TO64( _a, _b )         ( (unsigned long)(_a)*(unsigned long)(_b) )
+#define SYMCRYPT_MUL32x32TO64( _a, _b )         ( (UINT64)(_a)*(UINT64)(_b) )
 #else
     #error Unknown compiler
 #endif
diff --git a/modules_linux/common/CMakeLists.txt b/modules_linux/common/CMakeLists.txt
@@ -8,6 +8,12 @@ add_library(symcrypt_module_linux_common STATIC ${SOURCES})
 
 set(jitter_cflags "${CMAKE_C_FLAGS} -fwrapv -fvisibility=hidden")
 set(jitter_ldflags "")
+
+if(CMAKE_SYSTEM_PROCESSOR MATCHES X86)
+  set(jitter_cflags "${jitter_cflags} -m32")
+  set(jitter_ldflags "-m32")
+endif()
+
 if (CMAKE_C_COMPILER_ID MATCHES "Clang" AND DEFINED CMAKE_C_COMPILER_TARGET)
   set(jitter_cflags "--target=${CMAKE_C_COMPILER_TARGET} --sysroot=${CMAKE_SYSROOT_COMPILE} ${jitter_cflags}")
   set(jitter_ldflags "--target=${CMAKE_C_COMPILER_TARGET}")
