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

Dead code in mbedtls_mpi_exp_mod() #2309

Closed
pkolbus opened this issue Dec 20, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@pkolbus
Copy link
Contributor

commented Dec 20, 2018

Description

  • Type: Bug/Question
  • Priority: Minor

Bug

OS
FreeRTOS

mbed TLS build:
Version: 2.14.1
OS version: 9.0
Configuration: MBEDTLS_MPI_WINDOW_SIZE = 6 (default)
Compiler and options (if you used a pre-built binary, please indicate how you obtained it): arm-none-eabi-gcc 6 2017-q2-update
Additional environment information: Static analyzer (Coverity)

Actual behavior
In mbedtls_mpi_exp_mod(), the comparison of wsize is always false. Coverity reports a potential issue. (line 1673, as of commit b9eb786).

Admittedly, the compiler should be able to optimize this out...

Question

Would it be appropriate to add a preprocessor guard #if (MBEDTLS_MPI_WINDOW_SIZE < 6) here? Or should the Coverity finding be treated as an intentional practice?

If a preprocessor guard is appropriate, I'm happy to create a PR.

@RonEld

This comment has been minimized.

Copy link
Contributor

commented Dec 24, 2018

@pkolbus Thank you for raising this issue!
I agree that when MBEDTLS_MPI_WINDOW_SIZE is 6, this comparison is always false, and coverity might warn on this, therefore marking this as a bug.
You are welcome to raise a PR, assuming you have accepted our CLA.
Thanks again for your interest in Mbed TLS!

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Dec 24, 2018

ARM Internal Ref: IOTSSL-2681

@ciarmcom ciarmcom added the mirrored label Dec 24, 2018

@pkolbus pkolbus referenced this issue Dec 27, 2018

Merged

Fix dead code in mbedtls_mpi_exp_mod() #2317

2 of 4 tasks complete

@Patater Patater closed this in #2317 Mar 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.