From 36cd57ad0488cb33994a2ae1ef871c9b5209d629 Mon Sep 17 00:00:00 2001 From: Steven Bellock Date: Mon, 26 Jun 2023 16:59:07 -0700 Subject: [PATCH] memlib enhancements Fix #1585. Signed-off-by: Steven Bellock --- include/hal/library/memlib.h | 66 +++++++++----------------------- os_stub/memlib/copy_mem.c | 73 +++--------------------------------- os_stub/memlib/set_mem.c | 20 +--------- os_stub/memlib/zero_mem.c | 24 +----------- 4 files changed, 26 insertions(+), 157 deletions(-) diff --git a/include/hal/library/memlib.h b/include/hal/library/memlib.h index c7ac04446d4..4d784a5313d 100644 --- a/include/hal/library/memlib.h +++ b/include/hal/library/memlib.h @@ -15,76 +15,44 @@ #ifndef BASE_MEMORY_LIB #define BASE_MEMORY_LIB +#include "hal/base.h" + /** * Copies bytes from a source buffer to a destination buffer. * - * This function copies "src_len" bytes from "src_buf" to "dst_buf". - * - * Asserts and returns a non-zero value if any of the following are true: - * 1) "src_buf" or "dst_buf" are NULL. - * 2) "src_len" or "dst_len" is greater than (SIZE_MAX >> 1). - * 3) "src_len" is greater than "dst_len". - * 4) "src_buf" and "dst_buf" overlap. - * - * If any of these cases fail, a non-zero value is returned. Additionally if - * "dst_buf" points to a non-NULL value and "dst_len" is valid, then "dst_len" - * bytes of "dst_buf" are zeroed. - * - * This function follows the C11 cppreference description of memcpy_s. - * https://en.cppreference.com/w/c/string/byte/memcpy - * The cppreferece description does NOT allow the source or destination - * buffers to be NULL. - * - * This function differs from the Microsoft and Safeclib memcpy_s implementations - * in that the Microsoft and Safeclib implementations allow for NULL source and - * destinations pointers when the number of bytes to copy (src_len) is zero. - * - * In addition the Microsoft and Safeclib memcpy_s functions return different - * negative values on error. For best support, clients should generally check - * against zero for success or failure. + * This function copies src_len bytes from src_buf to dst_buf. The following properties should + * hold by the caller: + * - dst_buf and src_buf are non-NULL. + * - src_len is less than or equal to dst_len. + * - The source and destination buffers do not overlap. * + * The implementer of this function is free to check that these properties hold and take action + * if they fail to do so. + * @param dst_buf Destination buffer to copy to. - * @param dst_len Maximum length in bytes of the destination buffer. + * @param dst_len Size, in bytes, of the destination buffer. * @param src_buf Source buffer to copy from. * @param src_len The number of bytes to copy from the source buffer. - * - * @return 0 on success. non-zero on error. - * **/ -int libspdm_copy_mem(void *dst_buf, size_t dst_len, - const void *src_buf, size_t src_len); +void libspdm_copy_mem(void *dst_buf, size_t dst_len, + const void *src_buf, size_t src_len); /** - * Fills a target buffer with a byte value, and returns the target buffer. - * - * This function fills length bytes of buffer with value, and returns buffer. - * - * If length is greater than (MAX_ADDRESS - buffer + 1), then ASSERT(). + * Fills a target buffer with a byte value. * * @param buffer The memory to set. * @param length The number of bytes to set. * @param value The value with which to fill length bytes of buffer. - * - * @return buffer. - * **/ -void *libspdm_set_mem(void *buffer, size_t length, uint8_t value); +void libspdm_set_mem(void *buffer, size_t length, uint8_t value); /** - * Fills a target buffer with zeros, and returns the target buffer. - * - * This function fills length bytes of buffer with zeros, and returns buffer. - * - * If length > 0 and buffer is NULL, then ASSERT(). - * If length is greater than (MAX_ADDRESS - buffer + 1), then ASSERT(). + * Fills a target buffer with zeros. * * @param buffer The pointer to the target buffer to fill with zeros. * @param length The number of bytes in buffer to fill with zeros. - * - * @return buffer. - * **/ -void *libspdm_zero_mem(void *buffer, size_t length); +void libspdm_zero_mem(void *buffer, size_t length); /** * Compares the contents of two buffers in constant time. diff --git a/os_stub/memlib/copy_mem.c b/os_stub/memlib/copy_mem.c index 756f255cb48..dfd1f9cef8f 100644 --- a/os_stub/memlib/copy_mem.c +++ b/os_stub/memlib/copy_mem.c @@ -4,52 +4,11 @@ * License: BSD 3-Clause License. For full text see link: https://github.com/DMTF/libspdm/blob/main/LICENSE.md **/ -/** @file - * libspdm_copy_mem() implementation. - **/ - -#include "base.h" #include "library/debuglib.h" #include "hal/library/memlib.h" -/** - * Copies bytes from a source buffer to a destination buffer. - * - * This function copies "src_len" bytes from "src_buf" to "dst_buf". - * - * Asserts and returns a non-zero value if any of the following are true: - * 1) "src_buf" or "dst_buf" are NULL. - * 2) "src_len" or "dst_len" is greater than (SIZE_MAX >> 1). - * 3) "src_len" is greater than "dst_len". - * 4) "src_buf" and "dst_buf" overlap. - * - * If any of these cases fail, a non-zero value is returned. Additionally if - * "dst_buf" points to a non-NULL value and "dst_len" is valid, then "dst_len" - * bytes of "dst_buf" are zeroed. - * - * This function follows the C11 cppreference description of memcpy_s. - * https://en.cppreference.com/w/c/string/byte/memcpy - * The cppreferece description does NOT allow the source or destination - * buffers to be NULL. - * - * This function differs from the Microsoft and Safeclib memcpy_s implementations - * in that the Microsoft and Safeclib implementations allow for NULL source and - * destinations pointers when the number of bytes to copy (src_len) is zero. - * - * In addition the Microsoft and Safeclib memcpy_s functions return different - * negative values on error. For best support, clients should generally check - * against zero for success or failure. - * - * @param dst_buf Destination buffer to copy to. - * @param dst_len Maximum length in bytes of the destination buffer. - * @param src_buf Source buffer to copy from. - * @param src_len The number of bytes to copy from the source buffer. - * - * @return 0 on success. non-zero on error. - * - **/ -int libspdm_copy_mem(void *dst_buf, size_t dst_len, - const void *src_buf, size_t src_len) +void libspdm_copy_mem(void *dst_buf, size_t dst_len, + const void *src_buf, size_t src_len) { volatile uint8_t* dst; const volatile uint8_t* src; @@ -57,39 +16,17 @@ int libspdm_copy_mem(void *dst_buf, size_t dst_len, dst = (volatile uint8_t*) dst_buf; src = (const volatile uint8_t*) src_buf; - /* Check for case where "dst" or "dst_len" may be invalid. - * Do not zero "dst" in this case. */ - if (dst == NULL || dst_len > (SIZE_MAX >> 1)) { + if ((dst == NULL) || (src == NULL)) { LIBSPDM_ASSERT(0); - return -1; } - - /* Gaurd against invalid source. Zero "dst" in this case. */ - if (src == NULL) { - libspdm_zero_mem(dst_buf, dst_len); + if (((src < dst) && ((src + src_len) > dst)) || ((dst < src) && ((dst + src_len) > src))) { LIBSPDM_ASSERT(0); - return -1; } - - /* Guard against overlap case. Zero "dst" in these cases. */ - if ((src < dst && src + src_len > dst) || (dst < src && dst + src_len > src)) { - libspdm_zero_mem(dst_buf, dst_len); + if (src_len > dst_len) { LIBSPDM_ASSERT(0); - return -1; - } - - /* Guard against invalid lengths. Zero "dst" in these cases. */ - if (src_len > dst_len || - src_len > (SIZE_MAX >> 1)) { - - libspdm_zero_mem(dst_buf, dst_len); - LIBSPDM_ASSERT(0); - return -1; } while (src_len-- != 0) { *(dst++) = *(src++); } - - return 0; } diff --git a/os_stub/memlib/set_mem.c b/os_stub/memlib/set_mem.c index d487fce32fe..f2d0fbc48ad 100644 --- a/os_stub/memlib/set_mem.c +++ b/os_stub/memlib/set_mem.c @@ -4,23 +4,9 @@ * License: BSD 3-Clause License. For full text see link: https://github.com/DMTF/libspdm/blob/main/LICENSE.md **/ -#include "base.h" +#include "hal/library/memlib.h" -/** - * Fills a target buffer with a byte value, and returns the target buffer. - * - * This function fills length bytes of buffer with value, and returns buffer. - * - * If length is greater than (MAX_ADDRESS - buffer + 1), then LIBSPDM_ASSERT(). - * - * @param buffer The memory to set. - * @param length The number of bytes to set. - * @param value The value with which to fill length bytes of buffer. - * - * @return buffer. - * - **/ -void *libspdm_set_mem(void *buffer, size_t length, uint8_t value) +void libspdm_set_mem(void *buffer, size_t length, uint8_t value) { volatile uint8_t *pointer; @@ -28,6 +14,4 @@ void *libspdm_set_mem(void *buffer, size_t length, uint8_t value) while (length-- != 0) { *(pointer++) = value; } - - return buffer; } diff --git a/os_stub/memlib/zero_mem.c b/os_stub/memlib/zero_mem.c index 0ca19b0224f..7895974d3be 100644 --- a/os_stub/memlib/zero_mem.c +++ b/os_stub/memlib/zero_mem.c @@ -4,27 +4,9 @@ * License: BSD 3-Clause License. For full text see link: https://github.com/DMTF/libspdm/blob/main/LICENSE.md **/ -/** @file - * libspdm_zero_mem() implementation. - **/ - -#include "base.h" +#include "hal/library/memlib.h" -/** - * Fills a target buffer with zeros, and returns the target buffer. - * - * This function fills length bytes of buffer with zeros, and returns buffer. - * - * If length > 0 and buffer is NULL, then LIBSPDM_ASSERT(). - * If length is greater than (MAX_ADDRESS - buffer + 1), then LIBSPDM_ASSERT(). - * - * @param buffer The pointer to the target buffer to fill with zeros. - * @param length The number of bytes in buffer to fill with zeros. - * - * @return buffer. - * - **/ -void *libspdm_zero_mem(void *buffer, size_t length) +void libspdm_zero_mem(void *buffer, size_t length) { volatile uint8_t *pointer; @@ -32,6 +14,4 @@ void *libspdm_zero_mem(void *buffer, size_t length) while (length-- != 0) { *(pointer++) = 0; } - - return buffer; }