[libdbi-devel] Memory problem in libdbi/sqlite3 dbd

[Phil L. <plo...@ro...>]

Valgrind has provided info on a memory handling problem:

==14314== Invalid free() / delete / delete[]                                                       
==14314==    at 0x4025DFA: free (vg_replace_malloc.c:323)                                          
==14314==    by 0x7F9EB84: dbi_conn_error (in /usr/lib/libdbi.so.0.0.5)                            
==14314==    by 0x7F5EBE2: sqlite3_error_fn (gnc-backend-dbi.c:156)                                
==14314==    by 0x7F9DB90: _error_handler (in /usr/lib/libdbi.so.0.0.5)                            
==14314==    by 0x7F9DC80: dbi_conn_get_table_list (in 
/usr/lib/libdbi.so.0.0.5)                   
==14314==    by 0x7F62309: conn_does_table_exist (gnc-backend-dbi.c:1243)
==14314==  Address 0x8950cf0 is 0 bytes inside a block of size 1 free'd
==14314==    at 0x4025DFA: free (vg_replace_malloc.c:323)
==14314==    by 0x7F968CE: dbd_list_tables (in /usr/lib/dbd/libdbdsqlite3.so)
==14314==    by 0x7F9DC5B: dbi_conn_get_table_list (in 
/usr/lib/libdbi.so.0.0.5)
==14314==    by 0x7F62309: conn_does_table_exist (gnc-backend-dbi.c:1243)

I've removed parts of the stack traces which are not important.

 1102 dbi_result dbi_conn_get_table_list(dbi_conn Conn, const char *db, const 
char *pattern) {
 1103 	dbi_conn_t *conn = Conn;
 1104 	dbi_result_t *result;
 1105 
 1106 	if (!conn) return NULL;
 1107 
 1108 	_reset_conn_error(conn);
 1109 
 1110 	result = conn->driver->functions->list_tables(conn, db, pattern);
 1111 
 1112 	if (result == NULL) {
 1113 		_error_handler(conn, DBI_ERROR_DBD);
 1114 	}
 1115 
 1116 	return (dbi_result)result;
 1117 }
(this includes the line numbers from the web cvs browser)
dbd_sqlite3.c has:
  529   dbi_result = dbd_query(tempconn, sql_cmd);
  530   free(sql_cmd);
  531   /*   fprintf(stderr, "select from sqlite3_master has run\n"); */
  532   if (dbi_result) {
  533     while (dbi_result_next_row(dbi_result)) {
  534       asprintf(&sql_cmd, "INSERT INTO libdbi_tablenames VALUES ('%s')", dbi_result_get_string(dbi_result, "name"));
  535       retval = sqlite3_exec((sqlite3*)(conn->connection), sql_cmd, NULL, NULL, &sq_errmsg);
  536       free(sql_cmd);
  537     }
  538     dbi_result_free(dbi_result);
  539   }
  540   else {
  541     dbi_conn_error(tempconn, (const char**)&sq_errmsg);
  542     free(sq_errmsg);
  543   }
  544 
  545   //  sqlite3_close((sqlite3*)(tempconn->connection));
  546   dbi_conn_close(tempconn);
  547 
  548   return dbd_query(conn, "SELECT tablename FROM libdbi_tablenames ORDER BY tablename");

dbi_conn_error has:
  618 int dbi_conn_error(dbi_conn Conn, const char **errmsg_dest) {
  619 	dbi_conn_t *conn = Conn;
  620 	char number_portion[20];
  621 	static char *errmsg = NULL; // XXX quick hack, revisit this when API is redesigned
  622 
  623 	if (errmsg_dest) {
  624 		if (errmsg) free(errmsg);
  625 
  626 		if (conn->error_number) {
  627 			snprintf(number_portion, 20, "%d: ", conn->error_number);
  628 		}
  629 		else {
  630 			number_portion[0] = '\0';
  631 		}
  632 
  633 		asprintf(&errmsg, "%s%s", number_portion, conn->error_message ? conn->error_message : "");
  634 		*errmsg_dest = errmsg;
  635 	}
  636 
  637 	return conn->error_number;
  638 }

Lines 541/542 of dbd_sqlite3 are dangerous.  They free the static char* errmsg without resetting the pointer to NULL.  This will cause line 624 in dbi_conn_error() to free the pointer again.
I don't know the architecture of libdbi well enough to propose a fix.  Perhaps remove line 542 of dbd_sqlite3.c.  Perhaps add a dbi_conn_clear_error() which frees the pointer and then clears it.

Phil