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

Fix mpi_bigendian_to_host() on bigendian systems that don't have BYTE_ORDER defined #2623

Merged

Conversation

hanno-becker
Copy link

The previous implementation of mpi_uint_bigendian_to_host_c() did a byte-swapping regardless of the endianness of the system. Fixes #2622 found by @mrpippy.

Still needs a ChangeLog entry.

The previous implementation of mpi_bigendian_to_host() did
a byte-swapping regardless of the endianness of the system.

Fixes Mbed-TLS#2622.
@hanno-becker hanno-becker added bug mbed TLS team needs-work needs-review Every commit must be reviewed by at least two team members, component-crypto Crypto primitives and low-level interfaces labels May 1, 2019
@hanno-becker
Copy link
Author

hanno-becker commented May 1, 2019

@mrpippy How would you like to be credited in the ChangeLog?

@mrpippy
Copy link
Contributor

mrpippy commented May 1, 2019

"Brendan Shanks" is fine, thanks!

@hanno-becker hanno-becker requested review from RonEld and mpg May 2, 2019 08:34
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@mpg
Copy link
Contributor

mpg commented May 2, 2019

Question: do we already have backlog item to add a big-endian architecture to our CI? cc @Patater

@Patater
Copy link
Contributor

Patater commented May 2, 2019

@mrpippy Could you please confirm this patch works for you? Thanks.

@Patater
Copy link
Contributor

Patater commented May 2, 2019

@mpg #2627

@hanno-becker hanno-becker requested review from jarlamsa and removed request for RonEld May 3, 2019 12:39
@mrpippy
Copy link
Contributor

mrpippy commented May 3, 2019

Looks good, all tests pass on big-endian PowerPC

Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

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

LGTM

@Patater Patater added the needs-backports Backports are missing or are pending review and approval. label May 3, 2019
@Patater
Copy link
Contributor

Patater commented May 3, 2019

I believe this needs a Mbed TLS 2.16 backport, as the implementation of mpi_uint_bigendian_to_host_c() that doesn't work on big endian is also present in that branch.

@Patater Patater removed the needs-review Every commit must be reviewed by at least two team members, label May 3, 2019
@hanno-becker hanno-becker removed the request for review from jarlamsa May 15, 2019 12:08
@hanno-becker
Copy link
Author

Backport available here: #2645.

@Patater Patater added approved Design and code approved - may be waiting for CI or backports and removed needs-backports Backports are missing or are pending review and approval. labels Sep 5, 2019
@Patater Patater merged commit 5f9aa2b into Mbed-TLS:development Sep 5, 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.

PKCS and X.509 selftests fail on big-endian PowerPC since 2.16.1
4 participants