[riot-notifications] [RIOT-OS/RIOT] Initial implementation of IEEE 802.15.4 security (#15150)

Martine Lenders notifications at github.com
Wed Nov 4 15:03:47 CET 2020


@miri64 requested changes on this pull request.

Sorry for stepping in so late. I tried to find the reason for the increased stack size, but then found an issue in the `gnrc_pktbuf` usage.

>      };
-
+#if IS_USED(MODULE_IEEE802154_SECURITY)
+    gnrc_pktbuf_merge(pkt->next);
+    iolist_payload.iol_next = (iolist_t *)pkt->next->next,
+    iolist_payload.iol_base = pkt->next->data,
+    iolist_payload.iol_len = pkt->next->size;
+    uint8_t hdr_len = res;
+    uint8_t mic_len;
+    uint8_t mic[IEEE802154_MAC_SIZE];

This might be the reason for the increased stack usage (but did not actually confirm).

>      };
-
+#if IS_USED(MODULE_IEEE802154_SECURITY)
+    gnrc_pktbuf_merge(pkt->next);

Due tot his write action, `pkt->next` needs to be write-protected first (see [doc of that function](https://doc.riot-os.org/group__net__gnrc__pktbuf.html#gae41835e987f693706a388a3d9e14f83a). To my knowledge there was no write protection issued granted before in the send operation, as it usually just copies data over from the snip (and releases of course, but those do not need to be write-protected). Also, the function should be checked for errors and return as such.

```suggestion
    /* write protect `pkt` to set `pkt->next` */
    gnrc_pktsnip_t *tmp = gnrc_pktbuf_start_write(pkt);
    if (tmp == NULL) {
        DEBUG("...");
        return -ENOMEM;
    }
    pkt = tmp;
    tmp = gnrc_pktbuf_start_write(pkt->next);
    if (tmp == NULL) {
        DEBUG("...");
        return -ENOMEM;
    }
    pkt->next = tmp->next;
    res = gnrc_pktbuf_merge(pkt->next);
    if (res < 0) {
        return res;
    }
```

I am sure there is a nicer way to code this, but I wanted to get this out quickly before this PR is merged.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/15150#pullrequestreview-523379638
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20201104/9f2884ab/attachment.htm>


More information about the notifications mailing list