Skip to content

Conversation

@jenswi-linaro
Copy link
Contributor

Fixes memory leak in mpi_miller_rabin() that occurs when the function has
failed to obtain a usable random 'A' 30 turns in a row.

Signed-off-by: Jens Wiklander jens.wiklander@linaro.org

Notes:

  • Pull requests cannot be accepted until:
  • This is just a template, so feel free to use/remove the unnecessary things

Description

A few sentences describing the overall goals of the pull request's commits.

Status

READY/IN DEVELOPMENT/HOLD

Requires Backporting

When there is a bug fix, it should be backported to all maintained and supported branches.
Changes do not have to be backported if:

  • This PR is a new feature\enhancement
  • This PR contains changes in the API. If this is true, and there is a need for the fix to be backported, the fix should be handled differently in the legacy branch

Yes | NO
Which branch?

Migrations

If there is any API change, what's the incentive and logic for it.

YES | NO

Additional comments

Any additional information that could be of interest

Todos

  • Tests
  • Documentation
  • Changelog updated
  • Backported

Steps to test or reproduce

Outline the steps to test or reproduce the PR here.

Fixes memory leak in mpi_miller_rabin() that occurs when the function has
failed to obtain a usable random 'A' 30 turns in a row.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
@RonEld RonEld added bug CLA valid needs-review Every commit must be reviewed by at least two team members, component-crypto Crypto primitives and low-level interfaces labels Jan 17, 2019
@RonEld
Copy link
Contributor

RonEld commented Jan 17, 2019

@jenswi-linaro Thank you for your contribution!

Would you be willing to add an entry in the ChangeLog, crediting yourself \ Linaro for the contribution?

@RonEld
Copy link
Contributor

RonEld commented Jan 17, 2019

Note: CLA is valid, as part of the Linaro project CLA

Copy link
Contributor

@RonEld RonEld left a comment

Choose a reason for hiding this comment

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

The code change is good.
However, a ChangeLog entry is required.

I understand that you may not have time. We can add the entry on your behlaf, if you are busy.

@RonEld
Copy link
Contributor

RonEld commented Jan 17, 2019

Note: Requires backporting to mbedtls-2.7 and mbedtls-2.16

@RonEld RonEld added the needs-backports Backports are missing or are pending review and approval. label Jan 17, 2019
@jenswi-linaro
Copy link
Contributor Author

Added a change log entry. Please let me know if this is what you had in mind.

ChangeLog Outdated
previously lead to a stack overflow on constrained targets.
* Add `MBEDTLS_SELF_TEST` for the mbedtls_self_test functions
in the header files, which missed the precompilation check. #971
* Fix memory leak in in mpi_miller_rabin()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Fix memory leak in in mpi_miller_rabin()
* Fix memory leak in in mpi_miller_rabin(). Contributed by Jens Wiklander in #2363.

Unless you prefer the credit to be given to Linaro.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I added both. :-)

@RonEld
Copy link
Contributor

RonEld commented Jan 17, 2019

Thank you for adding the ChangeLog entry. I added a suggestion for a change, as we credit the contributors in the ChangeLog.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
@RonEld RonEld removed the needs-work label Jan 18, 2019
@RonEld RonEld requested a review from dgreen-arm January 18, 2019 15:44
@RonEld RonEld removed the needs-review Every commit must be reviewed by at least two team members, label Jan 22, 2019
@simonbutcher simonbutcher added the needs-ci Needs to pass CI tests label Jan 24, 2019
@RonEld
Copy link
Contributor

RonEld commented Jan 31, 2019

Backports available at #2398 and #2399

@RonEld RonEld removed the needs-backports Backports are missing or are pending review and approval. label Aug 22, 2019
@RonEld
Copy link
Contributor

RonEld commented Aug 22, 2019

Backports have been fully approved, so removed the "needs backports" label

@Patater Patater removed the needs-ci Needs to pass CI tests label Aug 22, 2019
@Patater Patater added the approved Design and code approved - may be waiting for CI or backports label Aug 22, 2019
@Patater Patater merged commit 035eaea into Mbed-TLS:development Sep 3, 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 component-crypto Crypto primitives and low-level interfaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants