Merge remote-tracking branch 'origin/mbedtls-2.16' into baremetal

ChangeLog

@@ -20,6 +20,7 @@ Bugfix
      irwir.
    * Enable Suite B with subset of ECP curves. Make sure the code compiles even
      if some curves are not defined. Fixes #1591 reported by dbedev.
+   * Fix misuse of signed arithmetic in the HAVEGE module. #2598
 
 Changes
    * Make it easier to define MBEDTLS_PARAM_FAILED as assert (which config.h

library/havege.c

@@ -38,8 +38,19 @@
 #include "mbedtls/timing.h"
 #include "mbedtls/platform_util.h"
 
+#include <limits.h>
 #include <string.h>
 
+/* If int isn't capable of storing 2^32 distinct values, the code of this
+ * module may cause a processor trap or a miscalculation. If int is more
+ * than 32 bits, the code may not calculate the intended values. */
+#if INT_MIN + 1 != -0x7fffffff
+#error "The HAVEGE module requires int to be exactly 32 bits, with INT_MIN = -2^31."
+#endif
+#if UINT_MAX != 0xffffffff
+#error "The HAVEGE module requires unsigned to be exactly 32 bits."
+#endif
+
 /* ------------------------------------------------------------------------
  * On average, one iteration accesses two 8-word blocks in the havege WALK
  * table, and generates 16 words in the RES array.
@@ -54,7 +65,7 @@
  * ------------------------------------------------------------------------
  */
 
-#define SWAP(X,Y) { int *T = (X); (X) = (Y); (Y) = T; }
+#define SWAP(X,Y) { unsigned *T = (X); (X) = (Y); (Y) = T; }
 
 #define TST1_ENTER if( PTEST & 1 ) { PTEST ^= 3; PTEST >>= 1;
 #define TST2_ENTER if( PTEST & 1 ) { PTEST ^= 3; PTEST >>= 1;
@@ -77,7 +88,7 @@
     PTX = (PT1 >> 18) & 7;                              \
     PT1 &= 0x1FFF;                                      \
     PT2 &= 0x1FFF;                                      \
-    CLK = (int) mbedtls_timing_hardclock();                            \
+    CLK = (unsigned) mbedtls_timing_hardclock();        \
                                                         \
     i = 0;                                              \
     A = &WALK[PT1    ]; RES[i++] ^= *A;                 \
@@ -100,7 +111,7 @@
                                                         \
     IN = (*A >> (5)) ^ (*A << (27)) ^ CLK;              \
     *A = (*B >> (6)) ^ (*B << (26)) ^ CLK;              \
-    *B = IN; CLK = (int) mbedtls_timing_hardclock();                   \
+    *B = IN; CLK = (unsigned) mbedtls_timing_hardclock(); \
     *C = (*C >> (7)) ^ (*C << (25)) ^ CLK;              \
     *D = (*D >> (8)) ^ (*D << (24)) ^ CLK;              \
                                                         \
@@ -151,19 +162,20 @@
     PT1 ^= (PT2 ^ 0x10) & 0x10;                         \
                                                         \
     for( n++, i = 0; i < 16; i++ )                      \
-        hs->pool[n % MBEDTLS_HAVEGE_COLLECT_SIZE] ^= RES[i];
+        POOL[n % MBEDTLS_HAVEGE_COLLECT_SIZE] ^= RES[i];
 
 /*
  * Entropy gathering function
  */
 static void havege_fill( mbedtls_havege_state *hs )
 {
-    int i, n = 0;
+    unsigned i, n = 0;
-    int  U1,  U2, *A, *B, *C, *D;
+    unsigned  U1,  U2, *A, *B, *C, *D;
-    int PT1, PT2, *WALK, RES[16];
+    unsigned PT1, PT2, *WALK, *POOL, RES[16];
-    int PTX, PTY, CLK, PTEST, IN;
+    unsigned PTX, PTY, CLK, PTEST, IN;
 
-    WALK = hs->WALK;
+    WALK = (unsigned *) hs->WALK;
+    POOL = (unsigned *) hs->pool;
     PT1  = hs->PT1;
     PT2  = hs->PT2;
 

scripts/abi_check.py

@@ -59,9 +59,7 @@ class AbiChecker(object):
 
     @staticmethod
     def check_repo_path():
-        current_dir = os.path.realpath('.')
+        if not all(os.path.isdir(d) for d in ["include", "library", "tests"]):
-        root_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
-        if current_dir != root_dir:
             raise Exception("Must be run from Mbed TLS root")
 
     def _setup_logger(self):
@@ -108,6 +106,12 @@ class AbiChecker(object):
             stderr=subprocess.STDOUT
         )
         self.log.debug(worktree_output.decode("utf-8"))
+        version.commit = subprocess.check_output(
+            [self.git_command, "rev-parse", worktree_rev],
+            cwd=git_worktree_path,
+            stderr=subprocess.STDOUT
+        ).decode("ascii").rstrip()
+        self.log.debug("Commit is {}".format(version.commit))
         return git_worktree_path
 
     def _update_git_submodules(self, git_worktree_path, version):
@@ -163,6 +167,13 @@ class AbiChecker(object):
                     os.path.join(root, file)
                 )
 
