Przeglądaj źródła

Merge branch 'bugfix/emac_memory_leak' into 'master'

Fixed false positive emac rx memory leak reported by Coverity

Closes IDF-6363

See merge request espressif/esp-idf!21341
Ondrej Kosta 3 lat temu
rodzic
commit
84ab32b4ef

+ 18 - 20
components/esp_eth/src/esp_eth_mac_dm9051.c

@@ -820,32 +820,30 @@ static void emac_dm9051_task(void *arg)
                 uint32_t frame_len = ETH_MAX_PACKET_SIZE;
                 uint8_t *buffer;
                 dm9051_alloc_recv_buf(emac, &buffer, &frame_len);
-                /* there is a waiting frame */
-                if (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) {
-                                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");
+                /* 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) {
                             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, "no mem for receive buffer");
+                        ESP_LOGE(TAG, "frame read from module failed");
                         dm9051_flush_recv_frame(emac);
+                        free(buffer);
                     }
+                /* if allocation failed and there is a waiting frame */
+                } else if (frame_len) {
+                    ESP_LOGE(TAG, "no mem for receive buffer");
+                    dm9051_flush_recv_frame(emac);
                 }
             } while (emac->packets_remain);
         }

+ 17 - 19
components/esp_eth/src/esp_eth_mac_esp.c

@@ -282,28 +282,26 @@ static void emac_esp32_rx_task(void *arg)
             /* set max expected frame len */
             uint32_t frame_len = ETH_MAX_PACKET_SIZE;
             buffer = emac_hal_alloc_recv_buf(&emac->hal, &frame_len);
-            /* there is a waiting frame */
-            if (frame_len) {
-                /* we have memory to receive the frame of maximal size previously defined */
-                if (buffer != NULL) {
-                    uint32_t recv_len = emac_hal_receive_frame(&emac->hal, buffer, EMAC_HAL_BUF_SIZE_AUTO, &emac->frames_remain, &emac->free_rx_descriptor);
-                    if (recv_len == 0) {
-                        ESP_LOGE(TAG, "frame copy error");
-                        free(buffer);
-                        /* ensure that interface to EMAC does not get stuck with unprocessed frames */
-                        emac_hal_flush_recv_frame(&emac->hal, &emac->frames_remain, &emac->free_rx_descriptor);
-                    } else if (frame_len > recv_len) {
-                        ESP_LOGE(TAG, "received frame was truncated");
-                        free(buffer);
-                    } else {
-                        ESP_LOGD(TAG, "receive len= %d", recv_len);
-                        emac->eth->stack_input(emac->eth, buffer, recv_len);
-                    }
-                } else {
-                    ESP_LOGE(TAG, "no mem for receive buffer");
+            /* we have memory to receive the frame of maximal size previously defined */
+            if (buffer != NULL) {
+                uint32_t recv_len = emac_hal_receive_frame(&emac->hal, buffer, EMAC_HAL_BUF_SIZE_AUTO, &emac->frames_remain, &emac->free_rx_descriptor);
+                if (recv_len == 0) {
+                    ESP_LOGE(TAG, "frame copy error");
+                    free(buffer);
                     /* ensure that interface to EMAC does not get stuck with unprocessed frames */
                     emac_hal_flush_recv_frame(&emac->hal, &emac->frames_remain, &emac->free_rx_descriptor);
+                } else if (frame_len > recv_len) {
+                    ESP_LOGE(TAG, "received frame was truncated");
+                    free(buffer);
+                } else {
+                    ESP_LOGD(TAG, "receive len= %d", recv_len);
+                    emac->eth->stack_input(emac->eth, buffer, recv_len);
                 }
+            /* if allocation failed and there is a waiting frame */
+            } else if (frame_len) {
+                ESP_LOGE(TAG, "no mem for receive buffer");
+                /* ensure that interface to EMAC does not get stuck with unprocessed frames */
+                emac_hal_flush_recv_frame(&emac->hal, &emac->frames_remain, &emac->free_rx_descriptor);
             }
 #if CONFIG_ETH_SOFT_FLOW_CONTROL
             // we need to do extra checking of remained frames in case there are no unhandled frames left, but pause frame is still undergoing

+ 18 - 20
components/esp_eth/src/esp_eth_mac_ksz8851snl.c

@@ -692,32 +692,30 @@ static void emac_ksz8851snl_task(void *arg)
                 uint32_t frame_len = ETH_MAX_PACKET_SIZE;
                 uint8_t *buffer;
                 emac_ksz8851_alloc_recv_buf(emac, &buffer, &frame_len);
-                /* there is a waiting frame */
-                if (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) {
-                                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");
+                /* 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) {
                             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, "no mem for receive buffer");
+                        ESP_LOGE(TAG, "frame read from module failed");
                         emac_ksz8851_flush_recv_queue(emac);
+                        free(buffer);
                     }
+                /* if allocation failed and there is a waiting frame */
+                } else if (frame_len) {
+                    ESP_LOGE(TAG, "no mem for receive buffer");
+                    emac_ksz8851_flush_recv_queue(emac);
                 }
             }
             ksz8851_write_reg(emac, KSZ8851_IER, ier);

+ 18 - 20
components/esp_eth/src/esp_eth_mac_w5500.c

@@ -656,30 +656,28 @@ static void emac_w5500_task(void *arg)
                 /* define max expected frame len */
                 frame_len = ETH_MAX_PACKET_SIZE;
                 emac_w5500_alloc_recv_buf(emac, &buffer, &frame_len);
-                /* there is a waiting frame */
-                if (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);
-                            } 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");
+                /* 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);
+                        } 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, "no mem for receive buffer");
-                        emac_w5500_flush_recv_frame(emac);
+                        ESP_LOGE(TAG, "frame read from module failed");
+                        free(buffer);
                     }
+                /* if allocation failed and there is a waiting frame */
+                } else if (frame_len) {
+                    ESP_LOGE(TAG, "no mem for receive buffer");
+                    emac_w5500_flush_recv_frame(emac);
                 }
             } while (emac->packets_remain);
         }