Skip to content

Memory leak on multiple calls to loadCACert and others on WiFiClientSecure #5826

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

Closed
sguarin opened this issue Nov 1, 2021 · 3 comments · Fixed by #6387
Closed

Memory leak on multiple calls to loadCACert and others on WiFiClientSecure #5826

sguarin opened this issue Nov 1, 2021 · 3 comments · Fixed by #6387
Labels
Area: BT&Wifi BT & Wifi related issues Status: Solved

Comments

@sguarin
Copy link

sguarin commented Nov 1, 2021

If you have a system that can accept certs configuration, then multiple calls to loadCACert or related functions will not reutilize the memory assigned.

https://github.com/sguarin/arduino-esp32/blob/caa8d07aafa04441bb85e6046a795249c01d9e39/libraries/WiFiClientSecure/src/WiFiClientSecure.cpp#L304

_streamLoad()
allocates memory for new certificates. A better approach should be using realloc().

https://github.com/sguarin/arduino-esp32/blob/caa8d07aafa04441bb85e6046a795249c01d9e39/libraries/WiFiClientSecure/src/WiFiClientSecure.cpp#L290

@sguarin
Copy link
Author

sguarin commented Nov 27, 2021

This temporary fix worked for me.
But I think the memory pointer should not be static and should live on the class.

https://github.com/espressif/arduino-esp32/compare/master...sguarin:fix_streamLoad?expand=1

@TD-er
Copy link
Contributor

TD-er commented Nov 27, 2021

Just a repeat of the comment I placed with that commit:

You're not checking the return value of realloc.
If the return value is not a nullptr, you should assign it to *destPtr.
If it is a nullptr, it depends on the C++ version whether it was a failed alloc or not changed (e.g. size was the same)

And no need to check if *destPtr is a nullptr, as realloc called with a nullptr acts like malloc.

@sguarin
Copy link
Author

sguarin commented Dec 25, 2021

You are right. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BT&Wifi BT & Wifi related issues Status: Solved
Projects
Development

Successfully merging a pull request may close this issue.

3 participants