Merge pull request #9434 from miri64/gnrc_pktbuf_static/fix/alignment-race

gnrc_pktbuf_static: fix alignment issue / leaks

sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c

@@ -33,7 +33,7 @@
 #define ENABLE_DEBUG (0)
 #include "debug.h"
 
-#define _ALIGNMENT_MASK    (sizeof(void *) - 1)
+#define _ALIGNMENT_MASK    (sizeof(_unused_t) - 1)
 
 typedef struct _unused {
     struct _unused *next;
@@ -108,8 +108,7 @@ gnrc_pktsnip_t *gnrc_pktbuf_mark(gnrc_pktsnip_t *pkt, size_t size, gnrc_nettype_
 {
     gnrc_pktsnip_t *marked_snip;
     /* size required for chunk */
-    size_t required_new_size = (size < sizeof(_unused_t)) ?
-                               _align(sizeof(_unused_t)) : _align(size);
+    size_t required_new_size = _align(size);
     void *new_data_marked;
 
     mutex_lock(&_mutex);
@@ -130,8 +129,7 @@ gnrc_pktsnip_t *gnrc_pktbuf_mark(gnrc_pktsnip_t *pkt, size_t size, gnrc_nettype_
     }
     /* marked data would not fit _unused_t marker => move data around to allow
      * for proper free */
-    if ((pkt->size != size) &&
-        ((size < required_new_size) || ((pkt->size - size) < sizeof(_unused_t)))) {
+    if ((pkt->size != size) && (size < required_new_size)) {
         void *new_data_rest;
         new_data_marked = _pktbuf_alloc(size);
         if (new_data_marked == NULL) {
@@ -169,8 +167,7 @@ gnrc_pktsnip_t *gnrc_pktbuf_mark(gnrc_pktsnip_t *pkt, size_t size, gnrc_nettype_
 
 int gnrc_pktbuf_realloc_data(gnrc_pktsnip_t *pkt, size_t size)
 {
-    size_t aligned_size = (size < sizeof(_unused_t)) ?
-                          _align(sizeof(_unused_t)) : _align(size);
+    size_t aligned_size = _align(size);
 
     mutex_lock(&_mutex);
     assert(pkt != NULL);
@@ -189,8 +186,7 @@ int gnrc_pktbuf_realloc_data(gnrc_pktsnip_t *pkt, size_t size)
         pkt->data = NULL;
     }
     /* if new size is bigger than old size */
-    else if ((size > pkt->size) ||                          /* new size does not fit */
-        ((pkt->size - aligned_size) < sizeof(_unused_t))) { /* resulting hole would not fit marker */
+    else if (size > pkt->size) {    /* new size does not fit */
         void *new_data = _pktbuf_alloc(size);
         if (new_data == NULL) {
             DEBUG("pktbuf: error allocating new data section\n");
@@ -396,7 +392,7 @@ static void *_pktbuf_alloc(size_t size)
 {
     _unused_t *prev = NULL, *ptr = _first_unused;
 
-    size = (size < sizeof(_unused_t)) ? _align(sizeof(_unused_t)) : _align(size);
+    size = _align(size);
     while (ptr && (size > ptr->size)) {
         prev = ptr;
         ptr = ptr->next;
@@ -466,11 +462,11 @@ static void _pktbuf_free(void *data, size_t size)
         ptr = ptr->next;
     }
     new->next = ptr;
-    new->size = (size < sizeof(_unused_t)) ? _align(sizeof(_unused_t)) : _align(size);
+    new->size = _align(size);
     /* calculate number of bytes between new _unused_t chunk and end of packet
      * buffer */
     bytes_at_end = ((&_pktbuf[0] + GNRC_PKTBUF_SIZE) - (((uint8_t *)new) + new->size));
-    if (bytes_at_end < _align(sizeof(_unused_t))) {
+    if (bytes_at_end < sizeof(_unused_t)) {
         /* new is very last segment and there is a little bit of memory left
          * that wouldn't fit _unused_t (cut of in _pktbuf_alloc()) => re-add it */
         new->size += bytes_at_end;

tests/gnrc_ndp/main.c

@@ -79,7 +79,9 @@ static void set_up(void)
 static void fill_pktbuf(void)
 {
     gnrc_pktsnip_t *pkt = gnrc_pktbuf_add(NULL, NULL,
-                                          GNRC_PKTBUF_SIZE - sizeof(gnrc_pktsnip_t),
+                                          /* 24 = sizeof(gnrc_pktsnip_t) +
+                                           * potential alignment */
+                                          GNRC_PKTBUF_SIZE - 24U,
                                           GNRC_NETTYPE_UNDEF);
     TEST_ASSERT_NOT_NULL(pkt);
     TEST_ASSERT(gnrc_pktbuf_is_sane());