Browse code

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.

Richard Fuchs authored on 10/04/2013 13:34:49
Showing 7 changed files
... ...
@@ -163,14 +163,14 @@ static int db_mysql_store_result(const db1_con_t* _h, db1_res_t** _r)
163 163
 		return -1;
164 164
 	}
165 165
 
166
-	*_r = db_new_result();
166
+	*_r = db_mysql_new_result();
167 167
 	if (*_r == 0) {
168 168
 		LM_ERR("no memory left\n");
169 169
 		return -2;
170 170
 	}
171 171
 
172
-	CON_RESULT(_h) = mysql_store_result(CON_CONNECTION(_h));
173
-	if (!CON_RESULT(_h)) {
172
+	RES_RESULT(*_r) = mysql_store_result(CON_CONNECTION(_h));
173
+	if (!RES_RESULT(*_r)) {
174 174
 		if (mysql_field_count(CON_CONNECTION(_h)) == 0) {
175 175
 			(*_r)->col.n = 0;
176 176
 			(*_r)->n = 0;
... ...
@@ -181,7 +181,7 @@ static int db_mysql_store_result(const db1_con_t* _h, db1_res_t** _r)
181 181
 			if (code == CR_SERVER_GONE_ERROR || code == CR_SERVER_LOST) {
182 182
 				counter_inc(mysql_cnts_h.driver_err);
183 183
 			}
184
-			db_free_result(*_r);
184
+			db_mysql_free_result(_h, *_r);
185 185
 			*_r = 0;
186 186
 			return -3;
187 187
 		}
... ...
@@ -195,14 +195,14 @@ static int db_mysql_store_result(const db1_con_t* _h, db1_res_t** _r)
195 195
 		/* all mem on Kamailio API side is already freed by
196 196
 		 * db_mysql_convert_result in case of error, but we also need
197 197
 		 * to free the mem from the mysql lib side */
198
-		mysql_free_result(CON_RESULT(_h));
198
+		mysql_free_result(RES_RESULT(*_r));
199 199
 #if (MYSQL_VERSION_ID >= 40100)
200 200
 		while( mysql_more_results(CON_CONNECTION(_h)) && mysql_next_result(CON_CONNECTION(_h)) > 0 ) {
201 201
 			MYSQL_RES *res = mysql_store_result( CON_CONNECTION(_h) );
202 202
 			mysql_free_result(res);
203 203
 		}
204 204
 #endif
205
-		CON_RESULT(_h) = 0;
205
+		RES_RESULT(*_r) = 0;
206 206
 		return -4;
207 207
 	}
208 208
 
... ...
@@ -224,19 +224,21 @@ done:
224 224
  * \param _r result set that should be freed
225 225
  * \return zero on success, negative value on failure
226 226
  */
227
-int db_mysql_free_result(db1_con_t* _h, db1_res_t* _r)
227
+int db_mysql_free_result(const db1_con_t* _h, db1_res_t* _r)
228 228
 {
229 229
      if ((!_h) || (!_r)) {
230 230
 	     LM_ERR("invalid parameter value\n");
231 231
 	     return -1;
232 232
      }
233 233
 
234
+     mysql_free_result(RES_RESULT(_r));
235
+     RES_RESULT(_r) = 0;
236
+     pkg_free(RES_PTR(_r));
237
+
234 238
      if (db_free_result(_r) < 0) {
235 239
 	     LM_ERR("unable to free result structure\n");
236 240
 	     return -1;
237 241
      }
238
-     mysql_free_result(CON_RESULT(_h));
239
-     CON_RESULT(_h) = 0;
240 242
      return 0;
241 243
 }
242 244
 
... ...
@@ -288,21 +290,21 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows)
288 288
 
289 289
 	/* exit if the fetch count is zero */
290 290
 	if (nrows == 0) {
291
-		db_free_result(*_r);
291
+		db_mysql_free_result(_h, *_r);
292 292
 		*_r = 0;
293 293
 		return 0;
294 294
 	}
295 295
 