+    @staticmethod
+    def _pretty_revision(version):
+        if version.revision == version.commit:
+            return version.revision
+        else:
+            return "{} ({})".format(version.revision, version.commit)
+
     def _get_abi_dumps_from_shared_libraries(self, version):
         """Generate the ABI dumps for the specified git revision.
         The shared libraries must have been built and the module paths
@@ -177,7 +188,7 @@ class AbiChecker(object):
                 "abi-dumper",
                 module_path,
                 "-o", output_path,
-                "-lver", version.revision
+                "-lver", self._pretty_revision(version),
             ]
             abi_dump_output = subprocess.check_output(
                 abi_dump_command,
@@ -222,70 +233,84 @@ class AbiChecker(object):
                 if not problems.getchildren():
                     report.remove(problems)
 
+    def _abi_compliance_command(self, mbed_module, output_path):
+        """Build the command to run to analyze the library mbed_module.
+        The report will be placed in output_path."""
+        abi_compliance_command = [
+            "abi-compliance-checker",
+            "-l", mbed_module,
+            "-old", self.old_version.abi_dumps[mbed_module],
+            "-new", self.new_version.abi_dumps[mbed_module],
+            "-strict",
+            "-report-path", output_path,
+        ]
+        if self.skip_file:
+            abi_compliance_command += ["-skip-symbols", self.skip_file,
+                                       "-skip-types", self.skip_file]
+        if self.brief:
+            abi_compliance_command += ["-report-format", "xml",
+                                       "-stdout"]
+        return abi_compliance_command
+
+    def _is_library_compatible(self, mbed_module, compatibility_report):
+        """Test if the library mbed_module has remained compatible.
+        Append a message regarding compatibility to compatibility_report."""
+        output_path = os.path.join(
+            self.report_dir, "{}-{}-{}.html".format(
+                mbed_module, self.old_version.revision,
+                self.new_version.revision
+            )
+        )
+        try:
+            subprocess.check_output(
+                self._abi_compliance_command(mbed_module, output_path),
+                stderr=subprocess.STDOUT
+            )
+        except subprocess.CalledProcessError as err:
+            if err.returncode != 1:
+                raise err
+            if self.brief:
+                self.log.info(
+                    "Compatibility issues found for {}".format(mbed_module)
+                )
+                report_root = ET.fromstring(err.output.decode("utf-8"))
+                self._remove_extra_detail_from_report(report_root)
+                self.log.info(ET.tostring(report_root).decode("utf-8"))
+            else:
+                self.can_remove_report_dir = False
+                compatibility_report.append(
+                    "Compatibility issues found for {}, "
+                    "for details see {}".format(mbed_module, output_path)
+                )
+            return False
+        compatibility_report.append(
+            "No compatibility issues for {}".format(mbed_module)
+        )
+        if not (self.keep_all_reports or self.brief):
+            os.remove(output_path)
+        return True
+
     def get_abi_compatibility_report(self):
         """Generate a report of the differences between the reference ABI
         and the new ABI. ABI dumps from self.old_version and self.new_version
         must be available."""
-        compatibility_report = ""
+        compatibility_report = ["Checking evolution from {} to {}".format(
+            self._pretty_revision(self.old_version),
+            self._pretty_revision(self.new_version)
+        )]
         compliance_return_code = 0
         shared_modules = list(set(self.old_version.modules.keys()) &
                               set(self.new_version.modules.keys()))
         for mbed_module in shared_modules:
-            output_path = os.path.join(
+            if not self._is_library_compatible(mbed_module,
-                self.report_dir, "{}-{}-{}.html".format(
+                                               compatibility_report):
-                    mbed_module, self.old_version.revision,
+                compliance_return_code = 1
-                    self.new_version.revision
-                )
-            )
-            abi_compliance_command = [
-                "abi-compliance-checker",
-                "-l", mbed_module,
-                "-old", self.old_version.abi_dumps[mbed_module],
-                "-new", self.new_version.abi_dumps[mbed_module],
-                "-strict",
-                "-report-path", output_path,
-            ]
-            if self.skip_file:
-                abi_compliance_command += ["-skip-symbols", self.skip_file,
-                                           "-skip-types", self.skip_file]
-            if self.brief:
-                abi_compliance_command += ["-report-format", "xml",
-                                           "-stdout"]
-            try:
-                subprocess.check_output(
-                    abi_compliance_command,
-                    stderr=subprocess.STDOUT
-                )
-            except subprocess.CalledProcessError as err:
-                if err.returncode == 1:
-                    compliance_return_code = 1
-                    if self.brief:
-                        self.log.info(
-                            "Compatibility issues found for {}".format(mbed_module)
-                        )
-                        report_root = ET.fromstring(err.output.decode("utf-8"))
-                        self._remove_extra_detail_from_report(report_root)
-                        self.log.info(ET.tostring(report_root).decode("utf-8"))
-                    else:
-                        self.can_remove_report_dir = False
-                        compatibility_report += (
-                            "Compatibility issues found for {}, "
-                            "for details see {}\n".format(mbed_module, output_path)
-                        )
-                else:
-                    raise err
-            else:
-                compatibility_report += (
-                    "No compatibility issues for {}\n".format(mbed_module)
-                )
-                if not (self.keep_all_reports or self.brief):
-                    os.remove(output_path)
         for version in [self.old_version, self.new_version]:
             for mbed_module, mbed_module_dump in version.abi_dumps.items():
                 os.remove(mbed_module_dump)
         if self.can_remove_report_dir:
             os.rmdir(self.report_dir)
-        self.log.info(compatibility_report)
+        self.log.info("\n".join(compatibility_report))
         return compliance_return_code
 
     def check_for_abi_changes(self):
@@ -357,7 +382,9 @@ def run_main():
         )
         parser.add_argument(
             "-s", "--skip-file", type=str,
-            help="path to file containing symbols and types to skip"
+            help=("path to file containing symbols and types to skip "
+                  "(typically \"-s identifiers\" after running "
+                  "\"tests/scripts/list-identifiers.sh --internal\")")
         )
         parser.add_argument(
             "-b", "--brief", action="store_true",
@@ -371,6 +398,7 @@ def run_main():
             version="old",
             repository=abi_args.old_repo,
             revision=abi_args.old_rev,
+            commit=None,
             crypto_repository=abi_args.old_crypto_repo,
             crypto_revision=abi_args.old_crypto_rev,
             abi_dumps={},
@@ -380,6 +408,7 @@ def run_main():
             version="new",
             repository=abi_args.new_repo,
             revision=abi_args.new_rev,
+            commit=None,
             crypto_repository=abi_args.new_crypto_repo,
             crypto_revision=abi_args.new_crypto_rev,
             abi_dumps={},

tests/scripts/check-files.py

@@ -1,14 +1,12 @@
 #!/usr/bin/env python3
+
+# This file is part of Mbed TLS (https://tls.mbed.org)
+# Copyright (c) 2018, Arm Limited, All Rights Reserved
+
 """
