From 195bbcc3156de3d5afaeadb6e653f540be4c7e01 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 3 May 2024 16:15:38 -0700 Subject: [PATCH] Generic Memory Pools Fix 1. Add some expository comments describing the purpose of: * WOLFMEM_MAX_BUCKETS * WOLFMEM_DEF_BUCKETS * WOLFMEM_BUCKETS * WOLFMEM_DIST 2. Switch the API test for LoadStaticMemory() to named constants. 3. Delete redundant test case. Add a new test case. 4. In the wolfCrypt test for the memory constants, check the sizes of the WOLFMEM_BUCKETS and WOLFMEM_DIST lists against WOLFMEM_DEF_BUCKETS which should be their length. Check that WOLFMEM_DEF_BUCKETS is not greater than WOLFMEM_MAX_BUCKETS. 5. Default for WOLFMEM_MAX_BUCKETS should be WOLFMEM_DEF_BUCKETS, set it to what is specified. Add a warning if MAX is less than DEF. 6. Separate the definition of the constant LARGEST_MEM_BUCKET so it is dependent on config and not if WOLFMEM_BUCKETS isn't set. --- tests/api.c | 41 ++++++++++++++++------ wolfcrypt/src/memory.c | 18 +++++++--- wolfcrypt/test/test.c | 12 ++++--- wolfssl/wolfcrypt/memory.h | 70 +++++++++++++++++++++++++------------- 4 files changed, 99 insertions(+), 42 deletions(-) diff --git a/tests/api.c b/tests/api.c index e336f6877..e5786b621 100644 --- a/tests/api.c +++ b/tests/api.c @@ -672,15 +672,32 @@ static int test_wolfCrypt_Cleanup(void) return EXPECT_RESULT(); } + +#ifdef WOLFSSL_STATIC_MEMORY + #define TEST_LSM_STATIC_SIZE 440000 + /* Create new bucket list, using the default list, adding + * one dang large buffer size. */ + #define TEST_LSM_DEF_BUCKETS (WOLFMEM_DEF_BUCKETS+1) + #define TEST_LSM_BUCKETS WOLFMEM_BUCKETS,(LARGEST_MEM_BUCKET*2) + #define TEST_LSM_DIST WOLFMEM_DIST,1 +#endif + static int test_wc_LoadStaticMemory_ex(void) { EXPECT_DECLS; #ifdef WOLFSSL_STATIC_MEMORY - byte staticMemory[440000]; - word32 sizeList[WOLFMEM_DEF_BUCKETS] = { WOLFMEM_BUCKETS }; - word32 distList[WOLFMEM_DEF_BUCKETS] = { WOLFMEM_DIST }; + byte staticMemory[TEST_LSM_STATIC_SIZE]; + word32 sizeList[TEST_LSM_DEF_BUCKETS] = { TEST_LSM_BUCKETS }; + word32 distList[TEST_LSM_DEF_BUCKETS] = { TEST_LSM_DIST }; WOLFSSL_HEAP_HINT* heap; + /* For this test, the size and dist lists will be the ones configured + * for the build, or default. The value of WOLFMEM_DEF_BUCKETS is 9, + * so these lists are 10 long. For most tests, the value of + * WOLFMEM_DEF_BUCKETS is used. There's a test case where one is added + * to that, to make sure the list size is larger than + * WOLFMEM_MAX_BUCKETS. */ + /* Pass in zero everything. */ ExpectIntEQ(wc_LoadStaticMemory_ex(NULL, 0, NULL, NULL, NULL, 0, 0, 0), BAD_FUNC_ARG); @@ -711,6 +728,7 @@ static int test_wc_LoadStaticMemory_ex(void) NULL, (word32)sizeof(staticMemory), 0, 1), BAD_FUNC_ARG); + /* Set the size of the static buffer to 0. */ heap = NULL; ExpectIntEQ(wc_LoadStaticMemory_ex(&heap, @@ -728,14 +746,6 @@ static int test_wc_LoadStaticMemory_ex(void) 0, 1), BUFFER_E); - /* Set the number of buckets to 1 too many allowed. */ - heap = NULL; - ExpectIntEQ(wc_LoadStaticMemory_ex(&heap, - WOLFMEM_MAX_BUCKETS+1, sizeList, distList, - staticMemory, (word32)sizeof(staticMemory), - 0, 1), - BAD_FUNC_ARG); - /* Set the size of the static buffer to exactly the minimum size. */ heap = NULL; ExpectIntEQ(wc_LoadStaticMemory_ex(&heap, @@ -746,6 +756,15 @@ static int test_wc_LoadStaticMemory_ex(void) 0); wc_UnloadStaticMemory(heap); + /* Use more buckets than able. Success case. */ + heap = NULL; + ExpectIntEQ(wc_LoadStaticMemory_ex(&heap, + WOLFMEM_DEF_BUCKETS*2, sizeList, distList, + staticMemory, (word32)sizeof(staticMemory), + 0, 1), + 0); + wc_UnloadStaticMemory(heap); + /* Success case. */ heap = NULL; ExpectIntEQ(wc_LoadStaticMemory_ex(&heap, diff --git a/wolfcrypt/src/memory.c b/wolfcrypt/src/memory.c index 61fe735b6..2e221a9c1 100644 --- a/wolfcrypt/src/memory.c +++ b/wolfcrypt/src/memory.c @@ -656,11 +656,16 @@ int wc_LoadStaticMemory_ex(WOLFSSL_HEAP_HINT** pHint, WOLFSSL_ENTER("wc_LoadStaticMemory_ex"); - if (pHint == NULL || buf == NULL || listSz > WOLFMEM_MAX_BUCKETS - || sizeList == NULL || distList == NULL) { + if (pHint == NULL || buf == NULL || sizeList == NULL || distList == NULL) { return BAD_FUNC_ARG; } + /* Cap the listSz to the actual number of items allocated in the list. */ + if (listSz > WOLFMEM_MAX_BUCKETS) { + WOLFSSL_MSG("Truncating the list of memory buckets"); + listSz = WOLFMEM_MAX_BUCKETS; + } + if ((sizeof(WOLFSSL_HEAP) + sizeof(WOLFSSL_HEAP_HINT)) > sz - idx) { WOLFSSL_MSG("Not enough memory for partition tracking"); return BUFFER_E; /* not enough memory for structures */ @@ -761,11 +766,16 @@ int wolfSSL_StaticBufferSz_ex(unsigned int listSz, WOLFSSL_ENTER("wolfSSL_StaticBufferSz_ex"); - if (buffer == NULL || listSz > WOLFMEM_MAX_BUCKETS - || sizeList == NULL || distList == NULL) { + if (buffer == NULL || sizeList == NULL || distList == NULL) { return BAD_FUNC_ARG; } + /* Cap the listSz to the actual number of items allocated in the list. */ + if (listSz > WOLFMEM_MAX_BUCKETS) { + WOLFSSL_MSG("Truncating the list of memory buckets"); + listSz = WOLFMEM_MAX_BUCKETS; + } + /* align pt */ while ((wc_ptr_t)pt % WOLFSSL_STATIC_ALIGN && pt < (buffer + sz)) { pt++; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index d7589fc8b..6c025c3e0 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -16089,22 +16089,26 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t memory_test(void) #ifdef WOLFSSL_STATIC_MEMORY /* check macro settings */ - if (sizeof(size)/sizeof(word32) != WOLFMEM_MAX_BUCKETS) { + if (sizeof(size)/sizeof(word32) != WOLFMEM_DEF_BUCKETS) { return WC_TEST_RET_ENC_NC; } - if (sizeof(dist)/sizeof(word32) != WOLFMEM_MAX_BUCKETS) { + if (sizeof(dist)/sizeof(word32) != WOLFMEM_DEF_BUCKETS) { return WC_TEST_RET_ENC_NC; } - for (i = 0; i < WOLFMEM_MAX_BUCKETS; i++) { + if (WOLFMEM_DEF_BUCKETS > WOLFMEM_MAX_BUCKETS) { + return WC_TEST_RET_ENC_NC; + } + + for (i = 0; i < WOLFMEM_DEF_BUCKETS; i++) { if ((size[i] % WOLFSSL_STATIC_ALIGN) != 0) { /* each element in array should be divisible by alignment size */ return WC_TEST_RET_ENC_NC; } } - for (i = 1; i < WOLFMEM_MAX_BUCKETS; i++) { + for (i = 1; i < WOLFMEM_DEF_BUCKETS; i++) { if (size[i - 1] >= size[i]) { return WC_TEST_RET_ENC_NC; /* sizes should be in increasing order */ } diff --git a/wolfssl/wolfcrypt/memory.h b/wolfssl/wolfcrypt/memory.h index 714f56557..e6b7dc546 100644 --- a/wolfssl/wolfcrypt/memory.h +++ b/wolfssl/wolfcrypt/memory.h @@ -101,48 +101,72 @@ WOLFSSL_API int wolfSSL_GetAllocators(wolfSSL_Malloc_cb* mf, #ifndef WOLFSSL_STATIC_ALIGN #define WOLFSSL_STATIC_ALIGN 16 #endif - #ifndef WOLFMEM_MAX_BUCKETS - #define WOLFMEM_MAX_BUCKETS 9 +/* WOLFMEM_BUCKETS - list of the sizes of buckets in the pool + * WOLFMEM_DIST - list of quantities of buffers in the buckets + * WOLFMEM_DEF_BUCKETS - number of values in WOLFMEM_BUCKETS and WOLFMEM_DIST + * WOLFMEM_MAX_BUCKETS - size of the arrays used to store the buckets and + * dists in the memory pool; defaults to WOLFMEM_DEF_BUCKETS + * + * The following defines provide a reasonable set of buckets in the memory + * pool for running wolfSSL on a Linux box. The bucket and dist lists below + * have nine items each, so WOLFMEM_DEF_BUCKETS is set to 9. + * + * If WOLFMEM_DEF_BUCKETS is less then WOLFMEM_MAX_BUCKETS, the unused values + * are set to zero and ignored. If WOLFMEM_MAX_BUCKETS is less than + * WOLFMEM_DEF_BUCKETS, not all the buckets will be created in the pool. + */ + #ifndef WOLFMEM_DEF_BUCKETS + #define WOLFMEM_DEF_BUCKETS 9 /* number of default memory blocks */ #endif - #define WOLFMEM_DEF_BUCKETS 9 /* number of default memory blocks */ + + #ifndef WOLFMEM_MAX_BUCKETS + #define WOLFMEM_MAX_BUCKETS WOLFMEM_DEF_BUCKETS + #endif + + #if WOLFMEM_MAX_BUCKETS < WOLFMEM_DEF_BUCKETS + #warning "ignoring excess buckets, MAX_BUCKETS less than DEF_BUCKETS" + #endif + #ifndef WOLFMEM_IO_SZ #define WOLFMEM_IO_SZ 16992 /* 16 byte aligned */ #endif + + #ifndef LARGEST_MEM_BUCKET + #ifndef SESSION_CERTS + #define LARGEST_MEM_BUCKET 16128 + #elif defined(OPENSSL_EXTRA) + #ifdef WOLFSSL_TLS13 + #define LARGEST_MEM_BUCKET 30400 + #else + #define LARGEST_MEM_BUCKET 25600 + #endif + #elif defined(WOLFSSL_CERT_EXT) + /* certificate extensions requires 24k for the SSL struct */ + #define LARGEST_MEM_BUCKET 24576 + #else + /* increase 23k for object member of WOLFSSL_X509_NAME_ENTRY */ + #define LARGEST_MEM_BUCKET 23440 + #endif + #endif + #ifndef WOLFMEM_BUCKETS #ifndef SESSION_CERTS /* default size of chunks of memory to separate into */ - #ifndef LARGEST_MEM_BUCKET - #define LARGEST_MEM_BUCKET 16128 - #endif #define WOLFMEM_BUCKETS 64,128,256,512,1024,2432,3456,4544,\ LARGEST_MEM_BUCKET - #elif defined (OPENSSL_EXTRA) + #elif defined(OPENSSL_EXTRA) /* extra storage in structs for multiple attributes and order */ - #ifndef LARGEST_MEM_BUCKET - #ifdef WOLFSSL_TLS13 - #define LARGEST_MEM_BUCKET 30400 - #else - #define LARGEST_MEM_BUCKET 25600 - #endif - #endif #define WOLFMEM_BUCKETS 64,128,256,512,1024,2432,3360,4480,\ LARGEST_MEM_BUCKET - #elif defined (WOLFSSL_CERT_EXT) - /* certificate extensions requires 24k for the SSL struct */ - #ifndef LARGEST_MEM_BUCKET - #define LARGEST_MEM_BUCKET 24576 - #endif + #elif defined(WOLFSSL_CERT_EXT) #define WOLFMEM_BUCKETS 64,128,256,512,1024,2432,3456,4544,\ LARGEST_MEM_BUCKET #else - /* increase 23k for object member of WOLFSSL_X509_NAME_ENTRY */ - #ifndef LARGEST_MEM_BUCKET - #define LARGEST_MEM_BUCKET 23440 - #endif #define WOLFMEM_BUCKETS 64,128,256,512,1024,2432,3456,4544,\ LARGEST_MEM_BUCKET #endif #endif + #ifndef WOLFMEM_DIST #ifndef WOLFSSL_STATIC_MEMORY_SMALL #define WOLFMEM_DIST 49,10,6,14,5,6,9,1,1