Set mysql->extension to NULL after freeing it

[dveeden]

On reconnect mysql->extension gets used while it isn't safe.
To ensure this doesn't happen let's set it to NULL.

From a valgrind run:

==21888== Invalid read of size 8
==21888==    at 0xC6941DF: mariadb_reconnect (mariadb_lib.c:1595)
==21888==    by 0xC69118C: mthd_my_send_cmd (mariadb_lib.c:365)
==21888==    by 0xC6A494B: mysql_stmt_prepare (mariadb_stmt.c:1616)
==21888==    by 0xC661432: XS_DBD__mysql__db_do (mysql.xs:334)
==21888==    by 0xBE40028: XS_DBI_dispatch (in /usr/lib64/perl5/vendor_perl/auto/DBI/DBI.so)
==21888==    by 0x496EB08: Perl_pp_entersub (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0x4964CF4: Perl_runops_standard (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0x48E193E: perl_run (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0x109349: ??? (in /usr/bin/perl)
==21888==    by 0x4DC3412: (below main) (in /usr/lib64/libc-2.28.so)
==21888==  Address 0xc47e560 is 0 bytes inside a block of size 120 free'd
==21888==    at 0x4839A0C: free (vg_replace_malloc.c:540)
==21888==    by 0xC69557A: mysql_close (mariadb_lib.c:1947)
==21888==    by 0xC657FDA: mysql_db_disconnect (dbdimp.c:2409)
==21888==    by 0xC66438A: XS_DBD__mysql__db_disconnect (mysql.xsi:342)
==21888==    by 0xBE40028: XS_DBI_dispatch (in /usr/lib64/perl5/vendor_perl/auto/DBI/DBI.so)
==21888==    by 0x496EB08: Perl_pp_entersub (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0x4964CF4: Perl_runops_standard (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0x48E193E: perl_run (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0x109349: ??? (in /usr/bin/perl)
==21888==    by 0x4DC3412: (below main) (in /usr/lib64/libc-2.28.so)
==21888==  Block was alloc'd at
==21888==    at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==21888==    by 0xC692692: mysql_init (mariadb_lib.c:996)
==21888==    by 0xC655175: mysql_dr_connect (dbdimp.c:1688)
==21888==    by 0xC657B62: my_login (dbdimp.c:2235)
==21888==    by 0xC657C9F: mysql_db_login (dbdimp.c:2285)
==21888==    by 0xC664C0A: XS_DBD__mysql__db__login (mysql.xsi:126)
==21888==    by 0x496EB08: Perl_pp_entersub (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0x4964CF4: Perl_runops_standard (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0x48D95EC: Perl_call_sv (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0xBE41188: XS_DBI_dispatch (in /usr/lib64/perl5/vendor_perl/auto/DBI/DBI.so)
==21888==    by 0x496EB08: Perl_pp_entersub (in /usr/lib64/libperl.so.5.28.1)
==21888==    by 0x4964CF4: Perl_runops_standard (in /usr/lib64/libperl.so.5.28.1)
==21888==

Related:

• Test failures with MariaDB 10.3.9 perl5-dbi/DBD-mysql#275
• https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=917303

Thanks to @ericherman for helping with this.

[dveeden]

Looks related to https://jira.mariadb.org/browse/CONC-289

[pali]

This does not seems to be correct. After calling mysql_close, its argument structure is freed and therefore its memory should not be accessed. Why then setting some member of deallocated structure to NULL? Looks like bug is somewhere else.

[dr-m]

This does not seems to be correct. After calling mysql_close, its argument structure is freed and therefore its memory should not be accessed. Why then setting some member of deallocated structure to NULL? Looks like bug is somewhere else.

We only invoke free(mysql) if mysql->free_me holds. So, setting mysql->extensions=NULL after free(mysql->extensions) does not seem to be entirely redundant.

[pali]

Yea, but MYSQL* pointer after calling mysql_close on it, is invalid. Therefore any other memory corruption can happen if somebody try to use it in some other mysql_* function...

[9EOR9]

Sorry, but I don't understand the sense of this fix:
For reusing a connection handle after calling mysql_close() it's required to initialize the MYSQL structure again via mysql_init() which initializes the memory of the MYSQL structure.
See also mysql_close

[dveeden]

I agree that this is not the correct fix for the problem I tried to fix.
If mysql->free_me isn't set then mysql_close() doesn't actually free all memory, however in that case the only difference is that something else is supposed to call free().

Side note: the mysql_close() docs say all memory is released (which is not always true). The docs for mysql_init() mention this correctly.