You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2017/11/20 23:28:24 UTC
[GitHub] mkiiskila closed pull request #661: OS minimal stack check at context switch time
mkiiskila closed pull request #661: OS minimal stack check at context switch time
URL: https://github.com/apache/mynewt-core/pull/661
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/kernel/os/include/os/os.h b/kernel/os/include/os/os.h
index 9ff6ed58e..80eaf8217 100644
--- a/kernel/os/include/os/os.h
+++ b/kernel/os/include/os/os.h
@@ -23,6 +23,8 @@
#include <stdlib.h>
#include <inttypes.h>
+#include <syscfg/syscfg.h>
+
#ifdef __cplusplus
extern "C" {
#endif
diff --git a/kernel/os/src/os_mempool.c b/kernel/os/src/os_mempool.c
index 2c0d42edf..65e4f6159 100644
--- a/kernel/os/src/os_mempool.c
+++ b/kernel/os/src/os_mempool.c
@@ -29,15 +29,45 @@
* @defgroup OSMempool Memory Pools
* @{
*/
-/*
-#define OS_CHECK_DUP_FREE 1
-*/
-#define OS_MEMPOOL_TRUE_BLOCK_SIZE(bsize) OS_ALIGN(bsize, OS_ALIGNMENT)
+#define OS_MEM_TRUE_BLOCK_SIZE(bsize) OS_ALIGN(bsize, OS_ALIGNMENT)
+#define OS_MEMPOOL_TRUE_BLOCK_SIZE(mp) OS_MEM_TRUE_BLOCK_SIZE(mp->mp_block_size)
STAILQ_HEAD(, os_mempool) g_os_mempool_list =
STAILQ_HEAD_INITIALIZER(g_os_mempool_list);
+#if MYNEWT_VAL(OS_MEMPOOL_POISON)
+static uint32_t os_mem_poison = 0xde7ec7ed;
+
+static void
+os_mempool_poison(void *start, int sz)
+{
+ int i;
+ char *p = start;
+
+ for (i = sizeof(struct os_memblock); i < sz;
+ i = i + sizeof(os_mem_poison)) {
+ memcpy(p + i, &os_mem_poison, min(sizeof(os_mem_poison), sz - i));
+ }
+}
+
+static void
+os_mempool_poison_check(void *start, int sz)
+{
+ int i;
+ char *p = start;
+
+ for (i = sizeof(struct os_memblock); i < sz;
+ i = i + sizeof(os_mem_poison)) {
+ assert(!memcmp(p + i, &os_mem_poison,
+ min(sizeof(os_mem_poison), sz - i)));
+ }
+}
+#else
+#define os_mempool_poison(start, sz)
+#define os_mempool_poison_check(start, sz)
+#endif
+
/**
* os mempool init
*
@@ -60,7 +90,7 @@ os_mempool_init(struct os_mempool *mp, int blocks, int block_size,
struct os_memblock *block_ptr;
/* Check for valid parameters */
- if ((!mp) || (blocks < 0) || (block_size <= 0)) {
+ if (!mp || (blocks < 0) || (block_size <= sizeof(struct os_memblock))) {
return OS_INVALID_PARM;
}
@@ -76,7 +106,7 @@ os_mempool_init(struct os_mempool *mp, int blocks, int block_size,
return OS_MEM_NOT_ALIGNED;
}
}
- true_block_size = OS_MEMPOOL_TRUE_BLOCK_SIZE(block_size);
+ true_block_size = OS_MEM_TRUE_BLOCK_SIZE(block_size);
/* Initialize the memory pool structure */
mp->mp_block_size = block_size;
@@ -85,6 +115,7 @@ os_mempool_init(struct os_mempool *mp, int blocks, int block_size,
mp->mp_num_blocks = blocks;
mp->mp_membuf_addr = (uint32_t)membuf;
mp->name = name;
+ os_mempool_poison(membuf, true_block_size);
SLIST_FIRST(mp) = membuf;
/* Chain the memory blocks to the free list */
@@ -92,6 +123,7 @@ os_mempool_init(struct os_mempool *mp, int blocks, int block_size,
block_ptr = (struct os_memblock *)block_addr;
while (blocks > 1) {
block_addr += true_block_size;
+ os_mempool_poison(block_addr, true_block_size);
SLIST_NEXT(block_ptr, mb_next) = (struct os_memblock *)block_addr;
block_ptr = (struct os_memblock *)block_addr;
--blocks;
@@ -125,6 +157,7 @@ os_mempool_is_sane(const struct os_mempool *mp)
if (!os_memblock_from(mp, block)) {
return false;
}
+ os_mempool_poison_check(block, OS_MEMPOOL_TRUE_BLOCK_SIZE(mp));
}
return true;
@@ -150,7 +183,7 @@ os_memblock_from(const struct os_mempool *mp, const void *block_addr)
"Pointer to void must be 32-bits.");
baddr32 = (uint32_t)block_addr;
- true_block_size = OS_MEMPOOL_TRUE_BLOCK_SIZE(mp->mp_block_size);
+ true_block_size = OS_MEMPOOL_TRUE_BLOCK_SIZE(mp);
end = mp->mp_membuf_addr + (mp->mp_num_blocks * true_block_size);
/* Check that the block is in the memory buffer range. */
@@ -190,6 +223,8 @@ os_memblock_get(struct os_mempool *mp)
/* Get a free block */
block = SLIST_FIRST(mp);
+ os_mempool_poison_check(block, OS_MEMPOOL_TRUE_BLOCK_SIZE(mp));
+
/* Set new free list head */
SLIST_FIRST(mp) = SLIST_NEXT(block, mb_next);
@@ -226,16 +261,18 @@ os_memblock_put(struct os_mempool *mp, void *block_addr)
return OS_INVALID_PARM;
}
+#if MYNEWT_VAL(OS_MEMPOOL_CHECK)
/* Check that the block we are freeing is a valid block! */
- if (!os_memblock_from(mp, block_addr)) {
- return OS_INVALID_PARM;
- }
+ assert(os_memblock_from(mp, block_addr));
-#ifdef OS_CHECK_DUP_FREE
+ /*
+ * Check for duplicate free.
+ */
SLIST_FOREACH(block, mp, mb_next) {
assert(block != (struct os_memblock *)block_addr);
}
#endif
+ os_mempool_poison(block_addr, OS_MEMPOOL_TRUE_BLOCK_SIZE(mp));
block = (struct os_memblock *)block_addr;
OS_ENTER_CRITICAL(sr);
diff --git a/kernel/os/src/os_msys_init.c b/kernel/os/src/os_msys_init.c
index 8a27f33ae..1f5e1c35c 100644
--- a/kernel/os/src/os_msys_init.c
+++ b/kernel/os/src/os_msys_init.c
@@ -46,40 +46,6 @@ static struct os_mbuf_pool os_msys_init_2_mbuf_pool;
static struct os_mempool os_msys_init_2_mempool;
#endif
-#if MYNEWT_VAL(MSYS_3_BLOCK_COUNT) > 0
-#define SYSINIT_MSYS_3_MEMBLOCK_SIZE \
- OS_ALIGN(MYNEWT_VAL(MSYS_3_BLOCK_SIZE), 4)
-#define SYSINIT_MSYS_3_MEMPOOL_SIZE \
- OS_MEMPOOL_SIZE(MYNEWT_VAL(MSYS_3_BLOCK_COUNT), \
- SYSINIT_MSYS_3_MEMBLOCK_SIZE)
-static os_membuf_t os_msys_init_3_data[SYSINIT_MSYS_3_MEMPOOL_SIZE];
-static struct os_mbuf_pool os_msys_init_3_mbuf_pool;
-static struct os_mempool os_msys_init_3_mempool;
-#endif
-
-#if MYNEWT_VAL(MSYS_4_BLOCK_COUNT) > 0
-#define SYSINIT_MSYS_4_MEMBLOCK_SIZE \
- OS_ALIGN(MYNEWT_VAL(MSYS_4_BLOCK_SIZE), 4)
-#define SYSINIT_MSYS_4_MEMPOOL_SIZE \
- OS_MEMPOOL_SIZE(MYNEWT_VAL(MSYS_4_BLOCK_COUNT), \
- SYSINIT_MSYS_4_MEMBLOCK_SIZE)
-static os_membuf_t os_msys_init_4_data[SYSINIT_MSYS_4_MEMPOOL_SIZE];
-static struct os_mbuf_pool os_msys_init_4_mbuf_pool;
-static struct os_mempool os_msys_init_4_mempool;
-#endif
-
-#if MYNEWT_VAL(MSYS_5_BLOCK_COUNT) > 0
-#define SYSINIT_MSYS_5_MEMBLOCK_SIZE \
- OS_ALIGN(MYNEWT_VAL(MSYS_5_BLOCK_SIZE), 4)
-#define SYSINIT_MSYS_5_MEMPOOL_SIZE \
- OS_MEMPOOL_SIZE(MYNEWT_VAL(MSYS_5_BLOCK_COUNT), \
- SYSINIT_MSYS_5_MEMBLOCK_SIZE)
-
-static os_membuf_t os_msys_init_5_data[SYSINIT_MSYS_5_MEMPOOL_SIZE];
-static struct os_mbuf_pool os_msys_init_5_mbuf_pool;
-static struct os_mempool os_msys_init_5_mempool;
-#endif
-
static void
os_msys_init_once(void *data, struct os_mempool *mempool,
struct os_mbuf_pool *mbuf_pool,
@@ -118,31 +84,4 @@ os_msys_init(void)
SYSINIT_MSYS_2_MEMBLOCK_SIZE,
"msys_2");
#endif
-
-#if MYNEWT_VAL(MSYS_3_BLOCK_COUNT) > 0
- os_msys_init_once(os_msys_init_3_data,
- &os_msys_init_3_mempool,
- &os_msys_init_3_mbuf_pool,
- MYNEWT_VAL(MSYS_3_BLOCK_COUNT),
- SYSINIT_MSYS_3_MEMBLOCK_SIZE,
- "msys_3");
-#endif
-
-#if MYNEWT_VAL(MSYS_4_BLOCK_COUNT) > 0
- os_msys_init_once(os_msys_init_4_data,
- &os_msys_init_4_mempool,
- &os_msys_init_4_mbuf_pool,
- MYNEWT_VAL(MSYS_4_BLOCK_COUNT),
- SYSINIT_MSYS_4_MEMBLOCK_SIZE,
- "msys_4");
-#endif
-
-#if MYNEWT_VAL(MSYS_5_BLOCK_COUNT) > 0
- os_msys_init_once(os_msys_init_5_data,
- &os_msys_init_5_mempool,
- &os_msys_init_5_mbuf_pool,
- MYNEWT_VAL(MSYS_5_BLOCK_COUNT),
- SYSINIT_MSYS_5_MEMBLOCK_SIZE,
- "msys_5");
-#endif
}
diff --git a/kernel/os/src/os_sched.c b/kernel/os/src/os_sched.c
index 31319390a..cb0b02063 100644
--- a/kernel/os/src/os_sched.c
+++ b/kernel/os/src/os_sched.c
@@ -84,6 +84,15 @@ os_sched_insert(struct os_task *t)
void
os_sched_ctx_sw_hook(struct os_task *next_t)
{
+#if MYNEWT_VAL(OS_CTX_SW_STACK_CHECK)
+ os_stack_t *top;
+ int i;
+
+ top = g_current_task->t_stacktop - g_current_task->t_stacksize;
+ for (i = 0; i < MYNEWT_VAL(OS_CTX_SW_STACK_GUARD); i++) {
+ assert(top[i] == OS_STACK_PATTERN);
+ }
+#endif
os_trace_task_start_exec(next_t->t_taskid);
next_t->t_ctx_sw_cnt++;
g_current_task->t_run_time += g_os_time - g_os_last_ctx_sw_time;
diff --git a/kernel/os/syscfg.yml b/kernel/os/syscfg.yml
index d8206fed8..41122956d 100644
--- a/kernel/os/syscfg.yml
+++ b/kernel/os/syscfg.yml
@@ -39,6 +39,18 @@ syscfg.defs:
OS_SCHEDULING:
description: 'Whether OS will be started or not'
value: 1
+ OS_CTX_SW_STACK_CHECK:
+ description: 'Whether to do stack sanity check during context switch'
+ value: 0
+ OS_CTX_SW_STACK_GUARD:
+ description: 'How many os_stack_ts to keep as stack guard'
+ value: 4
+ OS_MEMPOOL_CHECK:
+ description: 'Whether to do stack sanity check of mempool operations'
+ value: 0
+ OS_MEMPOOL_POISON:
+ description: 'Whether to do write known pattern to freed memory'
+ value: 0
OS_CPUTIME_FREQ:
description: 'Frequency of os cputime'
value: 1000000
@@ -63,24 +75,6 @@ syscfg.defs:
MSYS_2_BLOCK_SIZE:
description: '2nd system pool of mbufs; size of an entry'
value: 0
- MSYS_3_BLOCK_COUNT:
- description: '3rd system pool of mbufs; number of entries'
- value: 0
- MSYS_3_BLOCK_SIZE:
- description: '3rd system pool of mbufs; size of an entry'
- value: 0
- MSYS_4_BLOCK_COUNT:
- description: '4th system pool of mbufs; number of entries'
- value: 0
- MSYS_4_BLOCK_SIZE:
- description: '4th system pool of mbufs; size of an entry'
- value: 0
- MSYS_5_BLOCK_COUNT:
- description: '5th system pool of mbufs; number of entries'
- value: 0
- MSYS_5_BLOCK_SIZE:
- description: '5th system pool of mbufs; size of an entry'
- value: 0
FLOAT_USER:
descriptiong: 'Enable float support for users'
value: 0
diff --git a/kernel/os/test/src/testcases/os_mempool_test_case.c b/kernel/os/test/src/testcases/os_mempool_test_case.c
index f9e8ab219..a97f3d85c 100644
--- a/kernel/os/test/src/testcases/os_mempool_test_case.c
+++ b/kernel/os/test/src/testcases/os_mempool_test_case.c
@@ -6,7 +6,7 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
@@ -19,11 +19,11 @@
#include "os_test_priv.h"
/**
- * os mempool test
- *
- * Main test loop for memory pool testing.
- *
- * @return int
+ * os mempool test
+ *
+ * Main test loop for memory pool testing.
+ *
+ * @return int
*/
void
mempool_test(int num_blocks, int block_size)
@@ -31,7 +31,6 @@ 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;
@@ -40,7 +39,7 @@ mempool_test(int num_blocks, int block_size)
/* Check for too many blocks */
TEST_ASSERT(num_blocks <= MEMPOOL_TEST_MAX_BLOCKS);
- rc = os_mempool_init(&g_TstMempool, num_blocks, MEM_BLOCK_SIZE,
+ rc = os_mempool_init(&g_TstMempool, num_blocks, MEM_BLOCK_SIZE,
&TstMembuf[0], "TestMemPool");
TEST_ASSERT_FATAL(rc == 0, "Error creating memory pool %d", rc);
@@ -125,7 +124,7 @@ mempool_test(int num_blocks, int block_size)
}
}
- TEST_ASSERT((cnt == g_TstMempool.mp_num_blocks) &&
+ TEST_ASSERT((cnt == g_TstMempool.mp_num_blocks) &&
(cnt != MEMPOOL_TEST_MAX_BLOCKS),
"Got more blocks than mempool contains (%d vs %d)",
cnt, g_TstMempool.mp_num_blocks);
@@ -139,7 +138,7 @@ mempool_test(int num_blocks, int block_size)
for (cnt = 0; cnt < g_TstMempool.mp_num_blocks; ++cnt) {
rc = os_memblock_put(&g_TstMempool, block_array[cnt]);
TEST_ASSERT(rc == 0,
- "Error putting back block %p (cnt=%d err=%d)",
+ "Error putting back block %p (cnt=%d err=%d)",
block_array[cnt], cnt, rc);
}
@@ -158,21 +157,6 @@ 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");
}
TEST_CASE(os_mempool_test_case)
diff --git a/net/oic/src/api/oc_rep.c b/net/oic/src/api/oc_rep.c
index 64a1d0639..aab7d6fc0 100644
--- a/net/oic/src/api/oc_rep.c
+++ b/net/oic/src/api/oc_rep.c
@@ -72,6 +72,8 @@ static oc_rep_t *
_alloc_rep(void)
{
oc_rep_t *rep = os_memblock_get(&oc_rep_objects);
+
+ memset(rep, 0, sizeof(*rep));
#ifdef DEBUG
oc_assert(rep != NULL);
#endif
diff --git a/net/oic/src/api/oc_ri.c b/net/oic/src/api/oc_ri.c
index 6f85ae553..73556af54 100644
--- a/net/oic/src/api/oc_ri.c
+++ b/net/oic/src/api/oc_ri.c
@@ -796,6 +796,7 @@ oc_ri_invoke_client_cb(struct coap_packet_rx *rsp, oc_endpoint_t *endpoint)
if (oc_ri_process_discovery_payload(rsp, cb->handler,
endpoint) == OC_STOP_DISCOVERY) {
free_client_cb(cb);
+ return true;
}
} else {
client_response.packet = rsp;
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services