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 aarch64 signed bit shift issue found by UBSAN #18816

Conversation

tom-cosgrove-arm
Copy link
Contributor

Also fix conditional branch out of range when using sanitisers.

Fixes #18813

Also fix conditional branch out of range when using sanitisers.

Fixes openssl#18813

Signed-off-by: Tom Cosgrove <tom.cosgrove@arm.com>

Change-Id: Ic543885091ed3ef2ddcbe21de0a4ac0bca1e2494
@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug branch: 3.0 Merge to openssl-3.0 branch labels Jul 18, 2022
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Jul 18, 2022
@mattcaswell mattcaswell added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Jul 18, 2022
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Jul 19, 2022
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Jul 19, 2022
openssl-machine pushed a commit that referenced this pull request Jul 19, 2022
Also fix conditional branch out of range when using sanitisers.

Fixes #18813

Signed-off-by: Tom Cosgrove <tom.cosgrove@arm.com>

Change-Id: Ic543885091ed3ef2ddcbe21de0a4ac0bca1e2494

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #18816)
@t8m
Copy link
Member

t8m commented Jul 19, 2022

Merged to master branch. And the header fix cherry-picked to 3.0 branch too. The crypto/aes/asm/bsaes-armv8.pl is not present in 3.0 so that part of the patch was skipped.

@t8m t8m closed this Jul 19, 2022
openssl-machine pushed a commit that referenced this pull request Jul 19, 2022
Fixes #18813

Signed-off-by: Tom Cosgrove <tom.cosgrove@arm.com>

Change-Id: Ic543885091ed3ef2ddcbe21de0a4ac0bca1e2494

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #18816)

(cherry picked from commit 1efd853)
@tom-cosgrove-arm tom-cosgrove-arm deleted the fix-linux-aarch64-ubsan-and-asan-builds branch July 19, 2022 12:16
@tom-cosgrove-arm
Copy link
Contributor Author

@t8m @mattcaswell @paulidale In the discussion about the problems with OpenSSL v3.0.3 on Tandem NonStop (#18232) , Matt Caswell asked about them adding a buildbot worker to the OpenSSL project CI.

Is there something we can do like this to add native (rather than cross-compiled) linux-aarch64 builds (and therefore also test runs) into the project CI? We have a suitable system identified internally, and while our internal CI tracks OpenSSL master, it would be better if something were part of the project CI to be used on PRs etc (catching potential issues sooner is always better).

What's the best way to progress this?

sftcd pushed a commit to sftcd/openssl that referenced this pull request Sep 24, 2022
Also fix conditional branch out of range when using sanitisers.

Fixes openssl#18813

Signed-off-by: Tom Cosgrove <tom.cosgrove@arm.com>

Change-Id: Ic543885091ed3ef2ddcbe21de0a4ac0bca1e2494

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#18816)
OttoHollmann added a commit to OttoHollmann/openssl that referenced this pull request Mar 10, 2023
Also fix conditional branch out of range when using sanitisers.

Fixes openssl#18813

Signed-off-by: Tom Cosgrove <tom.cosgrove@arm.com>

Change-Id: Ic543885091ed3ef2ddcbe21de0a4ac0bca1e2494

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#18816)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch severity: fips change The pull request changes FIPS provider sources triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git Hub Actions Build/Test Failure: non-caching, aarch64
5 participants