Skip to content
This repository has been archived by the owner on Jan 29, 2023. It is now read-only.

corrupt heap at MySQL_Connection destructor #19

Closed
pabloandresm opened this issue Mar 11, 2022 · 5 comments
Closed

corrupt heap at MySQL_Connection destructor #19

pabloandresm opened this issue Mar 11, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@pabloandresm
Copy link
Contributor

The MySQL_Connection destructor performs:
if (server_version) free(server_version);

...but....

the variable "server_version" is not initialized to NULL in the MySQL_Packet constructor.

The variable "server_version" must be initialized to NULL in the MySQL_Packet constructor, exactly as the "buffer" is (see MySQL_Generic_Packet_Impl.h line 74)

This ends up, in some cases:
CORRUPT HEAP: Bad head at 0x3fff2344. Expected 0xabba1234 got 0x3ffe4364
assert failed: multi_heap_free multi_heap_poisoning.c:253 (head != NULL)

@pabloandresm
Copy link
Contributor Author

Hello.

That is not the only problem. I still receive "CORRUPT HEAP: Bad head at 0x3fff2334. Expected 0xabba1234 got 0x3ffe4364", "assert failed: multi_heap_free multi_heap_poisoning.c:253 (head != NULL)"

Let me investigate further and I'll keep you informed on my research.

Just one question, why do you free(server_version) in the destructor of the MySQL_Connection? I think it should be done in the destructor of MySQL_Packet, same way the "free(buffer)", as both variables are of that class and malloc on that class.

@pabloandresm
Copy link
Contributor Author

FOUND IT!

I suggest 3 things to fix this:

  1. initialize "server_version=NULL;" at MySQL_Packet constructor, exactly as the "buffer" is.
  2. free server version the same way "buffer" is, in the destructor of MySQL_Packet, and not destructor of MySQL_Connection
  3. the two connect() in MySQL_Connection do "free(server_version)", but you forget to do "server_version=NULL;" afterwards, so the destructor will try to free it again, triggering the segmentation fault.

two times you can find this in your code:
if (server_version)
free(server_version); // don't need it anymore

and it should be:
if (server_version) {
free(server_version); // don't need it anymore
server_version=NULL;
}

hope this helps. In any case the segmentation fault dissappears and everything works correctly after those 4 changes (the last one is located in two places)

@khoih-prog
Copy link
Owner

Hi @pabloandresm

Thanks for spending time to investigate and propose the fix.
I've been actually so busy with other libraries (100+) and could spend only a short time yesterday with this library, after so long time. Therefore, a sloppy job.

Will spend some more time to have a more detailed look. I'd appreciate it if you can help by creating a PR to save me some time. If not, I'll do it anyway.

Regards,

@pabloandresm
Copy link
Contributor Author

pleasure to help.

I've tested these 3 modifications I am suggestion, as well as the other bug fix I've created, and everything works fine.

Regards,

@khoih-prog
Copy link
Owner

Close #19 via PR #21

@khoih-prog khoih-prog added the bug Something isn't working label Mar 12, 2022
khoih-prog added a commit that referenced this issue Mar 12, 2022
### Release v1.6.1

1. Fix memory management bugs. Check [corrupt heap at MySQL_Connection destructor #19](#19) and [malloc server_version result not correctly handled may lead to memory corruption #20](#20)
2. Add support to SAMD21/SAMD51 boards using [Fab_SAM_Arduino core](https://github.com/qbolsee/ArduinoCore-fab-sam)
3. Add support to RP2040 boards using `Seeed RP2040 core`
4. Add `Packages' Patches` for [Fab_SAM_Arduino core](https://github.com/qbolsee/ArduinoCore-fab-sam)
khoih-prog added a commit that referenced this issue Mar 12, 2022
### Release v1.6.1

1. Fix memory management bugs. Check [corrupt heap at MySQL_Connection destructor #19](#19) and [malloc server_version result not correctly handled may lead to memory corruption #20](#20)
2. Add support to SAMD21/SAMD51 boards using [Fab_SAM_Arduino core](https://github.com/qbolsee/ArduinoCore-fab-sam)
3. Add support to RP2040 boards using `Seeed RP2040 core`
4. Add `Packages' Patches` for [Fab_SAM_Arduino core](https://github.com/qbolsee/ArduinoCore-fab-sam)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants