Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport 2.16: Fix dead code in mbedtls_mpi_exp_mod() #2388

Merged
merged 3 commits into from
Mar 6, 2019

Conversation

k-stachowiak
Copy link
Contributor

This is a backport of #2317

AndrzejKurek
AndrzejKurek previously approved these changes Jan 30, 2019
Copy link
Contributor

@AndrzejKurek AndrzejKurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a faithful backport.

@RonEld RonEld added bug CLA valid needs-review Every commit must be reviewed by at least two team members, labels Jan 30, 2019
RonEld
RonEld previously approved these changes Jan 30, 2019
@RonEld RonEld removed the needs-review Every commit must be reviewed by at least two team members, label Jan 30, 2019
@Patater Patater added the approved Design and code approved - may be waiting for CI or backports label Jan 31, 2019
@Patater
Copy link
Contributor

Patater commented Feb 4, 2019

Please rebase to update for all.sh changes.

In mbedtls_mpi_exp_mod(), the limit check on wsize is never true when
MBEDTLS_MPI_WINDOW_SIZE is at least 6. Wrap in a preprocessor guard
to remove the dead code and resolve a Coverity finding from the
DEADCODE checker.

Change-Id: Ice7739031a9e8249283a04de11150565b613ae89
There were no tests for a non-default MPI window size. Add one.

Change-Id: Ic08fbc9161d0b3ee67eb3c91f9baf602646c9dfe
@k-stachowiak
Copy link
Contributor Author

@Patater I have rebased the PR.
@RonEld, @AndrzejKurek could you please re-review?

RonEld
RonEld previously approved these changes Feb 5, 2019
ChangeLog Outdated
@@ -31,6 +31,10 @@ Changes
Inserted as an enhancement for #1371
* Add support for alternative CSR headers, as used by Microsoft and defined
in RFC 7468. Found by Michael Ernst. Fixes #767.
* Remove dead code from bignum.c in the default configuration.
Found by Coverity, reported and fixed by Peter Kolbus. Fixes #2309.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please credit my employer as well: "Peter Kolbus (Garmin)"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks for a quick response!

@k-stachowiak k-stachowiak force-pushed the mpi-deadcode-2.16 branch 2 times, most recently from 0d48080 to b17ac5b Compare February 11, 2019 08:42
@k-stachowiak
Copy link
Contributor Author

@AndrzejKurek @RonEld Could you please review the change log update?

@pkolbus
Copy link
Contributor

pkolbus commented Feb 11, 2019

Thanks, LGTM.

@Patater Patater merged commit dc5893d into Mbed-TLS:mbedtls-2.16 Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants