[sr-dev] git:4.0: db_mysql: fix segfault when recursive queries are made

Richard Fuchs rfuchs at sipwise.com
Fri Apr 12 16:33:25 CEST 2013


Module: sip-router
Branch: 4.0
Commit: 5e5ba673fa4d2351047eadeaec655017c962424d
URL:    http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5e5ba673fa4d2351047eadeaec655017c962424d

Author: Richard Fuchs <rfuchs at sipwise.com>
Committer: Richard Fuchs <rfuchs at sipwise.com>
Date:   Wed Apr 10 09:34:49 2013 -0400

db_mysql: fix segfault when recursive queries are made

The MySQL result object (MYSQL_RES) should not be stored within the
srdb1 connection object, but rather within the srdb1 result object.
Otherwise recursive queries overwrite each other's result sets, which
results in segfault.

Conflicts:
	modules/db_mysql/km_my_con.h

---

 modules/db_mysql/km_dbase.c  |   36 +++++++++++++++++++-----------------
 modules/db_mysql/km_dbase.h  |    2 +-
 modules/db_mysql/km_my_con.c |    1 -
 modules/db_mysql/km_my_con.h |    4 ----
 modules/db_mysql/km_res.c    |   27 +++++++++++++++++++++++----
 modules/db_mysql/km_res.h    |   19 +++++++++++++++++++
 modules/db_mysql/km_row.c    |    5 +++--
 7 files changed, 65 insertions(+), 29 deletions(-)

diff --git a/modules/db_mysql/km_dbase.c b/modules/db_mysql/km_dbase.c
index e1cce08..151a24a 100644
--- a/modules/db_mysql/km_dbase.c
+++ b/modules/db_mysql/km_dbase.c
@@ -163,14 +163,14 @@ static int db_mysql_store_result(const db1_con_t* _h, db1_res_t** _r)
 		return -1;
 	}
 
-	*_r = db_new_result();
+	*_r = db_mysql_new_result();
 	if (*_r == 0) {
 		LM_ERR("no memory left\n");
 		return -2;
 	}
 
-	CON_RESULT(_h) = mysql_store_result(CON_CONNECTION(_h));
-	if (!CON_RESULT(_h)) {
+	RES_RESULT(*_r) = mysql_store_result(CON_CONNECTION(_h));
+	if (!RES_RESULT(*_r)) {
 		if (mysql_field_count(CON_CONNECTION(_h)) == 0) {
 			(*_r)->col.n = 0;
 			(*_r)->n = 0;
@@ -181,7 +181,7 @@ static int db_mysql_store_result(const db1_con_t* _h, db1_res_t** _r)
 			if (code == CR_SERVER_GONE_ERROR || code == CR_SERVER_LOST) {
 				counter_inc(mysql_cnts_h.driver_err);
 			}
-			db_free_result(*_r);
+			db_mysql_free_result(_h, *_r);
 			*_r = 0;
 			return -3;
 		}
@@ -195,14 +195,14 @@ static int db_mysql_store_result(const db1_con_t* _h, db1_res_t** _r)
 		/* all mem on Kamailio API side is already freed by
 		 * db_mysql_convert_result in case of error, but we also need
 		 * to free the mem from the mysql lib side */
-		mysql_free_result(CON_RESULT(_h));
+		mysql_free_result(RES_RESULT(*_r));
 #if (MYSQL_VERSION_ID >= 40100)
 		while( mysql_more_results(CON_CONNECTION(_h)) && mysql_next_result(CON_CONNECTION(_h)) > 0 ) {
 			MYSQL_RES *res = mysql_store_result( CON_CONNECTION(_h) );
 			mysql_free_result(res);
 		}
 #endif
-		CON_RESULT(_h) = 0;
+		RES_RESULT(*_r) = 0;
 		return -4;
 	}
 
@@ -224,19 +224,21 @@ done:
  * \param _r result set that should be freed
  * \return zero on success, negative value on failure
  */
-int db_mysql_free_result(db1_con_t* _h, db1_res_t* _r)
+int db_mysql_free_result(const db1_con_t* _h, db1_res_t* _r)
 {
      if ((!_h) || (!_r)) {
 	     LM_ERR("invalid parameter value\n");
 	     return -1;
      }
 
+     mysql_free_result(RES_RESULT(_r));
+     RES_RESULT(_r) = 0;
+     pkg_free(RES_PTR(_r));
+
      if (db_free_result(_r) < 0) {
 	     LM_ERR("unable to free result structure\n");
 	     return -1;
      }
-     mysql_free_result(CON_RESULT(_h));
-     CON_RESULT(_h) = 0;
      return 0;
 }
 
@@ -288,21 +290,21 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows)
 
 	/* exit if the fetch count is zero */
 	if (nrows == 0) {
-		db_free_result(*_r);
+		db_mysql_free_result(_h, *_r);
 		*_r = 0;
 		return 0;
 	}
 
 	if(*_r==0) {
 		/* Allocate a new result structure */
-		*_r = db_new_result();
+		*_r = db_mysql_new_result();
 		if (*_r == 0) {
 			LM_ERR("no memory left\n");
 			return -2;
 		}
 
-		CON_RESULT(_h) = mysql_store_result(CON_CONNECTION(_h));
-		if (!CON_RESULT(_h)) {
+		RES_RESULT(*_r) = mysql_store_result(CON_CONNECTION(_h));
+		if (!RES_RESULT(*_r)) {
 			if (mysql_field_count(CON_CONNECTION(_h)) == 0) {
 				(*_r)->col.n = 0;
 				(*_r)->n = 0;
@@ -313,7 +315,7 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows)
 				if (code == CR_SERVER_GONE_ERROR || code == CR_SERVER_LOST) {
 					counter_inc(mysql_cnts_h.driver_err);
 				}
-				db_free_result(*_r);
+				db_mysql_free_result(_h, *_r);
 				*_r = 0;
 				return -3;
 			}
@@ -323,7 +325,7 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows)
 			return -4;
 		}
 
