From 45d28fd14317508e9784fc90d35dd333bd832269 Mon Sep 17 00:00:00 2001 From: Andrzej Kaczmarek Date: Tue, 23 Jul 2019 03:43:57 +0200 Subject: [PATCH] nimble/phy: Fix memblock overwrite on PDU copy Optimized ble_phy_rxpdu_copy (f19002e9) can overwrite header of adjacent block at specific payload lengths. This happens if length of data that should be copied to last mbuf in chain is less than 4 bytes of data. In such case the last but one mbuf is already full when remaining data are appended to it beyond its data buffer effectively overwriting adjacent buffer. We should always check if there is enough space in current mbuf to copy data. If not, we just advance to next mbuf in chain. Even in such case there will be free mbuf to store remaining few bytes of data as this is guaranteed by ble_ll_rxpdu_alloc. --- nimble/drivers/nrf51/src/ble_phy.c | 13 ++++++++----- nimble/drivers/nrf52/src/ble_phy.c | 5 ++++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/nimble/drivers/nrf51/src/ble_phy.c b/nimble/drivers/nrf51/src/ble_phy.c index baa0208f2..1ce3ed923 100644 --- a/nimble/drivers/nrf51/src/ble_phy.c +++ b/nimble/drivers/nrf51/src/ble_phy.c @@ -210,6 +210,7 @@ ble_phy_rxpdu_copy(uint8_t *dptr, struct os_mbuf *rxpdu) uint32_t rem_len; uint32_t copy_len; uint32_t block_len; + uint32_t block_rem_len; void *dst; void *src; struct os_mbuf * om; @@ -234,12 +235,14 @@ ble_phy_rxpdu_copy(uint8_t *dptr, struct os_mbuf *rxpdu) * Always copy blocks of length aligned to word size, only last mbuf * will have remaining non-word size bytes appended. */ + block_rem_len = copy_len; copy_len = min(copy_len, rem_len); copy_len &= ~3; dst = om->om_data; om->om_len = copy_len; rem_len -= copy_len; + block_rem_len -= copy_len; __asm__ volatile (".syntax unified \n" " mov r4, %[len] \n" @@ -250,13 +253,13 @@ ble_phy_rxpdu_copy(uint8_t *dptr, struct os_mbuf *rxpdu) " bpl 1b \n" " adds %[src], %[src], r4 \n" " adds %[dst], %[dst], r4 \n" - : [dst] "+r" (dst), [src] "+r" (src), - [len] "+r" (copy_len) + : [dst] "+l" (dst), [src] "+l" (src), + [len] "+l" (copy_len) : : "r3", "r4", "memory" ); - if (rem_len < 4) { + if ((rem_len < 4) && (block_rem_len >= rem_len)) { break; } @@ -273,8 +276,8 @@ ble_phy_rxpdu_copy(uint8_t *dptr, struct os_mbuf *rxpdu) " strb r3, [%[dst], %[len]] \n" "2: subs %[len], #1 \n" " bpl 1b \n" - : [len] "+r" (rem_len) - : [dst] "r" (dst), [src] "r" (src) + : [len] "+l" (rem_len) + : [dst] "l" (dst), [src] "l" (src) : "r3", "memory" ); diff --git a/nimble/drivers/nrf52/src/ble_phy.c b/nimble/drivers/nrf52/src/ble_phy.c index e49c7a180..7a499fd98 100644 --- a/nimble/drivers/nrf52/src/ble_phy.c +++ b/nimble/drivers/nrf52/src/ble_phy.c @@ -400,6 +400,7 @@ ble_phy_rxpdu_copy(uint8_t *dptr, struct os_mbuf *rxpdu) uint32_t rem_len; uint32_t copy_len; uint32_t block_len; + uint32_t block_rem_len; void *dst; void *src; struct os_mbuf * om; @@ -424,12 +425,14 @@ ble_phy_rxpdu_copy(uint8_t *dptr, struct os_mbuf *rxpdu) * Always copy blocks of length aligned to word size, only last mbuf * will have remaining non-word size bytes appended. */ + block_rem_len = copy_len; copy_len = min(copy_len, rem_len); copy_len &= ~3; dst = om->om_data; om->om_len = copy_len; rem_len -= copy_len; + block_rem_len -= copy_len; __asm__ volatile (".syntax unified \n" " mov r4, %[len] \n" @@ -446,7 +449,7 @@ ble_phy_rxpdu_copy(uint8_t *dptr, struct os_mbuf *rxpdu) : "r3", "r4", "memory" ); - if (rem_len < 4) { + if ((rem_len < 4) && (block_rem_len >= rem_len)) { break; }