diff --git a/ChangeLog b/ChangeLog index 3e85f3f66..0c579a8b2 100644 --- a/ChangeLog +++ b/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 diff --git a/library/havege.c b/library/havege.c index 54f897c6e..c139e1db0 100644 --- a/library/havege.c +++ b/library/havege.c @@ -38,8 +38,19 @@ #include "mbedtls/timing.h" #include "mbedtls/platform_util.h" +#include #include +/* 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; - int U1, U2, *A, *B, *C, *D; - int PT1, PT2, *WALK, RES[16]; - int PTX, PTY, CLK, PTEST, IN; + unsigned i, n = 0; + unsigned U1, U2, *A, *B, *C, *D; + unsigned PT1, PT2, *WALK, *POOL, RES[16]; + unsigned PTX, PTY, CLK, PTEST, IN; - WALK = hs->WALK; + WALK = (unsigned *) hs->WALK; + POOL = (unsigned *) hs->pool; PT1 = hs->PT1; PT2 = hs->PT2; diff --git a/scripts/abi_check.py b/scripts/abi_check.py index 502c7ae02..533aaea3d 100755 --- a/scripts/abi_check.py +++ b/scripts/abi_check.py @@ -59,9 +59,7 @@ class AbiChecker(object): @staticmethod def check_repo_path(): - current_dir = os.path.realpath('.') - root_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) - if current_dir != root_dir: + if not all(os.path.isdir(d) for d in ["include", "library", "tests"]): 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( - self.report_dir, "{}-{}-{}.html".format( - mbed_module, self.old_version.revision, - 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) + if not self._is_library_compatible(mbed_module, + compatibility_report): + compliance_return_code = 1 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={}, diff --git a/tests/scripts/check-files.py b/tests/scripts/check-files.py index feea36c34..1c2e6ea29 100755 --- a/tests/scripts/check-files.py +++ b/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( - 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 = argparse.ArgumentParser(description=__doc__) parser.add_argument( "-l", "--log_file", type=str, help="path to optional output log", )