-		RES_NUM_ROWS(*_r) = mysql_num_rows(CON_RESULT(_h));
+		RES_NUM_ROWS(*_r) = mysql_num_rows(RES_RESULT(*_r));
 		if (!RES_NUM_ROWS(*_r)) {
 			LM_DBG("no rows returned from the query\n");
 			RES_ROWS(*_r) = 0;
@@ -362,8 +364,8 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows)
 	}
 
 	for(i = 0; i < rows; i++) {
-		CON_ROW(_h) = mysql_fetch_row(CON_RESULT(_h));
-		if (!CON_ROW(_h)) {
+		RES_ROW(*_r) = mysql_fetch_row(RES_RESULT(*_r));
+		if (!RES_ROW(*_r)) {
 			LM_ERR("driver error: %s\n", mysql_error(CON_CONNECTION(_h)));
 			RES_ROW_N(*_r) = i;
 			db_free_rows(*_r);
diff --git a/modules/db_mysql/km_dbase.h b/modules/db_mysql/km_dbase.h
index 3a567f2..2acf517 100644
--- a/modules/db_mysql/km_dbase.h
+++ b/modules/db_mysql/km_dbase.h
@@ -57,7 +57,7 @@ void db_mysql_close(db1_con_t* _h);
 /*! \brief
  * Free all memory allocated by get_result
  */
-int db_mysql_free_result(db1_con_t* _h, db1_res_t* _r);
+int db_mysql_free_result(const db1_con_t* _h, db1_res_t* _r);
 
 
 /*! \brief
diff --git a/modules/db_mysql/km_my_con.c b/modules/db_mysql/km_my_con.c
index ce20be2..646d74b 100644
--- a/modules/db_mysql/km_my_con.c
+++ b/modules/db_mysql/km_my_con.c
@@ -144,7 +144,6 @@ void db_mysql_free_connection(struct pool_con* con)
 
 	_c = (struct my_con*) con;
 
-	if (_c->res) mysql_free_result(_c->res);
 	if (_c->id) free_db_id(_c->id);
 	if (_c->con) {
 		mysql_close(_c->con);
diff --git a/modules/db_mysql/km_my_con.h b/modules/db_mysql/km_my_con.h
index 2624f34..3d71d00 100644
--- a/modules/db_mysql/km_my_con.h
+++ b/modules/db_mysql/km_my_con.h
@@ -44,9 +44,7 @@ struct my_con {
 	unsigned int ref;        /*!< Reference count */
 	struct pool_con* next;   /*!< Next connection in the pool */
 
-	MYSQL_RES* res;          /*!< Actual result */
 	MYSQL* con;              /*!< Connection representation */
-	MYSQL_ROW row;           /*!< Actual row in the result */
 	time_t timestamp;        /*!< Timestamp of last query */
 };
 
@@ -54,9 +52,7 @@ struct my_con {
 /*
  * Some convenience wrappers
  */
-#define CON_RESULT(db_con)     (((struct my_con*)((db_con)->tail))->res)
 #define CON_CONNECTION(db_con) (((struct my_con*)((db_con)->tail))->con)
-#define CON_ROW(db_con)        (((struct my_con*)((db_con)->tail))->row)
 #define CON_TIMESTAMP(db_con)  (((struct my_con*)((db_con)->tail))->timestamp)
 
 
diff --git a/modules/db_mysql/km_res.c b/modules/db_mysql/km_res.c
index 10d3ce2..965e6dc 100644
--- a/modules/db_mysql/km_res.c
+++ b/modules/db_mysql/km_res.c
@@ -73,7 +73,7 @@ int db_mysql_get_columns(const db1_con_t* _h, db1_res_t* _r)
 		return -3;
 	}
 
-	fields = mysql_fetch_fields(CON_RESULT(_h));
+	fields = mysql_fetch_fields(RES_RESULT(_r));
 	for(col = 0; col < RES_COL_N(_r); col++) {
 		RES_NAMES(_r)[col] = (str*)pkg_malloc(sizeof(str));
 		if (! RES_NAMES(_r)[col]) {
@@ -164,7 +164,7 @@ static inline int db_mysql_convert_rows(const db1_con_t* _h, db1_res_t* _r)
 		return -1;
 	}
 
-	RES_ROW_N(_r) = mysql_num_rows(CON_RESULT(_h));
+	RES_ROW_N(_r) = mysql_num_rows(RES_RESULT(_r));
 	if (!RES_ROW_N(_r)) {
 		LM_DBG("no rows returned from the query\n");
 		RES_ROWS(_r) = 0;
@@ -177,8 +177,8 @@ static inline int db_mysql_convert_rows(const db1_con_t* _h, db1_res_t* _r)
 	}
 
 	for(row = 0; row < RES_ROW_N(_r); row++) {
-		CON_ROW(_h) = mysql_fetch_row(CON_RESULT(_h));
-		if (!CON_ROW(_h)) {
+		RES_ROW(_r) = mysql_fetch_row(RES_RESULT(_r));
+		if (!RES_ROW(_r)) {
 			LM_ERR("driver error: %s\n", mysql_error(CON_CONNECTION(_h)));
 			RES_ROW_N(_r) = row;
 			db_free_rows(_r);
@@ -221,3 +221,22 @@ int db_mysql_convert_result(const db1_con_t* _h, db1_res_t* _r)
 	return 0;
 }
 
+
+/*!
+ * \brief Allocate new result set with private structure
+ * \return db1_res_t object on success, NULL on failure
+ */
+db1_res_t* db_mysql_new_result(void)
+{
+	db1_res_t* obj;
+
+	obj = db_new_result();
+	if (!obj)
+		return NULL;
+	RES_PTR(obj) = pkg_malloc(sizeof(struct my_res));
+	if (!RES_PTR(obj)) {
+		db_free_result(obj);
+		return NULL;
+	}
+	return obj;
+}
diff --git a/modules/db_mysql/km_res.h b/modules/db_mysql/km_res.h
index 957a961..0b063d8 100644
--- a/modules/db_mysql/km_res.h
+++ b/modules/db_mysql/km_res.h
@@ -37,6 +37,18 @@
 #include "../../lib/srdb1/db_con.h"
 
 
+struct my_res {
+	MYSQL_RES* res;          /*!< Actual result */
+	MYSQL_ROW row;           /*!< Actual row in the result */
+};
+
+/*
+ * Some convenience wrappers
+ */
+#define RES_RESULT(db_res)     (((struct my_res*)((db_res)->ptr))->res)
+#define RES_ROW(db_res)        (((struct my_res*)((db_res)->ptr))->row)
+
+
 /*!
  * \brief Fill the result structure with data from database
  * \param _h database connection
@@ -54,4 +66,11 @@ int db_mysql_convert_result(const db1_con_t* _h, db1_res_t* _r);
  */
 int db_mysql_get_columns(const db1_con_t* _h, db1_res_t* _r);
 
+
+/*!
+ * \brief Allocate new result set with private structure
+ * \return db1_res_t object on success, NULL on failure
+ */
+db1_res_t* db_mysql_new_result(void);
+
 #endif
diff --git a/modules/db_mysql/km_row.c b/modules/db_mysql/km_row.c
index 42f2841..9f2c730 100644
--- a/modules/db_mysql/km_row.c
+++ b/modules/db_mysql/km_row.c
@@ -36,6 +36,7 @@
 #include "km_my_con.h"
 #include "km_val.h"
 #include "km_row.h"
+#include "km_res.h"
 
 /*!
  * \brief Convert a row from result into DB API representation
@@ -59,11 +60,11 @@ int db_mysql_convert_row(const db1_con_t* _h, db1_res_t* _res, db_row_t* _r)
 		return -2;
 	}
 	
-	lengths = mysql_fetch_lengths(CON_RESULT(_h));
+	lengths = mysql_fetch_lengths(RES_RESULT(_res));
 
 	for(i = 0; i < RES_COL_N(_res); i++) {
 		if (db_str2val(RES_TYPES(_res)[i], &(ROW_VALUES(_r)[i]),
-			    ((MYSQL_ROW)CON_ROW(_h))[i], lengths[i], 0) < 0) {
+			    ((MYSQL_ROW)RES_ROW(_res))[i], lengths[i], 0) < 0) {
 			LM_ERR("failed to convert value\n");
 			LM_DBG("free row at %p\n", _r);
 			db_free_row(_r);




More information about the sr-dev mailing list