xtest regression case 4017 #741
Conversation
There was a problem hiding this comment.
For "ta: crypt: fix authenc entries" and "xtest: return out-buffer sizes unconditionally":
Reviewed-by: Jerome Forissier <[email protected]>
There was a problem hiding this comment.
Reviewed-by: Etienne Carriere <[email protected]> for commits
"ta: crypt: fix authenc entries" and
"xtest: return out-buffer sizes unconditionally".
|
This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note that you can always re-open a closed pull request at any time. |
There was a problem hiding this comment.
For "xtest: regression: add case 4017", with my comments addressed:
Acked-by: Jerome Forissier <[email protected]>
| goto out; | ||
| } | ||
|
|
||
| for (n = 0; n < bs.text_size; n++) { |
There was a problem hiding this comment.
- A comment explaining the influence of level would be helpful here. What is tested when level is < 14, 14, 15, > 15 ?
- Could you shift the values down by 2? Reason being, we have
RFC6070_TEST(15, 4)at level 15 which is quite slow (several minutes on QEMUv8 on my machine) so it would be nice to be able to run the "moderately slow" AES tests without running RFC6070_TEST(15, 4).
jenswi-linaro
force-pushed
the
tee_buffer_4017
branch
from
c9860c5 to
ccd1288
Compare
|
@jforissier's comment addressed and tags applied. |
|
@etienne-lms any comment on "xtest: regression: add case 4017"? |
| for (n = 0; n < middle_count && n + initial_count < bs->text_size; | ||
| n += middle_increment) | ||
| if (!ADBG_EXPECT_TRUE(c, process_bytes_4017(c, bs, 1))) | ||
| return false; |
There was a problem hiding this comment.
this looks strange to me. I think it works because middle_increment is always 1, as stated in the inline comment at line 6561:
* If level < 12 test with middle_count of 32 else 0.
* (middle_count - n) bytes are processed one by one.
Unless you plan some day to change this middle_increment in the future, I think it would more better to remove middle_increment and use 1 straight. Otherwise, I suggest to change line 6396:
- if (!ADBG_EXPECT_TRUE(c, process_bytes_4017(c, bs, 1)))
+ if (!ADBG_EXPECT_TRUE(c, process_bytes_4017(c, bs, middle_increment)))and line 6562:
* If level < 12 test with middle_count of 32 else 0.
- * (middle_count - n) bytes are processed one by one.
+ * (middle_count - n) bytes are processed by middle_increment
+ * bytes bursts.If I understood that correctly of course :)
There was a problem hiding this comment.
I'll drop the middle_increment.
host/xtest/regression_4000.c
Outdated
| goto out; | ||
| Do_ADBG_EndSubCase(c, "TEE_ALG_AES_CCM 1 extra byte"); | ||
|
|
||
| out: |
There was a problem hiding this comment.
misses an call to Do_ADBG_EndSubCase() when we reach this point before line 6682 is reached.
Actually, I wonder if the goto out are really useful. If removed, we'll get a status of all the cases, maybe flowed of error trace messages if many test cases fail but i think its worth it, unless the session crashed soemwhere during the test cases.
There was a problem hiding this comment.
I'll remove the gotos.
|
Updated |
There was a problem hiding this comment.
Acked-by: Etienne Carriere <[email protected]>
for commit "xtest: regression: add case 4017"
jenswi-linaro
force-pushed
the
tee_buffer_4017
branch
from
85f2c53 to
cc01c38
Compare
|
Tag applied |
Fix the AE cipher entry functions:
ta_entry_ae_update() - return TEE_AEUpdate() instead of TEE_SUCCESS
ta_entry_ae_{en,de}crypt_final() - handle NULL buffers
Signed-off-by: Jens Wiklander <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]>
In ta_crypt_cmd_cipher_update(), ta_crypt_cmd_cipher_do_final(), ta_crypt_cmd_ae_update(), ta_crypt_cmd_ae_encrypt_final(), and ta_crypt_cmd_ae_decrypt_final(), always return out-buffer sizes even if the function doesn't return TEEC_SUCCESS. Signed-off-by: Jens Wiklander <[email protected]> Reviewed-by: Etienne Carriere <[email protected]>
Add case 4017 "Test TEE Internal API Cipher block buffering". This test tries to test the corner cases for Cipher and AE buffering. Signed-off-by: Jens Wiklander <[email protected]> Acked-by: Jerome Forissier <[email protected]> Acked-by: Etienne Carriere <[email protected]>
jenswi-linaro
force-pushed
the
tee_buffer_4017
branch
from
cc01c38 to
fdfb312
Compare
|
Rebased to fix CI issue. |
Depends on OP-TEE/optee_os#6802