From 547f8e7aaaf57516749636dc884401838eae439a Mon Sep 17 00:00:00 2001 From: Billy Kozak Date: Mon, 20 Jul 2015 10:18:02 -0600 Subject: [PATCH 1/3] Fixed race condition with rf cpu in read_frame Fixed a race condition that could occur in read_frame because the dataEntry is set to PENDING before we are finished reading from it. --- cpu/cc26xx/dev/cc26xx-rf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cpu/cc26xx/dev/cc26xx-rf.c b/cpu/cc26xx/dev/cc26xx-rf.c index ea12aa0d3..b26771a8a 100644 --- a/cpu/cc26xx/dev/cc26xx-rf.c +++ b/cpu/cc26xx/dev/cc26xx-rf.c @@ -1334,8 +1334,6 @@ read_frame(void *buf, unsigned short buf_len) int len = 0; if(GET_FIELD_V(rx_read_entry, dataEntry, status) == DATA_ENTRY_STATUS_FINISHED) { - /* Set status to 0 "Pending" in element */ - GET_FIELD_V(rx_read_entry, dataEntry, status) = DATA_ENTRY_STATUS_PENDING; if(rx_read_entry[8] > 0) { memcpy(buf, (char *)&rx_read_entry[9], buf_len); @@ -1352,6 +1350,9 @@ read_frame(void *buf, unsigned short buf_len) rx_read_entry[8] = 0; } + /* Set status to 0 "Pending" in element */ + GET_FIELD_V(rx_read_entry, dataEntry, status) = DATA_ENTRY_STATUS_PENDING; + /* Move read entry pointer to next entry */ rx_read_entry = GET_FIELD_V(rx_read_entry, dataEntry, pNextEntry); } From ceb24f656e6af5e24e36008b5a2f8b386db857a8 Mon Sep 17 00:00:00 2001 From: Billy Kozak Date: Mon, 20 Jul 2015 10:28:24 -0600 Subject: [PATCH 2/3] Improved style of read_frame - moved variable declaration to top of function in accordance with the Contiki style guide - made function flatter, reduced nesting to improve readability --- cpu/cc26xx/dev/cc26xx-rf.c | 54 ++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/cpu/cc26xx/dev/cc26xx-rf.c b/cpu/cc26xx/dev/cc26xx-rf.c index b26771a8a..efeee8450 100644 --- a/cpu/cc26xx/dev/cc26xx-rf.c +++ b/cpu/cc26xx/dev/cc26xx-rf.c @@ -1328,35 +1328,45 @@ send(const void *payload, unsigned short payload_len) return transmit(payload_len); } /*---------------------------------------------------------------------------*/ +static void +release_data_entry(void) +{ + /* Clear the length byte */ + rx_read_entry[8] = 0; + /* Set status to 0 "Pending" in element */ + GET_FIELD_V(rx_read_entry, dataEntry, status) = DATA_ENTRY_STATUS_PENDING; + rx_read_entry = GET_FIELD_V(rx_read_entry, dataEntry, pNextEntry); +} +/*---------------------------------------------------------------------------*/ static int read_frame(void *buf, unsigned short buf_len) { + int8_t rssi; int len = 0; + uint8_t status = GET_FIELD_V(rx_read_entry, dataEntry, status); - if(GET_FIELD_V(rx_read_entry, dataEntry, status) == DATA_ENTRY_STATUS_FINISHED) { - - if(rx_read_entry[8] > 0) { - memcpy(buf, (char *)&rx_read_entry[9], buf_len); - - /* Remove the footer */ - len = MIN(buf_len, rx_read_entry[8] - 4); - - int rssi = (int8_t)rx_read_entry[9 + len + 2]; - - packetbuf_set_attr(PACKETBUF_ATTR_RSSI, rssi); - RIMESTATS_ADD(llrx); - - /* Clear the length byte */ - rx_read_entry[8] = 0; - } - - /* Set status to 0 "Pending" in element */ - GET_FIELD_V(rx_read_entry, dataEntry, status) = DATA_ENTRY_STATUS_PENDING; - - /* Move read entry pointer to next entry */ - rx_read_entry = GET_FIELD_V(rx_read_entry, dataEntry, pNextEntry); + if(status != DATA_ENTRY_STATUS_FINISHED) { + /* No available data */ + return 0; } + if(!rx_read_entry[8]) { + release_data_entry(); + return 0; + } + + memcpy(buf, (char *)&rx_read_entry[9], buf_len); + + /* Remove the footer */ + len = MIN(buf_len, rx_read_entry[8] - 4); + + rssi = (int8_t)rx_read_entry[9 + len + 2]; + + packetbuf_set_attr(PACKETBUF_ATTR_RSSI, rssi); + RIMESTATS_ADD(llrx); + + release_data_entry(); + return len; } /*---------------------------------------------------------------------------*/ From feec05cdf21af047820987eb4217e2936045c86b Mon Sep 17 00:00:00 2001 From: Billy Kozak Date: Mon, 20 Jul 2015 12:02:22 -0600 Subject: [PATCH 3/3] CC26xx - fix misuse of len variable in read_frame read_frame was misuing the packet length in the following ways: - returning non-zero even if buf_len is too short for the packet - truncating the length to buf_len if len is too long then using the truncated (i.e. wrong) length to index into the buffer - memcpying too many bytes (used buf_len instead of real length) This commit fixes all of this and adds some code to report on packet length errors (to match with cc2538 driver). --- cpu/cc26xx/dev/cc26xx-rf.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/cpu/cc26xx/dev/cc26xx-rf.c b/cpu/cc26xx/dev/cc26xx-rf.c index efeee8450..8514b7e79 100644 --- a/cpu/cc26xx/dev/cc26xx-rf.c +++ b/cpu/cc26xx/dev/cc26xx-rf.c @@ -1350,15 +1350,26 @@ read_frame(void *buf, unsigned short buf_len) return 0; } - if(!rx_read_entry[8]) { + + if(rx_read_entry[8] < 4) { + PRINTF("RF: too short\n"); + RIMESTATS_ADD(tooshort); + release_data_entry(); return 0; } - memcpy(buf, (char *)&rx_read_entry[9], buf_len); + len = rx_read_entry[8] - 4; - /* Remove the footer */ - len = MIN(buf_len, rx_read_entry[8] - 4); + if(len > buf_len) { + PRINTF("RF: too long\n"); + RIMESTATS_ADD(toolong); + + release_data_entry(); + return 0; + } + + memcpy(buf, (char *)&rx_read_entry[9], len); rssi = (int8_t)rx_read_entry[9 + len + 2];