[bug?] spdm_responder_test_7_measurements: assert mismatch

[twilfredo]

Hey all,

When running the tests I noticed an assert fail (dead loop), on https://github.com/DMTF/SPDM-Responder-Validator/blob/e3bc88c1d635a68de167dabceb884ba94626485b/library/spdm_responder_conformance_test_lib/spdm_responder_test_7_measurements.c#L1682

and here too (although this one doesn't fail the assertion) https://github.com/DMTF/SPDM-Responder-Validator/blob/e3bc88c1d635a68de167dabceb884ba94626485b/library/spdm_responder_conformance_test_lib/spdm_responder_test_7_measurements.c#L1449

Should this assert be instead LIBSPDM_ASSERT(spdm_test_context->test_scratch_buffer_size == sizeof(*test_buffer));

The assert works with this change, I couldn't quite figure out why we are comparing it to the version field. Is this assert even required?

Update:

Some other places where this assertion failed:

https://github.com/DMTF/SPDM-Responder-Validator/blob/e3bc88c1d635a68de167dabceb884ba94626485b/library/spdm_responder_conformance_test_lib/spdm_responder_test_8_key_exchange_rsp.c#L1042

https://github.com/DMTF/SPDM-Responder-Validator/blob/e3bc88c1d635a68de167dabceb884ba94626485b/library/spdm_responder_conformance_test_lib/spdm_responder_test_9_finish_rsp.c#L853

Thanks

[steven-bellock]

@twilfredo Are you bringing in the current libspdm main branch into this repository, or using the synced version of libspdm?

[twilfredo]

@twilfredo Are you bringing in the current libspdm main branch into this repository, or using the synced version of libspdm?

Hey @steven-bellock ,

We are using libspdm@2.3.1 release and the only commit of SPDM-Responder-Validator that I found runs the tests properly (so far), was 1dc130e52f94c2419e8bdb4918f6969b421d2eac.

[jyao1]

@twilfredo , if you are using libspdm@2.3.1, would you please try the SPDM-Responder-Validator in spdm-emu@2.3.1 ? It is a submodule of spdm-emu.

[twilfredo]

@twilfredo , if you are using libspdm@2.3.1, would you please try the SPDM-Responder-Validator in spdm-emu@2.3.1 ? It is a submodule of spdm-emu.

Thanks for the response! I just tried this, spdm-emu@2.3.1 points to SPDM-Responder-Validator@c5a3b5b9. I'm seeing the same assert failures (built in debug mode, so goes into the infinite loop). Do those asserts look correct to you?

I'm able to get all of the tests running with this patch (to SPDM-Responder-Validator) and building libspdm with LIBSPDM_RECORD_TRANSCRIPT_DATA_SUPPORT=1, but it's more of a hacky work-around:

---
 .../spdm_responder_test_7_measurements.c                        | 2 +-
 .../spdm_responder_test_8_key_exchange_rsp.c                    | 2 --
 .../spdm_responder_test_9_finish_rsp.c                          | 2 --
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/library/spdm_responder_conformance_test_lib/spdm_responder_test_7_measurements.c b/library/spdm_responder_conformance_test_lib/spdm_responder_test_7_measurements.c
index eba20fe..25a9707 100644
--- a/library/spdm_responder_conformance_test_lib/spdm_responder_test_7_measurements.c
+++ b/library/spdm_responder_conformance_test_lib/spdm_responder_test_7_measurements.c
@@ -1676,7 +1676,7 @@ void spdm_test_case_measurements_unexpected_request_in_session (void *test_conte
     spdm_context = spdm_test_context->spdm_context;
     test_buffer = (void *)spdm_test_context->test_scratch_buffer;
     LIBSPDM_ASSERT(spdm_test_context->test_scratch_buffer_size ==
-                   sizeof(test_buffer->version));
+                   sizeof(*test_buffer));

     status = libspdm_send_receive_key_exchange (spdm_context,
                                                 SPDM_KEY_EXCHANGE_REQUEST_NO_MEASUREMENT_SUMMARY_HASH,
diff --git a/library/spdm_responder_conformance_test_lib/spdm_responder_test_8_key_exchange_rsp.c b/library/spdm_responder_conformance_test_lib/spdm_responder_test_8_key_exchange_rsp.c
index 70e9ffe..3294082 100644
--- a/library/spdm_responder_conformance_test_lib/spdm_responder_test_8_key_exchange_rsp.c
+++ b/library/spdm_responder_conformance_test_lib/spdm_responder_test_8_key_exchange_rsp.c
@@ -1044,8 +1044,6 @@ void spdm_test_case_key_exchange_rsp_unexpected_request_in_session (void *test_c
     spdm_test_context = test_context;
     spdm_context = spdm_test_context->spdm_context;
     test_buffer = (void *)spdm_test_context->test_scratch_buffer;
-    LIBSPDM_ASSERT(spdm_test_context->test_scratch_buffer_size ==
-                   sizeof(test_buffer->version));

     status = libspdm_start_session (spdm_context, false,
                                     SPDM_KEY_EXCHANGE_REQUEST_NO_MEASUREMENT_SUMMARY_HASH,
diff --git a/library/spdm_responder_conformance_test_lib/spdm_responder_test_9_finish_rsp.c b/library/spdm_responder_conformance_test_lib/spdm_responder_test_9_finish_rsp.c
index 533e1aa..f7f3416 100644
--- a/library/spdm_responder_conformance_test_lib/spdm_responder_test_9_finish_rsp.c
+++ b/library/spdm_responder_conformance_test_lib/spdm_responder_test_9_finish_rsp.c
@@ -853,8 +853,6 @@ void spdm_test_case_finish_rsp_unexpected_request_in_session (void *test_context
     spdm_test_context = test_context;
     spdm_context = spdm_test_context->spdm_context;
     test_buffer = (void *)spdm_test_context->test_scratch_buffer;
-    LIBSPDM_ASSERT(spdm_test_context->test_scratch_buffer_size ==
-                   sizeof(test_buffer->version));

     status = libspdm_start_session (spdm_context, false,
                                     SPDM_KEY_EXCHANGE_REQUEST_NO_MEASUREMENT_SUMMARY_HASH,
-- 
2.39.2

[twilfredo]

ping!

[Wenxing-hou]

I am checking it. Thanks.

[twilfredo]

I am checking it. Thanks.

Great, thanks!

[Wenxing-hou]

The assert mismatch is indeed a bug. And the PR is submitted to fix it. Thanks

[twilfredo]

The assert mismatch is indeed a bug. And the PR is submitted to fix it. Thanks

great, thanks!