Merge pull request #3586 from daniel-k/fix/pktbuf_release_check_contains

ng_pktbuf_static: check if pkt is in buffer before derefencing in ng_pktbuf_release
RIOT-OS · Aug 17, 2015 · bce1cf6 · bce1cf6
2 parents b0ad14c + 12a3a65
commit bce1cf6
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 17 deletions.
diff --git a/sys/include/net/ng_pktbuf.h b/sys/include/net/ng_pktbuf.h
@@ -143,6 +143,8 @@ void ng_pktbuf_hold(ng_pktsnip_t *pkt, unsigned int num);
  * @brief   Decreases ng_pktsnip_t::users of @p pkt atomically and removes it if it
  *          reaches 0.
  *
+ * @pre All snips of @p pkt must be in the packet buffer.
+ *
  * @param[in] pkt   A packet.
  */
 void ng_pktbuf_release(ng_pktsnip_t *pkt);

diff --git a/sys/net/crosslayer/ng_pktbuf_static/ng_pktbuf_static.c b/sys/net/crosslayer/ng_pktbuf_static/ng_pktbuf_static.c
@@ -199,7 +199,9 @@ void ng_pktbuf_release(ng_pktsnip_t *pkt)
 {
     mutex_lock(&_mutex);
     while (pkt) {
-        ng_pktsnip_t *tmp = pkt->next;
+        ng_pktsnip_t *tmp;
+        assert(_pktbuf_contains(pkt));
+        tmp = pkt->next;
         if (pkt->users == 1) {
             pkt->users = 0; /* not necessary but to be on the safe side */
             _pktbuf_free(pkt->data, pkt->size);

diff --git a/tests/unittests/tests-pktbuf/tests-pktbuf.c b/tests/unittests/tests-pktbuf/tests-pktbuf.c
@@ -572,20 +572,6 @@ static void test_pktbuf_hold__success2(void)
     TEST_ASSERT_EQUAL_INT(TEST_UINT8 + 1, pkt->users);
 }
 
-static void test_pktbuf_release__pkt_null(void)
-{
-    ng_pktbuf_release(NULL);
-    TEST_ASSERT(ng_pktbuf_is_empty());
-}
-
-static void test_pktbuf_release__pkt_external(void)
-{
-    ng_pktsnip_t pkt = { 1, NULL, TEST_STRING8, sizeof(TEST_STRING8), NG_NETTYPE_TEST };
-
-    ng_pktbuf_release(&pkt);
-    TEST_ASSERT(ng_pktbuf_is_empty());
-}
-
 static void test_pktbuf_release__success(void)
 {
     ng_pktsnip_t *pkt = ng_pktbuf_add(NULL, TEST_STRING16, sizeof(TEST_STRING16), NG_NETTYPE_TEST);
@@ -733,8 +719,6 @@ Test *tests_pktbuf_tests(void)
         new_TestFixture(test_pktbuf_hold__pkt_external),
         new_TestFixture(test_pktbuf_hold__success),
         new_TestFixture(test_pktbuf_hold__success2),
-        new_TestFixture(test_pktbuf_release__pkt_null),
-        new_TestFixture(test_pktbuf_release__pkt_external),
         new_TestFixture(test_pktbuf_release__success),
         new_TestFixture(test_pktbuf_start_write__NULL),
         new_TestFixture(test_pktbuf_start_write__pkt_users_1),