From f83eb82a4db16d49119b119c153f6fe9e005000f Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Mon, 30 Mar 2020 20:11:39 +0200
Subject: [PATCH] Don't restore *config.h before backing it up

Back up the config files at the beginning of all.sh, rather than before each
component. In particular, create the backup before running cleanup for the
first time. This fixes #3139 (all.sh using a config.h.bak from a previous
job), and makes all.sh more robust against accidentally using a modified
config.h midway through because a component messed with the backup.

Use a different extension (*.all.bak rather than *.bak) for the backups.
This is necessary to ensure that auxiliary scripts such as depends*.pl that
make their own backup don't remove all.sh's backup, which the code from this
commit does not support.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
---
 tests/scripts/all.sh | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index 8c75c9f052..5c2ab2778d 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -129,9 +129,14 @@ pre_check_environment () {
 
 pre_initialize_variables () {
     CONFIG_H='include/mbedtls/mbedtls_config.h'
-    CONFIG_BAK="$CONFIG_H.bak"
     CRYPTO_CONFIG_H='include/psa/crypto_config.h'
-    CRYPTO_CONFIG_BAK="$CRYPTO_CONFIG_H.bak"
+
+    # Files that are clobbered by some jobs will be backed up. Use a different
+    # suffix from auxiliary scripts so that all.sh and auxiliary scripts can
+    # independently decide when to remove the backup file.
+    backup_suffix='.all.bak'
+    # Files clobbered by config.py
+    files_to_back_up="$CONFIG_H $CRYPTO_CONFIG_H"
 
     append_outcome=0
     MEMORY=0
@@ -295,13 +300,18 @@ cleanup()
     rm -f programs/test/cmake_package_install/Makefile
     rm -f programs/test/cmake_package_install/cmake_package_install
 
-    if [ -f "$CONFIG_BAK" ]; then
-        mv "$CONFIG_BAK" "$CONFIG_H"
-    fi
+    # Restore files that may have been clobbered by the job
+    for x in $files_to_back_up; do
+        cp -p "$x$backup_suffix" "$x"
+    done
+}
 
-    if [ -f "$CRYPTO_CONFIG_BAK" ]; then
-        mv "$CRYPTO_CONFIG_BAK" "$CRYPTO_CONFIG_H"
-    fi
+final_cleanup () {
+    cleanup
+
+    for x in $files_to_back_up; do
+        rm -f "$x$backup_suffix"
+    done
 }
 
 # Executed on exit. May be redefined depending on command line options.
@@ -310,7 +320,7 @@ final_report () {
 }
 
 fatal_signal () {
-    cleanup
+    final_cleanup
     final_report $1
     trap - $1
     kill -$1 $$
@@ -485,6 +495,12 @@ pre_check_git () {
     fi
 }
 
+pre_back_up () {
+    for x in $files_to_back_up; do
+        cp -p "$x" "$x$backup_suffix"
+    done
+}
+
 pre_setup_keep_going () {
     failure_count=0 # Number of failed components
     last_failure_status=0 # Last failure status in this component
@@ -2666,7 +2682,7 @@ component_check_generate_test_code () {
 
 post_report () {
     msg "Done, cleaning up"
-    cleanup
+    final_cleanup
 
     final_report
 }
@@ -2692,10 +2708,6 @@ pseudo_component_error_test () {
 
 # Run one component and clean up afterwards.
 run_component () {
-    # Back up the configuration in case the component modifies it.
-    # The cleanup function will restore it.
-    cp -p "$CONFIG_H" "$CONFIG_BAK"
-    cp -p "$CRYPTO_CONFIG_H" "$CRYPTO_CONFIG_BAK"
     current_component="$1"
     export MBEDTLS_TEST_CONFIGURATION="$current_component"
 
@@ -2750,6 +2762,7 @@ pre_initialize_variables
 pre_parse_command_line "$@"
 
 pre_check_git
+pre_back_up
 
 build_status=0
 if [ $KEEP_GOING -eq 1 ]; then