From 62a2b270828ffd10249f2e3e94e94a67c06a2ac9 Mon Sep 17 00:00:00 2001 From: Ondrej Kosta Date: Tue, 13 Feb 2024 17:17:34 +0100 Subject: [PATCH] fix(esp_eth): improved SPI Ethernet _alloc_recv_buf error handling --- components/esp_eth/src/esp_eth_mac_dm9051.c | 45 +++++++++-------- .../esp_eth/src/esp_eth_mac_ksz8851snl.c | 45 +++++++++-------- components/esp_eth/src/esp_eth_mac_w5500.c | 50 +++++++++++-------- 3 files changed, 78 insertions(+), 62 deletions(-) diff --git a/components/esp_eth/src/esp_eth_mac_dm9051.c b/components/esp_eth/src/esp_eth_mac_dm9051.c index 06f0c0c81f..435b5a8557 100644 --- a/components/esp_eth/src/esp_eth_mac_dm9051.c +++ b/components/esp_eth/src/esp_eth_mac_dm9051.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -789,6 +789,7 @@ static void emac_dm9051_task(void *arg) { emac_dm9051_t *emac = (emac_dm9051_t *)arg; uint8_t status = 0; + esp_err_t ret; while (1) { // check if the task receives any notification if (ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(1000)) == 0 && // if no notification ... @@ -804,31 +805,35 @@ static void emac_dm9051_task(void *arg) /* define max expected frame len */ uint32_t frame_len = ETH_MAX_PACKET_SIZE; uint8_t *buffer; - dm9051_alloc_recv_buf(emac, &buffer, &frame_len); - /* we have memory to receive the frame of maximal size previously defined */ - if (buffer != NULL) { - uint32_t buf_len = DM9051_ETH_MAC_RX_BUF_SIZE_AUTO; - if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { - if (buf_len == 0) { + if ((ret = dm9051_alloc_recv_buf(emac, &buffer, &frame_len)) == ESP_OK) { + if (buffer != NULL) { + /* we have memory to receive the frame of maximal size previously defined */ + uint32_t buf_len = DM9051_ETH_MAC_RX_BUF_SIZE_AUTO; + if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { + if (buf_len == 0) { + dm9051_flush_recv_frame(emac); + free(buffer); + } else if (frame_len > buf_len) { + ESP_LOGE(TAG, "received frame was truncated"); + free(buffer); + } else { + ESP_LOGD(TAG, "receive len=%u", buf_len); + /* pass the buffer to stack (e.g. TCP/IP layer) */ + emac->eth->stack_input(emac->eth, buffer, buf_len); + } + } else { + ESP_LOGE(TAG, "frame read from module failed"); dm9051_flush_recv_frame(emac); free(buffer); - } else if (frame_len > buf_len) { - ESP_LOGE(TAG, "received frame was truncated"); - free(buffer); - } else { - ESP_LOGD(TAG, "receive len=%u", buf_len); - /* pass the buffer to stack (e.g. TCP/IP layer) */ - emac->eth->stack_input(emac->eth, buffer, buf_len); } - } else { - ESP_LOGE(TAG, "frame read from module failed"); - dm9051_flush_recv_frame(emac); - free(buffer); + } else if (frame_len) { + ESP_LOGE(TAG, "invalid combination of frame_len(%u) and buffer pointer(%p)", frame_len, buffer); } - /* if allocation failed and there is a waiting frame */ - } else if (frame_len) { + } else if (ret == ESP_ERR_NO_MEM) { ESP_LOGE(TAG, "no mem for receive buffer"); dm9051_flush_recv_frame(emac); + } else { + ESP_LOGE(TAG, "unexpected error 0x%x", ret); } } while (emac->packets_remain); } diff --git a/components/esp_eth/src/esp_eth_mac_ksz8851snl.c b/components/esp_eth/src/esp_eth_mac_ksz8851snl.c index f19600a59e..a32fa1227d 100644 --- a/components/esp_eth/src/esp_eth_mac_ksz8851snl.c +++ b/components/esp_eth/src/esp_eth_mac_ksz8851snl.c @@ -3,7 +3,7 @@ * * SPDX-License-Identifier: MIT * - * SPDX-FileContributor: 2021-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileContributor: 2021-2024 Espressif Systems (Shanghai) CO LTD */ #include @@ -638,6 +638,7 @@ static esp_err_t emac_ksz8851_set_peer_pause_ability(esp_eth_mac_t *mac, uint32_ static void emac_ksz8851snl_task(void *arg) { emac_ksz8851snl_t *emac = (emac_ksz8851snl_t *)arg; + esp_err_t ret; while (1) { ulTaskNotifyTake(pdTRUE, portMAX_DELAY); @@ -691,31 +692,35 @@ static void emac_ksz8851snl_task(void *arg) /* define max expected frame len */ uint32_t frame_len = ETH_MAX_PACKET_SIZE; uint8_t *buffer; - emac_ksz8851_alloc_recv_buf(emac, &buffer, &frame_len); - /* we have memory to receive the frame of maximal size previously defined */ - if (buffer != NULL) { - uint32_t buf_len = KSZ8851_ETH_MAC_RX_BUF_SIZE_AUTO; - if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { - if (buf_len == 0) { + if ((ret = emac_ksz8851_alloc_recv_buf(emac, &buffer, &frame_len)) == ESP_OK) { + if (buffer != NULL) { + /* we have memory to receive the frame of maximal size previously defined */ + uint32_t buf_len = KSZ8851_ETH_MAC_RX_BUF_SIZE_AUTO; + if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { + if (buf_len == 0) { + emac_ksz8851_flush_recv_queue(emac); + free(buffer); + } else if (frame_len > buf_len) { + ESP_LOGE(TAG, "received frame was truncated"); + free(buffer); + } else { + ESP_LOGD(TAG, "receive len=%u", buf_len); + /* pass the buffer to stack (e.g. TCP/IP layer) */ + emac->eth->stack_input(emac->eth, buffer, buf_len); + } + } else { + ESP_LOGE(TAG, "frame read from module failed"); emac_ksz8851_flush_recv_queue(emac); free(buffer); - } else if (frame_len > buf_len) { - ESP_LOGE(TAG, "received frame was truncated"); - free(buffer); - } else { - ESP_LOGD(TAG, "receive len=%u", buf_len); - /* pass the buffer to stack (e.g. TCP/IP layer) */ - emac->eth->stack_input(emac->eth, buffer, buf_len); } - } else { - ESP_LOGE(TAG, "frame read from module failed"); - emac_ksz8851_flush_recv_queue(emac); - free(buffer); + } else if (frame_len) { + ESP_LOGE(TAG, "invalid combination of frame_len(%u) and buffer pointer(%p)", frame_len, buffer); } - /* if allocation failed and there is a waiting frame */ - } else if (frame_len) { + } else if (ret == ESP_ERR_NO_MEM) { ESP_LOGE(TAG, "no mem for receive buffer"); emac_ksz8851_flush_recv_queue(emac); + } else { + ESP_LOGE(TAG, "unexpected error 0x%x", ret); } } ksz8851_write_reg(emac, KSZ8851_IER, ier); diff --git a/components/esp_eth/src/esp_eth_mac_w5500.c b/components/esp_eth/src/esp_eth_mac_w5500.c index cc8a572500..e58cedd454 100644 --- a/components/esp_eth/src/esp_eth_mac_w5500.c +++ b/components/esp_eth/src/esp_eth_mac_w5500.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -585,7 +585,7 @@ static esp_err_t emac_w5500_receive(esp_eth_mac_t *mac, uint8_t *buf, uint32_t * remain_bytes -= rx_len + 2; emac->packets_remain = remain_bytes > 0; - *length = rx_len; + *length = copy_len; return ret; err: *length = 0; @@ -608,12 +608,13 @@ static esp_err_t emac_w5500_flush_recv_frame(emac_w5500_t *emac) // read head first ESP_GOTO_ON_ERROR(w5500_read_buffer(emac, &rx_len, sizeof(rx_len), offset), err, TAG, "read frame header failed"); // update read pointer - offset = rx_len; + rx_len = __builtin_bswap16(rx_len); + offset += rx_len; + offset = __builtin_bswap16(offset); ESP_GOTO_ON_ERROR(w5500_write(emac, W5500_REG_SOCK_RX_RD(0), &offset, sizeof(offset)), err, TAG, "write RX RD failed"); /* issue RECV command */ ESP_GOTO_ON_ERROR(w5500_send_command(emac, W5500_SCR_RECV, 100), err, TAG, "issue RECV command failed"); // check if there're more data need to process - rx_len = __builtin_bswap16(rx_len); remain_bytes -= rx_len; emac->packets_remain = remain_bytes > 0; } @@ -639,6 +640,7 @@ static void emac_w5500_task(void *arg) uint8_t *buffer = NULL; uint32_t frame_len = 0; uint32_t buf_len = 0; + esp_err_t ret; while (1) { /* check if the task receives any notification */ if (ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(1000)) == 0 && // if no notification ... @@ -655,29 +657,33 @@ static void emac_w5500_task(void *arg) do { /* define max expected frame len */ frame_len = ETH_MAX_PACKET_SIZE; - emac_w5500_alloc_recv_buf(emac, &buffer, &frame_len); - /* we have memory to receive the frame of maximal size previously defined */ - if (buffer != NULL) { - buf_len = W5500_ETH_MAC_RX_BUF_SIZE_AUTO; - if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { - if (buf_len == 0) { - free(buffer); - } else if (frame_len > buf_len) { - ESP_LOGE(TAG, "received frame was truncated"); - free(buffer); + if ((ret = emac_w5500_alloc_recv_buf(emac, &buffer, &frame_len)) == ESP_OK) { + if (buffer != NULL) { + /* we have memory to receive the frame of maximal size previously defined */ + buf_len = W5500_ETH_MAC_RX_BUF_SIZE_AUTO; + if (emac->parent.receive(&emac->parent, buffer, &buf_len) == ESP_OK) { + if (buf_len == 0) { + free(buffer); + } else if (frame_len > buf_len) { + ESP_LOGE(TAG, "received frame was truncated"); + free(buffer); + } else { + ESP_LOGD(TAG, "receive len=%u", buf_len); + /* pass the buffer to stack (e.g. TCP/IP layer) */ + emac->eth->stack_input(emac->eth, buffer, buf_len); + } } else { - ESP_LOGD(TAG, "receive len=%u", buf_len); - /* pass the buffer to stack (e.g. TCP/IP layer) */ - emac->eth->stack_input(emac->eth, buffer, buf_len); + ESP_LOGE(TAG, "frame read from module failed"); + free(buffer); } - } else { - ESP_LOGE(TAG, "frame read from module failed"); - free(buffer); + } else if (frame_len) { + ESP_LOGE(TAG, "invalid combination of frame_len(%u) and buffer pointer(%p)", frame_len, buffer); } - /* if allocation failed and there is a waiting frame */ - } else if (frame_len) { + } else if (ret == ESP_ERR_NO_MEM) { ESP_LOGE(TAG, "no mem for receive buffer"); emac_w5500_flush_recv_frame(emac); + } else { + ESP_LOGE(TAG, "unexpected error 0x%x", ret); } } while (emac->packets_remain); }