296 296
 	if(*_r==0) {
297 297
 		/* Allocate a new result structure */
298
-		*_r = db_new_result();
298
+		*_r = db_mysql_new_result();
299 299
 		if (*_r == 0) {
300 300
 			LM_ERR("no memory left\n");
301 301
 			return -2;
302 302
 		}
303 303
 
304
-		CON_RESULT(_h) = mysql_store_result(CON_CONNECTION(_h));
305
-		if (!CON_RESULT(_h)) {
304
+		RES_RESULT(*_r) = mysql_store_result(CON_CONNECTION(_h));
305
+		if (!RES_RESULT(*_r)) {
306 306
 			if (mysql_field_count(CON_CONNECTION(_h)) == 0) {
307 307
 				(*_r)->col.n = 0;
308 308
 				(*_r)->n = 0;
... ...
@@ -313,7 +315,7 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows)
313 313
 				if (code == CR_SERVER_GONE_ERROR || code == CR_SERVER_LOST) {
314 314
 					counter_inc(mysql_cnts_h.driver_err);
315 315
 				}
316
-				db_free_result(*_r);
316
+				db_mysql_free_result(_h, *_r);
317 317
 				*_r = 0;
318 318
 				return -3;
319 319
 			}
... ...
@@ -323,7 +325,7 @@ int db_mysql_fetch_result(const db1_con_t* _h, db1_res_t** _r, const int nrows)
323 323
 			return -4;
324 324
 		}
325 325
 
326
-		RES_NUM_ROWS(*_r) = mysql_num_rows(CON_RESULT(_h));
326
+		RES_NUM_ROWS(*_r) = mysql_num_rows(RES_RESULT(*_r));
327 327
 		if (!RES_NUM_ROWS(*_r)) {
328 328
 			LM_DBG("no rows returned from the query\n");
329 329
 			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)
362 362
 	}
363 363
 
