Repository: incubator-mynewt-larva Updated Branches: refs/heads/master 75d5f52e9 -> 906f73eb6
MYNEWT-77: add more checks to make sure item being freed to memory pool is valid. Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/commit/906f73eb Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/tree/906f73eb Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/diff/906f73eb Branch: refs/heads/master Commit: 906f73eb687d300adb94c995d0b25d2cfd7e8258 Parents: 75d5f52 Author: wes3 <w...@micosa.io> Authored: Mon Feb 15 09:11:30 2016 -0800 Committer: wes3 <w...@micosa.io> Committed: Mon Feb 15 09:20:34 2016 -0800 ---------------------------------------------------------------------- libs/os/include/os/os_mempool.h | 1 + libs/os/src/os_mempool.c | 24 ++++++++++++++++++++++-- libs/os/src/test/mempool_test.c | 17 +++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/906f73eb/libs/os/include/os/os_mempool.h ---------------------------------------------------------------------- diff --git a/libs/os/include/os/os_mempool.h b/libs/os/include/os/os_mempool.h index 9888db5..6b0f24c 100644 --- a/libs/os/include/os/os_mempool.h +++ b/libs/os/include/os/os_mempool.h @@ -43,6 +43,7 @@ struct os_mempool { int mp_block_size; /* Size of the memory blocks, in bytes. */ int mp_num_blocks; /* The number of memory blocks. */ int mp_num_free; /* The number of free blocks left */ + uint32_t mp_membuf_addr; /* Address of memory buffer used by pool */ STAILQ_ENTRY(os_mempool) mp_list; SLIST_HEAD(,os_memblock); /* Pointer to list of free blocks */ char *name; /* Name for memory block */ http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/906f73eb/libs/os/src/os_mempool.c ---------------------------------------------------------------------- diff --git a/libs/os/src/os_mempool.c b/libs/os/src/os_mempool.c index fe64ec9..1e7428c 100644 --- a/libs/os/src/os_mempool.c +++ b/libs/os/src/os_mempool.c @@ -20,6 +20,9 @@ #include "os/os.h" #include <string.h> +#include <assert.h> + +#define OS_MEMPOOL_TRUE_BLOCK_SIZE(bsize) OS_ALIGN(bsize, OS_ALIGNMENT) STAILQ_HEAD(, os_mempool) g_os_mempool_list = STAILQ_HEAD_INITIALIZER(g_os_mempool_list); @@ -54,12 +57,13 @@ os_mempool_init(struct os_mempool *mp, int blocks, int block_size, void *membuf, if (((uint32_t)membuf & (OS_ALIGNMENT - 1)) != 0) { return OS_MEM_NOT_ALIGNED; } - true_block_size = OS_ALIGN(block_size, OS_ALIGNMENT); + true_block_size = OS_MEMPOOL_TRUE_BLOCK_SIZE(block_size); /* Initialize the memory pool structure */ mp->mp_block_size = block_size; mp->mp_num_free = blocks; mp->mp_num_blocks = blocks; + mp->mp_membuf_addr = (uint32_t)membuf; mp->name = name; SLIST_FIRST(mp) = membuf; @@ -130,14 +134,30 @@ os_memblock_get(struct os_mempool *mp) os_error_t os_memblock_put(struct os_mempool *mp, void *block_addr) { - struct os_memblock *block; os_sr_t sr; + uint32_t end; + uint32_t true_block_size; + uint32_t baddr32; + struct os_memblock *block; /* Make sure parameters are valid */ if ((mp == NULL) || (block_addr == NULL)) { return OS_INVALID_PARM; } + /* Check that the block we are freeing is a valid block! */ + baddr32 = (uint32_t)block_addr; + true_block_size = OS_MEMPOOL_TRUE_BLOCK_SIZE(mp->mp_block_size); + end = mp->mp_membuf_addr + (mp->mp_num_blocks * true_block_size); + if ((baddr32 < mp->mp_membuf_addr) || (baddr32 >= end)) { + return OS_INVALID_PARM; + } + + /* All freed blocks should be on true block size boundaries! */ + if (((baddr32 - mp->mp_membuf_addr) % true_block_size) != 0) { + return OS_INVALID_PARM; + } + /* * XXX: we should do boundary checks here! The block had better be within * the pool. If it fails, do we return an error or assert()? Add this when http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/906f73eb/libs/os/src/test/mempool_test.c ---------------------------------------------------------------------- diff --git a/libs/os/src/test/mempool_test.c b/libs/os/src/test/mempool_test.c index 9a4fdc0..cd17c90 100644 --- a/libs/os/src/test/mempool_test.c +++ b/libs/os/src/test/mempool_test.c @@ -66,6 +66,7 @@ mempool_test(int num_blocks, int block_size) int cnt; int true_block_size; int mem_pool_size; + uint32_t test_block; uint8_t *tstptr; void **free_ptr; void *block; @@ -190,6 +191,22 @@ mempool_test(int num_blocks, int block_size) TEST_ASSERT(os_memblock_get(NULL) == NULL, "No error trying to get a block from NULL pool"); + + /* Attempt to free a block outside the range of the membuf */ + test_block = g_TstMempool.mp_membuf_addr; + test_block -= 4; + rc = os_memblock_put(&g_TstMempool, (void *)test_block); + TEST_ASSERT(rc == OS_INVALID_PARM, "No error freeing bad block address"); + + test_block += (true_block_size * g_TstMempool.mp_num_blocks) + 100; + rc = os_memblock_put(&g_TstMempool, (void *)test_block); + TEST_ASSERT(rc == OS_INVALID_PARM, "No error freeing bad block address"); + + /* Attempt to free on bad boundary */ + test_block = g_TstMempool.mp_membuf_addr; + test_block += (true_block_size / 2); + rc = os_memblock_put(&g_TstMempool, (void *)test_block); + TEST_ASSERT(rc == OS_INVALID_PARM, "No error freeing bad block address"); } /**
