[PATCH] ldb: check return values

[Andrej Gessel]

Hello Jeremy,

Hello Volker,

I updated my patches. Current Gitlab pipeline: 
https://gitlab.com/samba-team/devel/samba/pipelines/24335681


Thanks,

Andrej



Am 21.06.2018 um 18:59 schrieb Jeremy Allison:
> On Thu, Jun 21, 2018 at 04:21:03PM +0000, Andrej Gessel via samba-technical wrote:
>> Hello Volker,
>>
>>
>> yes, you are right. I found some more of another mem leaks like this one. So I can resent the patchset with additional changes or you can review this one and i open new thread for other changes.
>>
>> What would you prefer?
> Can you update this patch with the fixes for the mem
> leak in this specific function ? We can then review and
> push as-is, then you can send more patches along as
> you find them.
>
> Jeremy

-------------- next part --------------
From f84042cb2c2d4ca8ced772b1eb39b2f2ebb84bb2 Mon Sep 17 00:00:00 2001
From: Andrej Gessel <Andrej.Gessel at janztec.com>
Date: Fri, 15 Jun 2018 11:02:15 +0200
Subject: [PATCH 1/2] ldb: check return values

Signed-off-by: Andrej Gessel <Andrej.Gessel at janztec.com>
---
 lib/ldb/ldb_tdb/ldb_index.c  | 7 +++++++
 lib/ldb/ldb_tdb/ldb_search.c | 5 ++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index d59b4b1..9ef786f 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -450,6 +450,10 @@ normal_index:
 
 		list->count = el->values[0].length / LTDB_GUID_SIZE;
 		list->dn = talloc_array(list, struct ldb_val, list->count);
+		if (list->dn == NULL) {
+			talloc_free(msg);
+			return LDB_ERR_OPERATIONS_ERROR;
+		}
 
 		/*
 		 * The actual data is on msg, due to
@@ -715,6 +719,9 @@ static int ltdb_dn_list_store(struct ldb_module *module, struct ldb_dn *dn,
 	}
 
 	key.dptr = discard_const_p(unsigned char, ldb_dn_get_linearized(dn));
+	if (key.dptr == NULL) {
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
 	key.dsize = strlen((char *)key.dptr);
 
 	rec = tdb_fetch(ltdb->idxptr->itdb, key);
diff --git a/lib/ldb/ldb_tdb/ldb_search.c b/lib/ldb/ldb_tdb/ldb_search.c
index 18f8405..832be9a 100644
--- a/lib/ldb/ldb_tdb/ldb_search.c
+++ b/lib/ldb/ldb_tdb/ldb_search.c
@@ -102,8 +102,11 @@ static int msg_add_distinguished_name(struct ldb_message *msg)
 	el.values = &val;
 	el.flags = 0;
 	val.data = (uint8_t *)ldb_dn_alloc_linearized(msg, msg->dn);
+	if (val.data == NULL) {
+		return -1;
+	}
 	val.length = strlen((char *)val.data);
-	
+
 	ret = msg_add_element(msg, &el, 1);
 	return ret;
 }
-- 
2.7.4


From d2990bcc83cbc252e6b4b1d279f3f565afcfabe7 Mon Sep 17 00:00:00 2001
From: Andrej Gessel <Andrej.Gessel at janztec.com>
Date: Tue, 19 Jun 2018 10:07:51 +0200
Subject: [PATCH 2/2] check return value before using key_values

there are also mem leaks in this function

Signed-off-by: Andrej Gessel <Andrej.Gessel at janztec.com>
---
 lib/ldb/ldb_tdb/ldb_index.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index 9ef786f..fb60612 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -1765,13 +1765,14 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 					  struct guid_tdb_key,
 					  dn_list->count);
 
+		if (key_values == NULL) {
+			talloc_free(keys);
+			return ldb_module_oom(ac->module);
+		}
 		for (i = 0; i < dn_list->count; i++) {
 			keys[i].dptr = key_values[i].guid_key;
 			keys[i].dsize = sizeof(key_values[i].guid_key);
 		}
-		if (key_values == NULL) {
-			return ldb_module_oom(ac->module);
-		}
 	} else {
 		for (i = 0; i < dn_list->count; i++) {
 			keys[i].dptr = NULL;
@@ -1788,6 +1789,7 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 				      &dn_list->dn[i],
 				      &keys[num_keys]);
 		if (ret != LDB_SUCCESS) {
+			talloc_free(keys);
 			return ret;
 		}
 
@@ -1825,6 +1827,7 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 		bool matched;
 		msg = ldb_msg_new(ac);
 		if (!msg) {
+			talloc_free(keys);
 			return LDB_ERR_OPERATIONS_ERROR;
 		}
 
@@ -1845,6 +1848,7 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 
 		if (ret != LDB_SUCCESS && ret != LDB_ERR_NO_SUCH_OBJECT) {
 			/* an internal error */
+			talloc_free(keys);
 			talloc_free(msg);
 			return LDB_ERR_OPERATIONS_ERROR;
 		}
@@ -1867,6 +1871,7 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 		}
 
 		if (ret != LDB_SUCCESS) {
+			talloc_free(keys);
 			talloc_free(msg);
 			return ret;
 		}
@@ -1881,6 +1886,7 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 		talloc_free(msg);
 
 		if (ret == -1) {
+			talloc_free(keys);
 			return LDB_ERR_OPERATIONS_ERROR;
 		}
 
@@ -1890,6 +1896,7 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 			 * is the callbacks responsiblity, and should
 			 * not be talloc_free()'ed */
 			ac->request_terminated = true;
+			talloc_free(keys);
 			return ret;
 		}
 
-- 
2.7.4