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 2018/09/05 14:42:54 UTC

[GitHub] mkiiskila closed pull request #1383: kernel/os; add syscfg knob to add a guard element between mempool

mkiiskila closed pull request #1383: kernel/os; add syscfg knob to add a guard element between mempool
URL: https://github.com/apache/mynewt-core/pull/1383
 
 
   

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_mempool.h b/kernel/os/include/os/os_mempool.h
index 6624915677..8cfcf7f3a9 100644
--- a/kernel/os/include/os/os_mempool.h
+++ b/kernel/os/include/os/os_mempool.h
@@ -146,11 +146,21 @@ struct os_mempool *os_mempool_info_get_next(struct os_mempool *,
  * is NOT in bytes! The size is the number of os_membuf_t elements required for
  * the memory pool.
  */
+#if MYNEWT_VAL(OS_MEMPOOL_GUARD)
+/*
+ * Leave extra 4 bytes of guard area at the end.
+ */
+#define OS_MEMPOOL_BLOCK_SZ(sz) ((sz) + sizeof(os_membuf_t))
+#else
+#define OS_MEMPOOL_BLOCK_SZ(sz) (sz)
+#endif
 #if (OS_CFG_ALIGNMENT == OS_CFG_ALIGN_4)
-#define OS_MEMPOOL_SIZE(n,blksize)      ((((blksize) + 3) / 4) * (n))
+#define OS_MEMPOOL_SIZE(n, blksize)                                     \
+    (((OS_MEMPOOL_BLOCK_SZ(blksize) + 3) / 4) * (n))
 typedef uint32_t os_membuf_t;
 #else
-#define OS_MEMPOOL_SIZE(n,blksize)      ((((blksize) + 7) / 8) * (n))
+#define OS_MEMPOOL_SIZE(n, blksize)                                     \
+    (((OS_MEMPOOL_BLOCK_SZ(blksize) + 7) / 8) * (n))
 typedef uint64_t os_membuf_t;
 #endif
 
diff --git a/kernel/os/src/os_mempool.c b/kernel/os/src/os_mempool.c
index 10a5aa391f..14aaa70e32 100644
--- a/kernel/os/src/os_mempool.c
+++ b/kernel/os/src/os_mempool.c
@@ -27,7 +27,14 @@
 #include "os/mynewt.h"
 
 #define OS_MEM_TRUE_BLOCK_SIZE(bsize)   OS_ALIGN(bsize, OS_ALIGNMENT)
+#if MYNEWT_VAL(OS_MEMPOOL_GUARD)
+#define OS_MEMPOOL_TRUE_BLOCK_SIZE(mp)                                  \
+    (((mp)->mp_flags & OS_MEMPOOL_F_EXT) ?                              \
+      OS_MEM_TRUE_BLOCK_SIZE(mp->mp_block_size) :                       \
+      (OS_MEM_TRUE_BLOCK_SIZE(mp->mp_block_size) + sizeof(os_membuf_t)))
+#else
 #define OS_MEMPOOL_TRUE_BLOCK_SIZE(mp) OS_MEM_TRUE_BLOCK_SIZE(mp->mp_block_size)
+#endif
 
 STAILQ_HEAD(, os_mempool) g_os_mempool_list =
     STAILQ_HEAD_INITIALIZER(g_os_mempool_list);
@@ -39,14 +46,13 @@ _Static_assert(sizeof(struct os_memblock) % 4 == 0, "sizeof(struct os_memblock)
 _Static_assert(sizeof(os_mem_poison) == 4, "sizeof(os_mem_poison) shall be 4");
 
 static void
-os_mempool_poison(void *start, int sz)
+os_mempool_poison(const struct os_mempool *mp, void *start)
 {
     uint32_t *p;
     uint32_t *end;
+    int sz;
 
-    assert(((uintptr_t)start & 0x03) == 0);
-    assert((sz & 0x03) == 0);
-
+    sz = OS_MEM_TRUE_BLOCK_SIZE(mp->mp_block_size);
     p = start;
     end = p + sz / 4;
     p += sizeof(struct os_memblock) / 4;
@@ -58,14 +64,13 @@ os_mempool_poison(void *start, int sz)
 }
 
 static void
-os_mempool_poison_check(void *start, int sz)
+os_mempool_poison_check(const struct os_mempool *mp, void *start)
 {
     uint32_t *p;
     uint32_t *end;
+    int sz;
 
-    assert(((uintptr_t)start & 0x03) == 0);
-    assert((sz & 0x03) == 0);
-
+    sz = OS_MEM_TRUE_BLOCK_SIZE(mp->mp_block_size);
     p = start;
     end = p + sz / 4;
     p += sizeof(struct os_memblock) / 4;
@@ -76,13 +81,44 @@ os_mempool_poison_check(void *start, int sz)
     }
 }
 #else
-#define os_mempool_poison(start, sz)
-#define os_mempool_poison_check(start, sz)
+#define os_mempool_poison(mp, start)
+#define os_mempool_poison_check(mp, start)
 #endif
+#if MYNEWT_VAL(OS_MEMPOOL_GUARD)
+#define OS_MEMPOOL_GUARD_PATTERN 0xBAFF1ED1
 
-os_error_t
-os_mempool_init(struct os_mempool *mp, uint16_t blocks, uint32_t block_size,
-                void *membuf, char *name)
+static void
+os_mempool_guard(const struct os_mempool *mp, void *start)
+{
+    uint32_t *tgt;
+
+    if ((mp->mp_flags & OS_MEMPOOL_F_EXT) == 0) {
+        tgt = (uint32_t *)((uintptr_t)start +
+                                     OS_MEM_TRUE_BLOCK_SIZE(mp->mp_block_size));
+        *tgt = OS_MEMPOOL_GUARD_PATTERN;
+    }
+}
+
+static void
+os_mempool_guard_check(const struct os_mempool *mp, void *start)
+{
+    uint32_t *tgt;
+
+    if ((mp->mp_flags & OS_MEMPOOL_F_EXT) == 0) {
+        tgt = (uint32_t *)((uintptr_t)start +
+                                     OS_MEM_TRUE_BLOCK_SIZE(mp->mp_block_size));
+        assert(*tgt == OS_MEMPOOL_GUARD_PATTERN);
+    }
+}
+#else
+#define os_mempool_guard(mp, start)
+#define os_mempool_guard_check(mp, start)
+#endif
+
+static os_error_t
+os_mempool_init_internal(struct os_mempool *mp, uint16_t blocks,
+                         uint32_t block_size, void *membuf, char *name,
+                         uint8_t flags)
 {
     int true_block_size;
     uint8_t *block_addr;
@@ -105,25 +141,28 @@ os_mempool_init(struct os_mempool *mp, uint16_t blocks, uint32_t block_size,
             return OS_MEM_NOT_ALIGNED;
         }
     }
-    true_block_size = OS_MEM_TRUE_BLOCK_SIZE(block_size);
 
     /* Initialize the memory pool structure */
     mp->mp_block_size = block_size;
     mp->mp_num_free = blocks;
     mp->mp_min_free = blocks;
-    mp->mp_flags = 0;
+    mp->mp_flags = flags;
     mp->mp_num_blocks = blocks;
     mp->mp_membuf_addr = (uint32_t)membuf;
     mp->name = name;
-    os_mempool_poison(membuf, true_block_size);
+    os_mempool_poison(mp, membuf);
+    os_mempool_guard(mp, membuf);
     SLIST_FIRST(mp) = membuf;
 
+    true_block_size = OS_MEMPOOL_TRUE_BLOCK_SIZE(mp);
+
     /* Chain the memory blocks to the free list */
     block_addr = (uint8_t *)membuf;
     block_ptr = (struct os_memblock *)block_addr;
     while (blocks > 1) {
         block_addr += true_block_size;
-        os_mempool_poison(block_addr, true_block_size);
+        os_mempool_poison(mp, block_addr);
+        os_mempool_guard(mp, block_addr);
         SLIST_NEXT(block_ptr, mb_next) = (struct os_memblock *)block_addr;
         block_ptr = (struct os_memblock *)block_addr;
         --blocks;
@@ -137,18 +176,25 @@ os_mempool_init(struct os_mempool *mp, uint16_t blocks, uint32_t block_size,
     return OS_OK;
 }
 
+os_error_t
+os_mempool_init(struct os_mempool *mp, uint16_t blocks, uint32_t block_size,
+                void *membuf, char *name)
+{
+    return os_mempool_init_internal(mp, blocks, block_size, membuf, name, 0);
+}
+
 os_error_t
 os_mempool_ext_init(struct os_mempool_ext *mpe, uint16_t blocks,
                     uint32_t block_size, void *membuf, char *name)
 {
     int rc;
 
-    rc = os_mempool_init(&mpe->mpe_mp, blocks, block_size, membuf, name);
+    rc = os_mempool_init_internal(&mpe->mpe_mp, blocks, block_size, membuf,
+                                  name, OS_MEMPOOL_F_EXT);
     if (rc != 0) {
         return rc;
     }
 
-    mpe->mpe_mp.mp_flags = OS_MEMPOOL_F_EXT;
     mpe->mpe_put_cb = NULL;
     mpe->mpe_put_arg = NULL;
 
@@ -205,12 +251,13 @@ os_mempool_clear(struct os_mempool *mp)
         return OS_INVALID_PARM;
     }
 
-    true_block_size = OS_MEM_TRUE_BLOCK_SIZE(mp->mp_block_size);
+    true_block_size = OS_MEMPOOL_TRUE_BLOCK_SIZE(mp);
 
     /* cleanup the memory pool structure */
     mp->mp_num_free = mp->mp_num_blocks;
     mp->mp_min_free = mp->mp_num_blocks;
-    os_mempool_poison((void *)mp->mp_membuf_addr, true_block_size);
+    os_mempool_poison(mp, (void *)mp->mp_membuf_addr);
+    os_mempool_guard(mp, (void *)mp->mp_membuf_addr);
     SLIST_FIRST(mp) = (void *)mp->mp_membuf_addr;
 
     /* Chain the memory blocks to the free list */
@@ -220,7 +267,8 @@ os_mempool_clear(struct os_mempool *mp)
 
     while (blocks > 1) {
         block_addr += true_block_size;
-        os_mempool_poison(block_addr, true_block_size);
+        os_mempool_poison(mp, block_addr);
+        os_mempool_guard(mp, block_addr);
         SLIST_NEXT(block_ptr, mb_next) = (struct os_memblock *)block_addr;
         block_ptr = (struct os_memblock *)block_addr;
         --blocks;
@@ -242,7 +290,8 @@ 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));
+        os_mempool_poison_check(mp, block);
+        os_mempool_guard_check(mp, block);
     }
 
     return true;
