Skip to content

gnrc_sixlowpan_iphc: check ptr != NULL in compressible check#11164

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:gnrc_sixlowpan_iphc/fix/check-NULL-ptr-for-compressible
Mar 26, 2019
Merged

gnrc_sixlowpan_iphc: check ptr != NULL in compressible check#11164
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:gnrc_sixlowpan_iphc/fix/check-NULL-ptr-for-compressible

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Mar 12, 2019 •
edited
Loading

Contribution description

Not only does this leave open a risk to crash the node for the check
in _compressible() but also is the tmp check below getting confused
when ptr is NULL, since gnrc_pktbuf_start_write() returns NULL
in that case.

Testing procedure

If #11161 was merged and #11163 is fixed you can just send a packet with empty payload like this in gnrc_networking on a 6Lo-capable device (e.g. samr21-xpro or iotlab-m3)

udp send <destination address> <port> ""

With this PR the packet should be received at the destination (if a server with that port is open), without it the packet will be silently dropped.

If the referenced issues / PRs are not merged / fixed, apply the following patch before flashing:

diff --git a/examples/gnrc_networking/udp.c b/examples/gnrc_networking/udp.c
index ada54e5..f52a9de 100644
--- a/examples/gnrc_networking/udp.c
+++ b/examples/gnrc_networking/udp.c
@@ -61,16 +61,20 @@ static void send(char *addr_str, char *port_str, char *data, unsigned int num,
     }
 
     for (unsigned int i = 0; i < num; i++) {
-        gnrc_pktsnip_t *payload, *udp, *ip;
-        unsigned payload_size;
-        /* allocate payload */
-        payload = gnrc_pktbuf_add(NULL, data, strlen(data), GNRC_NETTYPE_UNDEF);
-        if (payload == NULL) {
-            puts("Error: unable to copy data to packet buffer");
-            return;
+        gnrc_pktsnip_t *payload = NULL, *udp, *ip;
+        unsigned payload_size = 0;
+        size_t data_len = strlen(data);
+
+        if (data_len) {
+            /* allocate payload */
+            payload = gnrc_pktbuf_add(NULL, data, data_len, GNRC_NETTYPE_UNDEF);
+            if (payload == NULL) {
+                puts("Error: unable to copy data to packet buffer");
+                return;
+            }
+            /* store size for output */
+            payload_size = (unsigned)payload->size;
         }
-        /* store size for output */
-        payload_size = (unsigned)payload->size;
         /* allocate UDP header, set source port := destination port */
         udp = gnrc_udp_hdr_build(payload, port, port);
         if (udp == NULL) {

Issues/PRs references

Requires #11163 to be fixed and #11161 to be merged to test without patch.

Route to 6Lo minimal fragment forwarding

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 12, 2019
@miri64 miri64 requested review from bergzand and cgundogan March 12, 2019 15:07
@miri64 miri64 added this to the Release 2019.04 milestone Mar 12, 2019
@miri64 miri64 mentioned this pull request Mar 12, 2019
Merged
@miri64
gnrc_sixlowpan_iphc: check ptr != NULL in compressible check
3582ded 
Not only does this leave open a risk to crash the node for the check
in `_compressible()` but also is the `tmp` check below getting confused
when `ptr` is `NULL`, since `gnrc_pktbuf_start_write()` returns `NULL`
in that case.
@miri64 miri64 force-pushed the gnrc_sixlowpan_iphc/fix/check-NULL-ptr-for-compressible branch from 707bcea to 3582ded Compare March 12, 2019 15:48
@miri64 miri64 mentioned this pull request Mar 12, 2019
Merged
cgundogan
cgundogan approved these changes Mar 26, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sensible check. ACK!

@cgundogan cgundogan added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 26, 2019
@miri64 miri64 merged commit 116642f into RIOT-OS:master Mar 26, 2019
@miri64 miri64 deleted the gnrc_sixlowpan_iphc/fix/check-NULL-ptr-for-compressible branch March 26, 2019 16:49
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Mar 26, 2019

Thanks for the review :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgundogan cgundogan cgundogan approved these changes

@bergzand bergzand Awaiting requested review from bergzand

Assignees

No one assigned

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2019.04

Development

Successfully merging this pull request may close these issues.

2 participants

@miri64 @cgundogan