search

espressif / esp-iot-bridge

A smart bridge to make both ESP and the other MCU or smart device can access the Internet.
Apache License 2.0
140 stars 49 forks source link

USB RNDIS Bridge memory leak (AEGHB-193) #46

Closed wuyuanyi135 closed 12 months ago

wuyuanyi135 commented 12 months ago

In the function

https://github.com/espressif/esp-iot-bridge/blob/8ae9fd267615d4531bd20e0b9ee7c74ddcc3a745/components/iot_bridge/src/bridge_usb.c#L161

all it releases are the l2_buffers, which seems to be specific to wlan driver. When looking at the ethernet driver, l2_buffer was not used.

This causes continuous inflation of memory usage. After change all l2_buffer to buffer (leave l2_buffer unused) the memory no longer leaks.

wuyuanyi135 commented 12 months ago

It causes memory leak since I am using my custom implementation of the usb RNDIS driver, which is slightly different from the implementation in this repository.

Here, mistakenly releasing l2_buffer actually saved the chip from crash because https://github.com/espressif/esp-iot-bridge/blob/8ae9fd267615d4531bd20e0b9ee7c74ddcc3a745/components/usb/usb_device/additions/src/tusb_net.c#LL70C6-L70C25

passed in a static buffer

https://github.com/espressif/esp-iot-bridge/blob/8ae9fd267615d4531bd20e0b9ee7c74ddcc3a745/components/usb/usb_device/additions/tusb/src/class/net/ecm_rndis_device.c#L64

which will crash the system if one is actually trying to free it. Here l2_buffer should be empty as it is a buffer for wlan driver, hence not effective.

I just want to raise another alarm that, because the static buffer in USB driver was not copied and if L2 to L3 copy was not enabled, the netif will pass a PBUF_REF to the internal driver without copying the data in the static buffer. It is possible that the deferred access to the pointer will result in invalid data as it has been overwritten by new packet.