@@ -304,7 +353,8 @@ os_memblock_get(struct os_mempool *mp)
         OS_EXIT_CRITICAL(sr);
 
         if (block) {
-            os_mempool_poison_check(block, OS_MEMPOOL_TRUE_BLOCK_SIZE(mp));
+            os_mempool_poison_check(mp, block);
+            os_mempool_guard_check(mp, block);
         }
     }
 
@@ -322,7 +372,8 @@ os_memblock_put_from_cb(struct os_mempool *mp, void *block_addr)
     os_trace_api_u32x2(OS_TRACE_ID_MEMBLOCK_PUT_FROM_CB, (uint32_t)mp,
                        (uint32_t)block_addr);
 
-    os_mempool_poison(block_addr, OS_MEMPOOL_TRUE_BLOCK_SIZE(mp));
+    os_mempool_guard_check(mp, block_addr);
+    os_mempool_poison(mp, block_addr);
 
     block = (struct os_memblock *)block_addr;
     OS_ENTER_CRITICAL(sr);
@@ -371,7 +422,6 @@ os_memblock_put(struct os_mempool *mp, void *block_addr)
         assert(block != (struct os_memblock *)block_addr);
     }
 #endif
-
     /* If this is an extended mempool with a put callback, call the callback
      * instead of freeing the block directly.
      */
diff --git a/kernel/os/syscfg.yml b/kernel/os/syscfg.yml
index 0981a73bc6..668140b365 100644
--- a/kernel/os/syscfg.yml
+++ b/kernel/os/syscfg.yml
@@ -55,6 +55,9 @@ syscfg.defs:
     OS_MEMPOOL_POISON:
         description: 'Whether to do write known pattern to freed memory'
         value: 0
+    OS_MEMPOOL_GUARD:
+        description: 'Insert guard area at the end of mempool'
+        value: 0
     OS_CPUTIME_FREQ:
         description: 'Frequency of os cputime'
         value: 1000000


 

----------------------------------------------------------------
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