[db_sqlite] fix memory management bugs

* core free function was used to free starting with commit 980f15e which was not freeing the memory correctly(result rows were allocated all at once, but in the current version they were freed once at a time); * check if memory is allocated to prevent core dumps; (cherry picked from commit bb506d2563c2670094537e70cc35b85d4ce4cd80)
Vocalocity · Jul 4, 2016 · ccda718 · ccda718
1 parent 5cbe3fa
commit ccda718
Show file tree

Hide file tree

Showing 3 changed files with 209 additions and 1 deletion.
diff --git a/modules/db_sqlite/dbase.c b/modules/db_sqlite/dbase.c
@@ -760,6 +760,9 @@ static int db_sqlite_free_result_internal(const db_con_t* _h, db_res_t* _r)
  */
 int db_sqlite_free_result(db_con_t* _h, db_res_t* _r)
 {
+	int i, j;
+	db_val_t* val;
+
 	if (!_h) {
 		LM_ERR("invalid database handle\n");
 		return -1;
@@ -775,7 +778,54 @@ int db_sqlite_free_result(db_con_t* _h, db_res_t* _r)
 		return 0;
 	}
 
-	return db_free_result(_r);
+
+	db_free_columns(_r);
+
+	/* for each row iterate through all the values and free them
+	 * the values array(RES_ROW_N rows with RES_COL_N values for
+	 * each row) is allocated using a single chunk so in order
+	 * to free the array(check db_sqlite_allocate_rows function ) */
+	if (RES_ROWS(_r)) {
+		for(i=0; i < RES_ROW_N(_r); i++) {
+			for (j=0; j < RES_COL_N(_r); j++) {
+				val = &(_r->rows[i].values[j]);
+				if (VAL_NULL(val) || !VAL_FREE(val))
+					continue;
+
+				switch (VAL_TYPE(val)) {
+					case DB_STRING:
+					case DB_STR:
+						/*
+						 * FIXME
+						 * see row.c +121 ( last comment ) for
+						 * explanation why this will work
+						 *
+						 */
+						pkg_free(VAL_STR(val).s);
+						VAL_STR(val).s = 0;
+						break;
+					case DB_BLOB:
+						pkg_free(VAL_BLOB(val).s);
+						VAL_BLOB(val).s = 0;
+						break;
+					default:
+						break;
+				}
+
+			}
+		}
+		/* free all the columns; they are all allocated at once */
+		pkg_free( _r->rows[0].values);
+		/* free the rows */
+		pkg_free( _r->rows);
+		_r->rows = NULL;
+	}
+
+	RES_ROW_N(_r) = 0;
+	pkg_free(_r);
+	_r = NULL;
+
+	return 0;
 }
 
 /**

diff --git a/modules/db_sqlite/row.c b/modules/db_sqlite/row.c
@@ -102,6 +102,10 @@ int db_sqlite_convert_row(const db_con_t* _h, db_res_t* _res, db_row_t* _r)
 				db_value = sqlite3_column_blob(CON_SQLITE_PS(_h), col);
 
 				VAL_BLOB(_v).s = pkg_malloc(VAL_BLOB(_v).len+1);
+				if (VAL_BLOB(_v).s == NULL) {
+					LM_ERR("no more pkg mem!\n");
+					return -1;
+				}
 				memcpy(VAL_BLOB(_v).s, db_value, VAL_BLOB(_v).len);
 
 				VAL_BLOB(_v).s[VAL_BLOB(_v).len]='\0';
@@ -114,6 +118,11 @@ int db_sqlite_convert_row(const db_con_t* _h, db_res_t* _res, db_row_t* _r)
 				db_value = (char *)sqlite3_column_text(CON_SQLITE_PS(_h), col);
 
 				VAL_STR(_v).s = pkg_malloc(VAL_STR(_v).len+1);
+				if (VAL_STR(_v).s == NULL) {
+					LM_ERR("no more pkg mem!\n");
+					return -1;
+				}
+
 				memcpy(VAL_STR(_v).s, db_value, VAL_STR(_v).len);
 
 				VAL_STR(_v).s[VAL_STR(_v).len]='\0';

diff --git a/modules/db_sqlite/row.c.orig b/modules/db_sqlite/row.c.orig
@@ -0,0 +1,149 @@
+/**
+ *
+ * Copyright (C) 2015 OpenSIPS Foundation
+ *
+ * This file is part of opensips, a free SIP server.
+ *
+ * opensips is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version
+ *
+ * opensips is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301  USA
+ *
+ * History
+ * -------
+ *  2015-03-03  initial version (Ionut Ionita)
+*/
+
+#include "../../dprint.h"
+#include "../../mem/mem.h"
+#include "../../db/db_row.h"
+#include "../../db/db_ut.h"
+#include "../../db/db_val.h"
+#include "../../db/db_row.h"
+#include "my_con.h"
+#include "val.h"
+#include "row.h"
+
+#define DB_UNDEFINED 1024
+
+/**
+ * Convert a row from result into db API representation
+ */
+int db_sqlite_convert_row(const db_con_t* _h, db_res_t* _res, db_row_t* _r)
+{
+	int col;
+	db_val_t* _v;
+	const char* db_value;
+
+	if ((!_h) || (!_res) || (!_r)) {
+		LM_ERR("invalid parameter value\n");
+		return -1;
+	}
+
+	if (!CON_SQLITE_PS(_h)) {
+		LM_ERR("conn has no prepared statement! sqlite requires one\n");
+		return -1;
+	}
+
+	/* Save the number of columns in the ROW structure */
+	ROW_N(_r) = RES_COL_N(_res);
+
+	for(col=0; col < RES_COL_N(_res); col++) {
+		_v = &(ROW_VALUES(_r)[col]);
+
+		if (sqlite3_column_type(CON_SQLITE_PS(_h), col) == SQLITE_NULL) {
+			VAL_NULL(_v) = 1;
+			continue;
+		}
+
+		switch (RES_TYPES(_res)[col]) {
+			case DB_BITMAP:
+				/* value considered to be int; but stored as bigint;
+				 * can be used as VAL_INT() to be called
+				 * also can be used as VAL_BIGINT() */
+				VAL_BIGINT(_v) = sqlite3_column_int64(CON_SQLITE_PS(_h), col);
+				VAL_TYPE(_v) = DB_INT;
+
+				break;
+			case DB_INT:
+				VAL_BIGINT(_v) =sqlite3_column_int64(CON_SQLITE_PS(_h), col);
+				VAL_TYPE(_v) = DB_INT;
+
+				break;
+			case DB_BIGINT:
+				VAL_BIGINT(_v) = sqlite3_column_int64(CON_SQLITE_PS(_h), col);
+				VAL_TYPE(_v) = DB_BIGINT;
+
+				break;
+			case DB_DATETIME:
+				db_value = (char *)sqlite3_column_text(CON_SQLITE_PS(_h), col);
+				if (db_str2time(db_value, &VAL_TIME(_v)) < 0) {
+					LM_ERR("error while converting datetime value from string\n");
+					return -1;
+				}
+				VAL_TYPE(_v) = DB_DATETIME;
+				break;
+			case DB_DOUBLE:
+				VAL_DOUBLE(_v) = sqlite3_column_double(CON_SQLITE_PS(_h), col);
+				VAL_TYPE(_v) = DB_DOUBLE;
+
+				break;
+			case DB_BLOB:
+				VAL_BLOB(_v).len = sqlite3_column_bytes(CON_SQLITE_PS(_h), col);
+				db_value = sqlite3_column_blob(CON_SQLITE_PS(_h), col);
+
+				VAL_BLOB(_v).s = pkg_malloc(VAL_BLOB(_v).len+1);
+				memcpy(VAL_BLOB(_v).s, db_value, VAL_BLOB(_v).len);
+
+				VAL_BLOB(_v).s[VAL_BLOB(_v).len]='\0';
+				VAL_TYPE(_v) = DB_BLOB;
+				VAL_FREE(_v) = 1;
+
+				break;
+			case DB_STRING:
+				VAL_STR(_v).len = sqlite3_column_bytes(CON_SQLITE_PS(_h), col);
+				db_value = (char *)sqlite3_column_text(CON_SQLITE_PS(_h), col);
+
+				VAL_STR(_v).s = pkg_malloc(VAL_STR(_v).len+1);
+				memcpy(VAL_STR(_v).s, db_value, VAL_STR(_v).len);
+
+				VAL_STR(_v).s[VAL_STR(_v).len]='\0';
+<<<<<<< HEAD
+
+				/* WARNING: we set the type to DB_STRING based on str definition
+				 * { char*
+				 *   int
+				 * }
+				 *
+				 * this way we know that if someone will access VAL_STRING(...)
+				 * of this value will get the char* value from the str structure
+				 * if someone will call VAL_STR(...) there will still be no
+				 * problem since we set the len
+				 *
+				 * invalid memory access shouldn't be the problem since it's an
+				 * union so we definetely will have the space for a str allocated
+				 *
+				 */
+				VAL_TYPE(_v) = DB_STRING;
+=======
+				VAL_TYPE(_v) = DB_STR;
+				VAL_FREE(_v) = 1;
+
+>>>>>>> 308ff0d... Merge pull request #919 from Danfx/2.2
+				break;
+			default:
+				LM_ERR("invalid type for sqlite!\n");
+				return -1;
+		}
+	}
+	return 0;
+}