364 364
 	for(i = 0; i < rows; i++) {
365
-		CON_ROW(_h) = mysql_fetch_row(CON_RESULT(_h));
366
-		if (!CON_ROW(_h)) {
365
+		RES_ROW(*_r) = mysql_fetch_row(RES_RESULT(*_r));
366
+		if (!RES_ROW(*_r)) {
367 367
 			LM_ERR("driver error: %s\n", mysql_error(CON_CONNECTION(_h)));
368 368
 			RES_ROW_N(*_r) = i;
369 369
 			db_free_rows(*_r);
... ...
@@ -58,7 +58,7 @@ void db_mysql_close(db1_con_t* _h);
58 58
 /*! \brief
59 59
  * Free all memory allocated by get_result
60 60
  */
61
-int db_mysql_free_result(db1_con_t* _h, db1_res_t* _r);
61
+int db_mysql_free_result(const db1_con_t* _h, db1_res_t* _r);
62 62
 
63 63
 
64 64
 /*! \brief
... ...
@@ -144,7 +144,6 @@ void db_mysql_free_connection(struct pool_con* con)
144 144
 
145 145
 	_c = (struct my_con*) con;
146 146
 
147
-	if (_c->res) mysql_free_result(_c->res);
148 147
 	if (_c->id) free_db_id(_c->id);
149 148
 	if (_c->con) {
150 149
 		mysql_close(_c->con);
... ...
@@ -44,9 +44,7 @@ struct my_con {
44 44
 	unsigned int ref;        /*!< Reference count */
45 45
 	struct pool_con* next;   /*!< Next connection in the pool */
46 46
 
47
-	MYSQL_RES* res;          /*!< Actual result */
48 47
 	MYSQL* con;              /*!< Connection representation */
49
-	MYSQL_ROW row;           /*!< Actual row in the result */
50 48
 	time_t timestamp;        /*!< Timestamp of last query */
51 49
 	int transaction;	 /*!< indicates whether a multi-query transaction is currently open */
52 50
 };
... ...
@@ -55,9 +53,7 @@ struct my_con {
55 55
 /*
56 56
  * Some convenience wrappers
57 57
  */
58
-#define CON_RESULT(db_con)      (((struct my_con*)((db_con)->tail))->res)
59 58
 #define CON_CONNECTION(db_con)  (((struct my_con*)((db_con)->tail))->con)
60
-#define CON_ROW(db_con)         (((struct my_con*)((db_con)->tail))->row)
61 59
 #define CON_TIMESTAMP(db_con)   (((struct my_con*)((db_con)->tail))->timestamp)
62 60
 #define CON_TRANSACTION(db_con) (((struct my_con*)((db_con)->tail))->transaction)
63 61
 
... ...
@@ -73,7 +73,7 @@ int db_mysql_get_columns(const db1_con_t* _h, db1_res_t* _r)
73 73
 		return -3;
74 74
 	}
75 75
 
76
-	fields = mysql_fetch_fields(CON_RESULT(_h));
76
+	fields = mysql_fetch_fields(RES_RESULT(_r));
77 77
 	for(col = 0; col < RES_COL_N(_r); col++) {
78 78
 		RES_NAMES(_r)[col] = (str*)pkg_malloc(sizeof(str));
79 79
 		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)
164 164
 		return -1;
165 165
 	}
166 166
 
167
-	RES_ROW_N(_r) = mysql_num_rows(CON_RESULT(_h));
167
+	RES_ROW_N(_r) = mysql_num_rows(RES_RESULT(_r));
168 168
 	if (!RES_ROW_N(_r)) {
169 169
 		LM_DBG("no rows returned from the query\n");
170 170
 		RES_ROWS(_r) = 0;
... ...
@@ -177,8 +177,8 @@ static inline int db_mysql_convert_rows(const db1_con_t* _h, db1_res_t* _r)
177 177
 	}
178 178
 
179 179
 	for(row = 0; row < RES_ROW_N(_r); row++) {
180
-		CON_ROW(_h) = mysql_fetch_row(CON_RESULT(_h));
181
-		if (!CON_ROW(_h)) {
180
+		RES_ROW(_r) = mysql_fetch_row(RES_RESULT(_r));
181
+		if (!RES_ROW(_r)) {
182 182
 			LM_ERR("driver error: %s\n", mysql_error(CON_CONNECTION(_h)));
183 183
 			RES_ROW_N(_r) = row;
184 184
 			db_free_rows(_r);
... ...
@@ -221,3 +221,22 @@ int db_mysql_convert_result(const db1_con_t* _h, db1_res_t* _r)
221 221
 	return 0;
222 222
 }
223 223
 
224
+
225
+/*!
226
+ * \brief Allocate new result set with private structure
227
+ * \return db1_res_t object on success, NULL on failure
228
+ */
229
+db1_res_t* db_mysql_new_result(void)
230
+{
231
+	db1_res_t* obj;
232
+
233
+	obj = db_new_result();
234
+	if (!obj)
235
+		return NULL;
236
+	RES_PTR(obj) = pkg_malloc(sizeof(struct my_res));
237
+	if (!RES_PTR(obj)) {
238
+		db_free_result(obj);
239
+		return NULL;
240
+	}
241
+	return obj;
242
+}
... ...
@@ -37,6 +37,18 @@
37 37
 #include "../../lib/srdb1/db_con.h"
38 38
 
39 39
 
40
+struct my_res {
41
+	MYSQL_RES* res;          /*!< Actual result */
42
+	MYSQL_ROW row;           /*!< Actual row in the result */
43
+};
44
+
45
+/*
46
+ * Some convenience wrappers
47
+ */
48
+#define RES_RESULT(db_res)     (((struct my_res*)((db_res)->ptr))->res)
49
+#define RES_ROW(db_res)        (((struct my_res*)((db_res)->ptr))->row)
50
+
51
+
40 52
 /*!
41 53
  * \brief Fill the result structure with data from database
42 54
  * \param _h database connection
... ...
@@ -54,4 +66,11 @@ int db_mysql_convert_result(const db1_con_t* _h, db1_res_t* _r);
54 54
  */
55 55
 int db_mysql_get_columns(const db1_con_t* _h, db1_res_t* _r);
56 56
 
57
+
58
+/*!
59
+ * \brief Allocate new result set with private structure
60
+ * \return db1_res_t object on success, NULL on failure
61
+ */
62
+db1_res_t* db_mysql_new_result(void);
63
+
57 64
 #endif
... ...
@@ -36,6 +36,7 @@
36 36
 #include "km_my_con.h"
37 37
 #include "km_val.h"
38 38
 #include "km_row.h"
39
+#include "km_res.h"
39 40
 
40 41
 /*!
41 42
  * \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)
59 59
 		return -2;
60 60
 	}
61 61
 	
62
-	lengths = mysql_fetch_lengths(CON_RESULT(_h));
62
+	lengths = mysql_fetch_lengths(RES_RESULT(_res));
63 63
 
64 64
 	for(i = 0; i < RES_COL_N(_res); i++) {
65 65
 		if (db_str2val(RES_TYPES(_res)[i], &(ROW_VALUES(_r)[i]),
66
-			    ((MYSQL_ROW)CON_ROW(_h))[i], lengths[i], 0) < 0) {
66
+			    ((MYSQL_ROW)RES_ROW(_res))[i], lengths[i], 0) < 0) {
67 67
 			LM_ERR("failed to convert value\n");
68 68
 			LM_DBG("free row at %p\n", _r);
69 69
 			db_free_row(_r);