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

httpd_ssl_start potential memory leak (IDFGH-12519) #13526

Closed
3 tasks done
sascha-jopen opened this issue Apr 1, 2024 · 0 comments
Closed
3 tasks done

httpd_ssl_start potential memory leak (IDFGH-12519) #13526

sascha-jopen opened this issue Apr 1, 2024 · 0 comments
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Bug bugs in IDF

Comments

@sascha-jopen
Copy link

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

master

Espressif SoC revision.

ESP32

Operating System used.

Linux

How did you build your project?

CLion IDE

If you are using Windows, please specify command line type.

None

Development Kit.

ESP32-PICO-KIT

Power Supply used.

USB

What is the expected behavior?

When httpd_ssl_start() is invoked from a user application and an error occurs during httpd_start() within that function, all allocated memory of httpd_ssl_start should be freed to prevent memory leaks.

What is the actual behavior?

In

httpd_ssl_ctx_t *ssl_ctx = calloc(1, sizeof(httpd_ssl_ctx_t));
memory for a new ssl context is allocated. If starting the underlying http server in
ret = httpd_start(&handle, &config->httpd);
fails, the ssl context is not freed. Instead the function returns in
if (ret != ESP_OK) return ret;
without any cleanup. The cleanup is not done in
esp_err_t httpd_start(httpd_handle_t *handle, const httpd_config_t *config)
either.

Steps to reproduce.

I haven't checked the issue on an ESP32 device but only with code review. However, calling httpd_ssl_start with config->max_open_sockets = CONFIG_LWIP_MAX_SOCKETS or config->max_open_sockets = 15, depending on the build, should trigger the memory leak.

Debug Logs.

No response

More Information.

On a first glance

ret = create_secure_context(config, &ssl_ctx);
looks like producing a memory leak when create_secure_context() fails, too. This is actually not true as the memory is freed in in case of an error. However, it might be more readable to move freeing the memory in case of an error to the caller, where the memory was allocated in the first place.

@sascha-jopen sascha-jopen added the Type: Bug bugs in IDF label Apr 1, 2024
@espressif-bot espressif-bot added the Status: Opened Issue is new label Apr 1, 2024
@github-actions github-actions bot changed the title httpd_ssl_start potential memory leak httpd_ssl_start potential memory leak (IDFGH-12519) Apr 1, 2024
@espressif-bot espressif-bot added Status: Reviewing Issue is being reviewed and removed Status: Opened Issue is new labels Apr 22, 2024
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: Reviewing Issue is being reviewed labels May 6, 2024
espressif-bot pushed a commit that referenced this issue May 21, 2024
This MR This restructured code to prevent memory leak during the starting HTTP server.

Closes #13526
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

4 participants