RFR: 8349721: Add aarch64 intrinsics for ML-KEM [v7]

Ferenc Rakoczi duke at openjdk.org
Tue Apr 15 18:23:51 UTC 2025


On Tue, 15 Apr 2025 15:09:16 GMT, Ferenc Rakoczi <duke at openjdk.org> wrote:

>> @adinn Hi, Andrew,
>>  I think I addressed all of your comment improvement comments, in most cases I just changed them as you suggested. Thanks a lot for the thorough review!
>
>> @ferakocz
>> 
>> Hi Ferenc,
>> 
>> Sorry, but I still had a few comments to add to the KyberNTTMult routine to clarify exactly how the load, compute and store operations relate to the original Java source. That's the only remaining code that I felt needed further clarification for maintainers. So, after you work through them I can approve the PR.
> 
> No problem , it was easy to make the changes. Thanks again!

> @ferakocz I reran test jtreg:test/jdk/sun/security/provider/acvp/Launcher.java and hit a Java assertion:
> 
> ```
> >> ML-KEM-512 encapsulation
> 1 STDERR:
> java.lang.AssertionError
> 	at java.base/com.sun.crypto.provider.ML_KEM.twelve2Sixteen(ML_KEM.java:1371)
> 	at java.base/com.sun.crypto.provider.ML_KEM.decodePoly(ML_KEM.java:1408)
> 	at java.base/com.sun.crypto.provider.ML_KEM.decodeVector(ML_KEM.java:1337)
> 	at java.base/com.sun.crypto.provider.ML_KEM.kPkeEncrypt(ML_KEM.java:712)
> 	at java.base/com.sun.crypto.provider.ML_KEM.encapsulate(ML_KEM.java:555)
> 	at java.base/com.sun.crypto.provider.ML_KEM_Impls$K.implEncapsulate(ML_KEM_Impls.java:134)
> 	at java.base/sun.security.provider.NamedKEM$KeyConsumerImpl.engineEncapsulate(NamedKEM.java:124)
> 	at java.base/javax.crypto.KEM$Encapsulator.encapsulate(KEM.java:265)
> 	at java.base/javax.crypto.KEM$Encapsulator.encapsulate(KEM.java:225)
> 	at ML_KEM_Test.encapDecapTest(ML_KEM_Test.java:98)
> 	at ML_KEM_Test.run(ML_KEM_Test.java:41)
> 	at Launcher.run(Launcher.java:160)
> 	at Launcher.main(Launcher.java:122)
> 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
> 	at java.base/java.lang.reflect.Method.invoke(Method.java:565)
> 	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:335)
> 	at java.base/java.lang.Thread.run(Thread.java:1447)
> 
> JavaTest Message: Test threw exception: java.lang.AssertionError
> JavaTest Message: shutting down test
> ```
> 
> The offending code is this:
> 
> ```
> private void twelve2Sixteen(byte[] condensed, int index,
>                             short[] parsed, int parsedLength) {
>     int i = parsedLength / 64;
>     int remainder = parsedLength - i * 64;
>     if (remainder != 0) {
>         i++;
>     }
>     assert (((remainder != 0) && (remainder != 48)) || <== assert here
>         index + i * 96 > condensed.length);
> ```
> 
> I believe the logic is reversed here i.e. it should be:
> 
> ```
>     assert ((remainder == 0) || (remainder == 48)) &&
>         index + i * 96 <= condensed.length);
> ```
> 
> Does that sound right?

Aarrrrgh, yes. I forgot to negate that condition when I went from throwing an exception to assert, and I also thought, incorrectly, that -ea would enable my assertions when I tested  :-( .
Thanks a lot for catching it!

-------------

PR Comment: https://git.openjdk.org/jdk/pull/23663#issuecomment-2807096779


More information about the security-dev mailing list