Fix : vendor_response_get_id should not exist

ShitalJumbad · ShitalJumbad · commit b00d05583599 · 2025-11-03T08:44:57.000-07:00
Signed-off-by: Shital Jumbad &lt;sjumbad@nvidia.com&gt;
diff --git a/doc/user_guide.md b/doc/user_guide.md
@@ -453,42 +453,33 @@ Refer to spdm_server_init() in [spdm_responder.c](https://github.com/DMTF/spdm-e
 
     libspdm_register_get_response_func (spdm_context, libspdm_get_response);
     ```
-    3.2 This callbacks handle SPDM Vendor Defined Commands
-    ```C
-    libspdm_return_t libspdm_vendor_get_id_func(
-      void *spdm_context,
-      uint16_t *resp_standard_id,
-      uint8_t *resp_vendor_id_len,
-      void *resp_vendor_id)
-    {
-      // return responder vendor id
-      ...
-
-      return LIBSPDM_STATUS_SUCCESS;
-    }
-
-    vendor_response_get_id
-    libspdm_return_t libspdm_vendor_response_func(
-      void *spdm_context,
-      uint16_t req_standard_id,
-      uint8_t req_vendor_id_len,
-      const void *req_vendor_id,
-      uint16_t req_size,
-      const void *req_data,
-      uint16_t *resp_size,
-      void *resp_data)
-      {
-      // process request and create response
-      ...
-      // populate response header and payload
-      ...
-
-      return LIBSPDM_STATUS_SUCCESS;
-    }
+   3.2 This callback handles SPDM Vendor Defined Commands
+   ```C
+   libspdm_return_t libspdm_vendor_response_func(
+     void *spdm_context,
+     const uint32_t *session_id,
+     uint16_t req_standard_id,
+     uint8_t req_vendor_id_len,
+     const void *req_vendor_id,
+     uint32_t req_size,
+     const void *req_data,
+     uint16_t *resp_standard_id,
+     uint8_t *resp_vendor_id_len,
+     void *resp_vendor_id,
+     uint32_t *resp_size,
+     void *resp_data)
+   {
+     // set response Vendor/Standard IDs
+     *resp_standard_id = /* your standard id */;
+     *resp_vendor_id_len = /* your vendor id length */;
+     // write Vendor ID bytes into resp_vendor_id[0..*resp_vendor_id_len-1]
+
+     // write payload to resp_data and set *resp_size to payload size
+     return LIBSPDM_STATUS_SUCCESS;
+   }
 
-    libspdm_register_vendor_get_id_callback_func(spdm_context, libspdm_vendor_get_id_func);
-    libspdm_register_vendor_callback_func(spdm_context, libspdm_vendor_response_func);
-    ```
+   libspdm_register_vendor_callback_func(spdm_context, libspdm_vendor_response_func);
+   ```
 
 4. Free the memory of contexts within the SPDM context when all flow is over.
    This function does not free the SPDM context itself.
diff --git a/include/internal/libspdm_common_lib.h b/include/internal/libspdm_common_lib.h
@@ -676,7 +676,6 @@ typedef struct {
 
 #if LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES
     libspdm_vendor_response_callback_func vendor_response_callback;
-    libspdm_vendor_get_id_callback_func vendor_response_get_id;
 #endif /* LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES */
 
 #if LIBSPDM_EVENT_RECIPIENT_SUPPORT
diff --git a/include/library/spdm_common_lib.h b/include/library/spdm_common_lib.h
@@ -946,37 +946,25 @@ bool libspdm_get_fips_mode(void);
 #if LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES
 
 /**
- * Vendor Response Get Vendor ID Callback Function Pointer.
- * Required to be able to compose the Vendor Defined Response correctly
+ * Vendor Response Callback Function Pointer
  *
- * @param  spdm_context        A pointer to the SPDM context.
- * @param  session_id          If non-NULL then message is within a secure session.
- *                             If NULL then message is outside a secure session.
- * @param  resp_standard_id    Registry or Standards body used for response
- * @param  resp_vendor_id_len  Length in bytes of the vendor id field for the response
- * @param  resp_vendor_id      Vendor ID assigned by the Registry or Standards Body. Little-endian format
- **/
-typedef libspdm_return_t (*libspdm_vendor_get_id_callback_func)(
-    void *spdm_context,
-    const uint32_t *session_id,
-    uint16_t *resp_standard_id,
-    uint8_t *resp_vendor_id_len,
-    void *resp_vendor_id);
-
-/**
- * Vendor Response Callback Function Pointer.
+ * The library invokes this callback once. The integrator should:
+ * - Set resp_standard_id, resp_vendor_id_len, resp_vendor_id
+ * - Fill resp_data and set *resp_size based on the request parameters
  *
- * @param  spdm_context       A pointer to the SPDM context.
- * @param  session_id         If non-NULL then message is within a secure session.
- *                            If NULL then message is outside a secure session.
- * @param  req_standard_id    Registry or Standards body used for request
- * @param  req_vendor_id_len  Length in bytes of the vendor id field for the request
- * @param  req_vendor_id      Vendor ID assigned by the Registry or Standards Body. Little-endian format
- * @param  req_size           Length of the request
- * @param  req_data           The vendor defined request
- * @param  resp_size          Length of the response
- * @param  resp_data          The vendor defined response
- **/
+ * @param  spdm_context        A pointer to the SPDM context.
+ * @param  session_id          If non-NULL then message is within a secure session; otherwise NULL.
+ * @param  req_standard_id     Registry or Standards body used for request (from request header).
+ * @param  req_vendor_id_len   Length in bytes of the vendor id field for the request.
+ * @param  req_vendor_id       Request vendor ID (little-endian where applicable).
+ * @param  req_size            Length of the request.
+ * @param  req_data            Pointer to the request payload.
+ * @param  resp_standard_id    Registry/standards body used for response (header field).
+ * @param  resp_vendor_id_len  On input, capacity of resp_vendor_id; on output, bytes written.
+ * @param  resp_vendor_id      Buffer to receive response vendor ID.
+ * @param  resp_size           On input, capacity of resp_data; on output, bytes written.
+ * @param  resp_data           Buffer to receive the response payload.
+ */
 typedef libspdm_return_t (*libspdm_vendor_response_callback_func)(
     void *spdm_context,
     const uint32_t *session_id,
@@ -985,6 +973,9 @@ typedef libspdm_return_t (*libspdm_vendor_response_callback_func)(
     const void *req_vendor_id,
     uint32_t req_size,
     const void *req_data,
+    uint16_t *resp_standard_id,
+    uint8_t *resp_vendor_id_len,
+    void *resp_vendor_id,
     uint32_t *resp_size,
     void *resp_data);
 
diff --git a/include/library/spdm_responder_lib.h b/include/library/spdm_responder_lib.h
@@ -285,26 +285,12 @@ void libspdm_register_cert_chain_buffer(
 #if LIBSPDM_ENABLE_VENDOR_DEFINED_MESSAGES
 
 /**
- * This function registers the callback function for getting the Vendor ID for a VENDOR_DEFINED_RESPONSE to the device.
+ * Register the request-aware vendor-defined response callback.
  *
  * This is useful for creating unique responses to devices.
  *
  * @param  spdm_context     A pointer to the SPDM context.
- * @param  resp_callback_func   Response callback function
- *
- * @retval LIBSPDM_STATUS_SUCCESS Success
- * @retval LIBSPDM_STATUS_INVALID_PARAMETER Some parameters invalid or NULL
- **/
-libspdm_return_t libspdm_register_vendor_get_id_callback_func(void *spdm_context,
-                                                              libspdm_vendor_get_id_callback_func resp_callback);
-
-/**
- * This function registers the callback function for doing a VENDOR_DEFINED_RESPONSE to the device.
- *
- * This is useful for creating unique responses to devices.
- *
- * @param  spdm_context     A pointer to the SPDM context.
- * @param  resp_callback_func   Response callback function
+ * @param  resp_callback    Response callback function
  *
  * @retval LIBSPDM_STATUS_SUCCESS Success
  * @retval LIBSPDM_STATUS_INVALID_PARAMETER Some parameters invalid or NULL
diff --git a/library/spdm_responder_lib/libspdm_rsp_vendor_response.c b/library/spdm_responder_lib/libspdm_rsp_vendor_response.c
@@ -11,15 +11,6 @@
 /* expected number of bytes for VENDOR MESSAGE HEADERS */
 #define SPDM_VENDOR_DEFINED_FIXED_HEADER_LEN 7
 
-libspdm_return_t libspdm_register_vendor_get_id_callback_func(void *spdm_context,
-                                                              libspdm_vendor_get_id_callback_func resp_callback)
-{
-
-    libspdm_context_t *context = (libspdm_context_t *)spdm_context;
-    context->vendor_response_get_id = resp_callback;
-    return LIBSPDM_STATUS_SUCCESS;
-}
-
 libspdm_return_t libspdm_register_vendor_callback_func(void *spdm_context,
                                                        libspdm_vendor_response_callback_func resp_callback)
 {
@@ -79,9 +70,8 @@ libspdm_return_t libspdm_get_vendor_defined_response(libspdm_context_t *spdm_con
         session_id = &session_info->session_id;
     }
 
-    /* Check if caller is using the old Vendor Defined API. */
-    if ((spdm_context->vendor_response_callback == NULL ||
-         spdm_context->vendor_response_get_id == NULL)) {
+    /* Check if vendor callback is registered. */
+    if (spdm_context->vendor_response_callback == NULL) {
         if (spdm_context->get_response_func != NULL) {
             return ((libspdm_get_response_func)spdm_context->get_response_func)(
                 spdm_context,
@@ -182,8 +172,8 @@ libspdm_return_t libspdm_get_vendor_defined_response(libspdm_context_t *spdm_con
     libspdm_reset_message_buffer_via_request_code(spdm_context, NULL,
                                                   spdm_request->header.request_response_code);
 
-    /* length of spdm request/response header before payload start */
-    header_length = sizeof(spdm_vendor_defined_response_msg_t) + spdm_request->len +
+    /* reserve max vendor ID in header */
+    header_length = sizeof(spdm_vendor_defined_response_msg_t) + SPDM_MAX_VENDOR_ID_LENGTH +
                     sizeof(uint16_t);
     if (use_large_payload) {
         header_length += sizeof(uint32_t);
@@ -221,7 +211,7 @@ libspdm_return_t libspdm_get_vendor_defined_response(libspdm_context_t *spdm_con
      *      Len2 bytes Response Payload
      */
 
-    /* replace capacity with size */
+    /* Set up pointers for the callback */
     spdm_response->len = SPDM_MAX_VENDOR_ID_LENGTH;
     resp_data = ((uint8_t *)response) + sizeof(spdm_vendor_defined_response_msg_t);
 
@@ -237,39 +227,60 @@ libspdm_return_t libspdm_get_vendor_defined_response(libspdm_context_t *spdm_con
                    sizeof(uint16_t);
     }
 
-    status = spdm_context->vendor_response_get_id(
-        spdm_context,
-        session_id,
-        &spdm_response->standard_id,
-        &spdm_response->len,
-        resp_data);
+    /* Compute response pointers and maximum payload capacity. */
+    uint8_t *resp_vendor_id_ptr = resp_data;
+    uint8_t *resp_payload_ptr;
+    size_t len_field_size;
 
-    /* move pointer and adjust buffer size */
     if (use_large_payload) {
-        resp_data += spdm_response->len + sizeof(uint16_t) + sizeof(uint32_t);
-        response_capacity -= spdm_response->len + sizeof(uint16_t) + sizeof(uint32_t);
+        len_field_size = sizeof(uint16_t) + sizeof(uint32_t);
         resp_size = (uint32_t)response_capacity;
     } else {
-        resp_data += spdm_response->len + sizeof(uint16_t);
-        response_capacity -= spdm_response->len + sizeof(uint16_t);
+        len_field_size = sizeof(uint16_t);
         resp_size = (uint16_t)response_capacity;
     }
 
-    status = spdm_context->vendor_response_callback(spdm_context,
-                                                    session_id,
-                                                    spdm_request->standard_id,
-                                                    spdm_request->len,
-                                                    req_vendor_id, req_size, req_data,
-                                                    &resp_size,
-                                                    resp_data);
+    /* reserve max Vendor ID; compact after callback */
+    resp_payload_ptr = resp_vendor_id_ptr + spdm_response->len + len_field_size;
+
+    status = spdm_context->vendor_response_callback(
+        spdm_context,
+        session_id,
+        spdm_request->standard_id,
+        spdm_request->len,
+        req_vendor_id,
+        req_size,
+        req_data,
+        &spdm_response->standard_id,
+        &spdm_response->len,
+        resp_vendor_id_ptr,
+        &resp_size,
+        resp_payload_ptr);
+
+    if (LIBSPDM_STATUS_IS_ERROR(status)) {
+        return status;
+    }
+
+    /* compact payload after Vendor ID */
+    uint8_t *new_payload_ptr = resp_vendor_id_ptr + spdm_response->len + len_field_size;
+    if (new_payload_ptr != resp_payload_ptr && resp_size != 0) {
+        uint32_t bytes_to_move = resp_size;
+        uint8_t *src = resp_payload_ptr;
+        uint8_t *dst = new_payload_ptr;
+        while (bytes_to_move-- != 0) {
+            *(dst++) = *(src++);
+        }
+    }
 
-    /* store back the response payload size */
+    /* store response payload size */
     if (use_large_payload) {
-        libspdm_write_uint32((uint8_t *)(resp_data - sizeof(uint32_t)), resp_size);
-        *response_size = resp_size + (size_t)header_length;
+        libspdm_write_uint32((uint8_t *)(resp_data + spdm_response->len + sizeof(uint16_t)), resp_size);
+        *response_size = resp_size + sizeof(spdm_vendor_defined_response_msg_t) +
+                         spdm_response->len + sizeof(uint16_t) + sizeof(uint32_t);
     } else {
-        libspdm_write_uint16((uint8_t *)(resp_data - sizeof(uint16_t)), (uint16_t)resp_size);
-        *response_size = (uint16_t)resp_size + (size_t)header_length;
+        libspdm_write_uint16((uint8_t *)(resp_data + spdm_response->len), (uint16_t)resp_size);
+        *response_size = (uint16_t)resp_size + sizeof(spdm_vendor_defined_response_msg_t) +
+                         spdm_response->len + sizeof(uint16_t);
     }
 
     LIBSPDM_ASSERT(sizeof(spdm_vendor_defined_request_msg_t) ==
diff --git a/unit_test/fuzzing/test_responder/test_spdm_responder_vendor_cmds/vendor_cmds.c b/unit_test/fuzzing/test_responder/test_spdm_responder_vendor_cmds/vendor_cmds.c
@@ -15,16 +15,6 @@ size_t libspdm_get_max_buffer_size(void)
     return LIBSPDM_MAX_SPDM_MSG_SIZE;
 }
 
-libspdm_return_t libspdm_vendor_get_id_func_test(
-    void *spdm_context,
-    const uint32_t *session_id,
-    uint16_t *resp_standard_id,
-    uint8_t *resp_vendor_id_len,
-    void *resp_vendor_id)
-{
-    return LIBSPDM_STATUS_SUCCESS;
-}
-
 libspdm_return_t libspdm_vendor_response_func_test(
     void *spdm_context,
     const uint32_t *session_id,
@@ -33,9 +23,29 @@ libspdm_return_t libspdm_vendor_response_func_test(
     const void *req_vendor_id,
     uint32_t req_size,
     const void *req_data,
+    uint16_t *resp_standard_id,
+    uint8_t *resp_vendor_id_len,
+    void *resp_vendor_id,
     uint32_t *resp_size,
     void *resp_data)
 {
+    /* Validate required parameters */
+    if (resp_standard_id == NULL || resp_vendor_id_len == NULL || resp_vendor_id == NULL ||
+        resp_size == NULL || resp_data == NULL)
+        return LIBSPDM_STATUS_INVALID_PARAMETER;
+
+    /* Set response IDs */
+    *resp_standard_id = 6;
+    if (*resp_vendor_id_len >= 2) {
+        *resp_vendor_id_len = 2;
+        ((uint8_t*)resp_vendor_id)[0] = 0xAA;
+        ((uint8_t*)resp_vendor_id)[1] = 0xAA;
+    } else {
+        return LIBSPDM_STATUS_INVALID_PARAMETER;
+    }
+
+    /* Set response payload */
+    *resp_size = 0;
     return LIBSPDM_STATUS_SUCCESS;
 }
 
@@ -59,8 +69,6 @@ void libspdm_test_responder_vendor_cmds_case1(void **State)
         LIBSPDM_CONNECTION_STATE_NEGOTIATED;
     spdm_context->local_context.is_requester = true;
 
-    libspdm_register_vendor_get_id_callback_func(spdm_context,
-                                                 libspdm_vendor_get_id_func_test);
     libspdm_register_vendor_callback_func(spdm_context,
                                           libspdm_vendor_response_func_test);
 
diff --git a/unit_test/test_spdm_responder/error_test/vendor_response_err.c b/unit_test/test_spdm_responder/error_test/vendor_response_err.c
diff --git a/unit_test/test_spdm_responder/receive_send.c b/unit_test/test_spdm_responder/receive_send.c
diff --git a/unit_test/test_spdm_responder/vendor_defined_response.c b/unit_test/test_spdm_responder/vendor_defined_response.c
