Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in esp_bt_hidh_dev_report_write (IDFGH-7786) #9323

Closed
1 of 3 tasks
selimrecep opened this issue Jul 10, 2022 · 2 comments
Closed
1 of 3 tasks

Memory leak in esp_bt_hidh_dev_report_write (IDFGH-7786) #9323

selimrecep opened this issue Jul 10, 2022 · 2 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally

Comments

@selimrecep
Copy link

selimrecep commented Jul 10, 2022

Environment

Development Kit: ESP32-DevKitC
Kit version v4
Module or chip used: ESP32-WROOM-32
IDF version 4.4.1 (ESP_IDF_VERSION)
Build System: idf.py (on platform.io)
Compiler version: toolchain-xtensa-esp32 @ 8.4.0+2021r2-patch3
Operating System: Windows
(Windows only) environment type: PowerShell
Using an IDE?: Platform IO
Power Supply: USB

Problem Description

When I call esp_hidh_dev_output_set I detect heap memory leak, I think it is caused by here

if ((p_data = malloc(len + 1)) == NULL) {

when report_id is not NULL (and for most of the time it is not NULL) it allocates new memory block for the report to set the report id in the beginning of the report, then, it gets osi_malloc ed in btc_transfer_context which is (I believe) deallocated. But not the one allocated via malloc in esp_bt_hidh_dev_report_write

Expected Behavior

The function shouldn't leak memory

Actual Behavior

Function leaks memory

Steps to reproduce

  1. Call esp_hidh_dev_output_set with a non-zero report_id
  2. It leaks memory

// If possible, attach a picture of your setup/wiring here.
Bare USB Wiring

Code to reproduce this issue

typedef struct StructType {
  union {
    uint8_t Data[40];
    // Other stuff
  }
} StructType;
// I can't put the whole code here, but as an example
StructType aReport;

printf("\n-- %d ", esp_get_free_heap_size());
// The struct itself includes report id in the beginning so this sends address of the next uint8_t
esp_hidh_dev_output_set(dev, 0, 0x31, (uint8_t*)(&aReport.Data[1]), sizeof(aReport) - 1); 
printf("-- %d \n", esp_get_free_heap_size());

Debug Logs

-- 128708 -- 128468 
-- 128620 -- 128380 
-- 128532 -- 128292 
-- 128444 -- 128204 
-- 128356 -- 128116 
-- 128268 -- 128028 
-- 128180 -- 127940 

Other items if possible

Edit: Small corrections in the example code, doesn't affect the issue

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jul 10, 2022
@github-actions github-actions bot changed the title Memory leak in esp_bt_hidh_dev_report_write Memory leak in esp_bt_hidh_dev_report_write (IDFGH-7786) Jul 10, 2022
@selimrecep
Copy link
Author

selimrecep commented Jul 10, 2022

Adding

if(report_id)
   free(p_data);

right after

ret = esp_bt_hid_host_send_data(dev->bda, data, len);

seems to fix it, but there could be side effects... I am not completely sure

@boblane1
Copy link
Collaborator

@selimrecep

Your fix is right, thanks!

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Opened Issue is new Resolution: NA Issue resolution is unavailable labels Jul 12, 2022
espressif-bot pushed a commit that referenced this issue Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

3 participants