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

Aarch64 feature detection #83

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

thomwiggers
Copy link
Member

Should fix #82

@icota
Copy link

icota commented Mar 16, 2025

I can confirm that

[patch.crates-io]
pqcrypto-traits = { git = "https://github.com/rustpq/pqcrypto", branch = "aarch64-feature-detection" }
pqcrypto-mldsa = { git = "https://github.com/rustpq/pqcrypto", branch = "aarch64-feature-detection" }
pqcrypto-mlkem = { git = "https://github.com/rustpq/pqcrypto", branch = "aarch64-feature-detection" }

fixes Fatal signal 4 (SIGILL), code 1 (ILL_ILLOPC) on Android Pixel 7 for me.

@icota
Copy link

icota commented Mar 16, 2025

On second glance it only fixes ml-dsa. If I try to get an ml-kem keypair I still encounter:

signal 4 (SIGILL), code 1 (ILL_ILLOPC), fault addr 0x000000744d312fb4
    x0  0000007451950550  x1  000000744cdd9828  x2  00000074519503d0  x3  0000000000000018
    x4  00000074519503d0  x5  000000000000001f  x6  0000000000000010  x7  7f7f7f7f7f7f7f7f
    x8  0000000000000003  x9  0000000000000014  x10 000000000000001f  x11 f41107a453e2076c
    x12 0000000000021003  x13 000000007fffffff  x14 0000000000000000  x15 0000027433a5a687
    x16 000000744d5e4480  x17 0000007830d67940  x18 0000007443da0000  x19 b4000076f9369c70
    x20 b400007599339900  x21 000000744d5af710  x22 0000000000005232  x23 00000000000051dc
    x24 00000074519606c0  x25 00000074519606c0  x26 0000007451960a28  x27 0000007451960a80
    x28 0000000000206000  x29 0000007451950160
    lr  000000744d312f6c  sp  0000007451950110  pc  000000744d312fb4  pst 0000000020001000

@icota
Copy link

icota commented Mar 16, 2025

I can fix the above by having ml-dsa check for sha3 instead of merely neon. But I'm not sure if that's a proper fix.
I'll make a PR to this branch regardless.

Copy link

@icota icota left a comment

Choose a reason for hiding this comment

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

#84

@thomwiggers thomwiggers force-pushed the aarch64-feature-detection branch 2 times, most recently from 4c07016 to bb67ba9 Compare March 17, 2025 04:13
Enable AArch64 feature detection

Bump versions

Update changelog

Include ml-kem, bump versions

Falcon should not be bumped

Should be a semantic bump
@thomwiggers thomwiggers force-pushed the aarch64-feature-detection branch from bb67ba9 to 097a7b4 Compare March 17, 2025 04:26
@thomwiggers
Copy link
Member Author

Would be great if you could test this again

@icota
Copy link

icota commented Mar 17, 2025

@thomwiggers it crashes on 097a7b4

@thomwiggers
Copy link
Member Author

Can you elaborate? There are no differences with #84 in how the code is called (though I'm suspicious that the cfg guards in #84 are broken which leads to the code not getting included).

@hfunke
Copy link

hfunke commented Mar 17, 2025

From my perspective your fix works: ML-DSA runs on a RasPi now. Thanks a lot for your support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runtime feature detection fails for ML-DSA on Aarch64
3 participants