-This file is part of Mbed TLS (https://tls.mbed.org)
-
-Copyright (c) 2018, Arm Limited, All Rights Reserved
-
-Purpose
-
 This script checks the current state of the source code for minor issues,
 including incorrect file permissions, presence of tabs, non-Unix line endings,
-trailing whitespace, presence of UTF-8 BOM, and TODO comments.
+trailing whitespace, and presence of UTF-8 BOM.
 Note: requires python 3, must be run from Mbed TLS root.
 """
 
@@ -170,19 +168,6 @@ class MergeArtifactIssueTracker(LineIssueTracker):
             return True
         return False
 
-class TodoIssueTracker(LineIssueTracker):
-    """Track lines containing ``TODO``."""
-
-    heading = "TODO present:"
-    files_exemptions = frozenset([
-        os.path.basename(__file__),
-        "benchmark.c",
-        "pull_request_template.md",
-    ])
-
-    def issue_with_line(self, line, _filepath):
-        return b"todo" in line.lower()
-
 
 class IntegrityChecker(object):
     """Sanity-check files under the current directory."""
@@ -211,7 +196,6 @@ class IntegrityChecker(object):
             TrailingWhitespaceIssueTracker(),
             TabIssueTracker(),
             MergeArtifactIssueTracker(),
-            TodoIssueTracker(),
         ]
 
     @staticmethod
@@ -257,15 +241,7 @@ class IntegrityChecker(object):
 
 
 def run_main():
-    parser = argparse.ArgumentParser(
+    parser = argparse.ArgumentParser(description=__doc__)
-        description=(
-            "This script checks the current state of the source code for "
-            "minor issues, including incorrect file permissions, "
-            "presence of tabs, non-Unix line endings, trailing whitespace, "
-            "presence of UTF-8 BOM, and TODO comments. "
-            "Note: requires python 3, must be run from Mbed TLS root."
-        )
-    )
     parser.add_argument(
         "-l", "--log_file", type=str, help="path to optional output log",
     )