Repository navigation
native: Pass chknorm bound to assembly as int64_t - #1445
Conversation
CBMC Results (ML-DSA-44, REDUCE-RAM)
Full Results (229 configurations)
|
CBMC Results (ML-DSA-65, REDUCE-RAM)
Full Results (229 configurations)
|
CBMC Results (ML-DSA-87, REDUCE-RAM)
Full Results (229 configurations)
|
CBMC Results (ML-DSA-44)
Full Results (229 configurations)
|
CBMC Results (ML-DSA-65)
Full Results (229 configurations)
|
CBMC Results (ML-DSA-87)
Full Results (229 configurations)
|
hanno-becker
left a comment
There was a problem hiding this comment.
Why don't we pass the argument in 64-bit instead, as in pq-code-package/mlkem-native#1926?
e94fb52 to
39200d2
Compare
That indeed is easier. Changed. |
hanno-becker
left a comment
There was a problem hiding this comment.
In the HOL-Light spec we have
let MLDSA_POLY_CHKNORM_SUBROUTINE_CORRECT = prove(
`!a (x:num->int32) (bound:int32) pc returnaddress.
nonoverlapping (word pc, LENGTH mldsa_poly_chknorm_mc) (a, 1024)
==> ensures arm
(\s. aligned_bytes_loaded s (word pc) mldsa_poly_chknorm_mc /\
read PC s = word pc /\
read X30 s = returnaddress /\
C_ARGUMENTS [a; word_zx bound] s /\
(!i. i < 256 ==>
read(memory :> bytes32(word_add a (word(4 * i)))) s = x i) /\
(!i. i < 256 ==> abs(ival(x i)) < &2 pow 31))
(\s. read PC s = returnaddress /\
read X0 s = word(bitval(?i. i < 256 /\ abs(ival(x i)) >= ival bound)))
(MAYCHANGE_REGS_AND_FLAGS_PERMITTED_BY_ABI)`,
There is no functional mistake here, but the use of int32 and word_zx here is still suggestive of a reliance of upper 32-bits of 32-bit arguments being cleared.
I'd suggest to switch this to int64 to have a closer alignment between the HOL and C parameters.
The AArch64 and x86_64 poly_chknorm HOL-Light specs state C_ARGUMENTS [a; word_zx bound], which constrains the full 64-bit bound register. The C prototypes declared the bound as int32_t, for which SysV and AAPCS64 leave the upper 32 bits unspecified, so the proofs did not cover calls through the prototype. Declare the bound as int64_t and require 0 <= B <= INT32_MAX in the contracts so that the prototype matches the assembly and its specification. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
The AArch64 and x86_64 poly_chknorm specifications took the bound as int32 and stated C_ARGUMENTS [a; word_zx bound], which suggests a reliance on the upper 32 bits of a 32-bit argument being cleared. Since the C prototype now passes the bound as int64_t, state the specs over an int64 bound with C_ARGUMENTS [a; bound] and require 0 <= ival bound < 2^31, matching the CBMC contract 0 <= B <= INT32_MAX. Signed-off-by: Matthias J. Kannwischer <matthias@zerorisc.com>
39200d2 to
a1c60c9
Compare
Yes that's cleaner. I've changed it. |
hanno-becker
left a comment
There was a problem hiding this comment.
Thanks @mkannwischer!
The AArch64 and x86_64 poly_chknorm HOL-Light specs state
C_ARGUMENTS [a; word_zx bound], which constrains the full 64-bit bound
register. The C prototypes declared the bound as int32_t, for which
SysV and AAPCS64 leave the upper 32 bits unspecified, so the proofs did
not cover calls through the prototype. Declare the bound as int64_t and
require 0 <= B <= INT32_MAX in the contracts so that the prototype
matches the assembly and its specification.