-
Notifications
You must be signed in to change notification settings - Fork 34
CBMC: Prove mld_polymat_permute_bitrev_to_custom on top of native API #820
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
Conversation
3246468 to
ae1ed8d
Compare
mkannwischer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mld_polymat_permute_bitrev_to_custom needs to call the mld_polyvecl_permute_bitrev_to_custom function by contract, otherwise you are introducing a proof gap that the mld_polymat_permute_bitrev_to_custom isn't actually proven to be safe on top of the native backend.
The same problem exists in mlkem-native and should have been reported there rather than copied to mldsa-native. I opened a PR to fix that: pq-code-package/mlkem-native#1445.
ae1ed8d to
6fef01b
Compare
mkannwischer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes look good to me now.
Please clean up the commit message:
(1) The add changes seems to be an artifact of a squashed commit. You may want to use fixup rather than squash in such cases.
(2) From your current commit message no one can understand what you are trying to do. What you are doing is proving mld_polymat_permute_bitrev_to_custom on top of the native API. It should become clear why the refactoring is needed (pointing to the CBMC issue). It's often more important to describe why you are doing something in addition to what you are doing.
(3) The commit message is full of grammatical mistakes making it hard to follow. Please be more careful and consider spell-checking your commit messages.
b959870 to
55a9791
Compare
Hello, Matthias, Really appreciate your guidance and patience in reviewing this. Thank you very much. |
mkannwischer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @willieyz! LGTM.
Refactor the polynomial matrix permutation to enable CBMC verification when using native backend implementations. The current implementation causes CBMC proof timeouts due to nested loops over two-dimensional polynomial matrices, similar to the issue described in (diffblue/cbmc#8796). This refactoring decomposes the matrix-level operation into vector-level components with explicit contracts. As a result, CBMC can verify each component independently, avoiding the nested-loop state explosion while preserving safety verification for the native backend. Signed-off-by: willieyz <[email protected]>
55a9791 to
52df632
Compare
…ry_c #820 introduced a massive performance regression to the CBMC proofs caused by the polyvecl_pointwise_acc_montgomery_c. This commit increases the OBJECT_BITS for that proof fixing the problem. Signed-off-by: Matthias J. Kannwischer <[email protected]>
…ry_c #820 introduced a massive performance regression to the CBMC proofs caused by the polyvecl_pointwise_acc_montgomery_c. This commit increases the OBJECT_BITS for that proof fixing the problem. Signed-off-by: Matthias J. Kannwischer <[email protected]>
#820 introduced a massive performance regression to the CBMC proofs caused by the polyvecl_pointwise_acc_montgomery_c. This commit increases the OBJECT_BITS for that proof fixing the problem. Signed-off-by: Matthias J. Kannwischer <[email protected]>
#820 introduced a massive performance regression to the CBMC proofs caused by the polyvecl_pointwise_acc_montgomery_c. This commit increases the OBJECT_BITS for that proof fixing the problem. Signed-off-by: Matthias J. Kannwischer <[email protected]>
Resolve: CBMC: Add proofs atop of native functions #350
Related: Refactor mld_polymat_permute_bitrev_to_custom #770
This PR refactor the polynomial matrix permutation logic by extracting a reusable mld_polyvecl_permute_bitrev_to_custom function and adds comprehensive CBMC verification for both C and native implementations.
This is a temporary workaround until diffblue/cbmc#8796 is resolved.
This PR made follwoing changes:
Testing
make clean & MLD_CONFIG_PARAMETER_SET=44/65/87 make resultchecking the contract itself.tests cbmc -p "parrent_proof", this will check the parrent proof