[PATCH] lib/ldb minor fixes

Kamen Mazdrashki kamenim at samba.org
Mon Nov 10 21:29:44 MST 2014


I have update the last patch to this:

https://github.com/kamenim/samba/commit/be1808de8bc5232dd9b827f93ab0d3151653ff88
Also attached the whole set to this mail.

Cheers,
Kamen


On Tue, Nov 11, 2014 at 5:25 AM, Kamen Mazdrashki <kamenim at samba.org> wrote:

> Hi Andrew,
>
> Actually there is a bug with the last patche indeed
> The assertion 'if (a == NULL || a->name == NULL)' is too strong and I stop
> processing for default ldb attribute definition (which is with name ==
> NULL)
>
> I have changed it to 'if (a == NULL)' and now everything is fine
>
> Cheers,
> Kamen
>
>
> On Tue, Nov 11, 2014 at 3:45 AM, Kamen Mazdrashki <kamenim at samba.org>
> wrote:
>
>> Hi Andrew,
>>
>> On Tue, Nov 11, 2014 at 3:32 AM, Andrew Bartlett <abartlet at samba.org>
>> wrote:
>>
>>> G'Day Kamen,
>>>
>>> I'll push the rest of these shortly, but the last patch I'm not
>>> comfortable with.  The code should be written such that this cannot
>>> fail, and we should be able to print out the structure as LDIF even if
>>> we don't fully understand it - otherwise debugging is going to get
>>> really hard.
>>>
>>> What was the situation you hit this on?
>>>
>>> Pretty rookie actually - I have created message elements like
>>   m = Message()
>>   m[attr] = MessageElement(value)
>> and it segfault because there is no name for the attribute.
>> First it is going to segfault because of this:
>>
>> https://github.com/kamenim/samba/commit/f69a5f6618f64d6c6b584d90bfe1d61a62970deb
>> and then in 'ldb_ldif_write_trace' because now '
>> ldb_schema_attribute_by_name' doesn't segfault
>> but fail gracefully.
>>
>> Cheers,
>> Kamen
>>
>>
>>
>>> Thanks,
>>>
>>> Andrew Bartlett
>>>
>>> On Tue, 2014-11-11 at 03:20 +0100, Kamen Mazdrashki wrote:
>>> > From: Kamen Mazdrashki <kamenim at samba.org>
>>> > Date: Tue, 11 Nov 2014 02:57:50 +0100
>>> > Subject: [PATCH 5/5] lib-ldb_ldif: Stop processing if we can't resolve
>>> >  ldb_message element name
>>> >
>>> > This also prevent a segfault when using Python binding
>>> >
>>> > Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
>>> > ---
>>> >  lib/ldb/common/ldb_ldif.c | 7 +++++++
>>> >  1 file changed, 7 insertions(+)
>>> >
>>> > diff --git a/lib/ldb/common/ldb_ldif.c b/lib/ldb/common/ldb_ldif.c
>>> > index a2e4488..eb8179a 100644
>>> > --- a/lib/ldb/common/ldb_ldif.c
>>> > +++ b/lib/ldb/common/ldb_ldif.c
>>> > @@ -313,6 +313,13 @@ static int ldb_ldif_write_trace(struct
>>> > ldb_context *ldb,
>>> >                 const struct ldb_schema_attribute *a;
>>> >
>>> >                 a = ldb_schema_attribute_by_name(ldb,
>>> > msg->elements[i].name);
>>> > +               if (a == NULL || a->name == NULL) {
>>> > +                       ldb_debug(ldb, LDB_DEBUG_ERROR, "Error:
>>> > ldb_schema_attribute_by_name() "
>>> > +                                       "failed for Message element at
>>> > %d with name %s.",
>>> > +                                       i, msg->elements[i].name);
>>> > +                       talloc_free(mem_ctx);
>>> > +                       return -1;
>>> > +               }
>>> >
>>> >                 if (ldif->changetype == LDB_CHANGETYPE_MODIFY) {
>>> >                         switch (msg->elements[i].flags &
>>> > LDB_FLAG_MOD_MASK) {
>>>
>>> --
>>> Andrew Bartlett
>>> http://samba.org/~abartlet/
>>> Authentication Developer, Samba Team  http://samba.org
>>> Samba Developer, Catalyst IT
>>> http://catalyst.net.nz/services/samba
>>>
>>>
>>>
>>>
>>>
>>
>
-------------- next part --------------
From 2655d13adae82d7a1f1c29ce0c22d216673c9b60 Mon Sep 17 00:00:00 2001
From: Kamen Mazdrashki <kamenim at samba.org>
Date: Sun, 9 Nov 2014 04:28:47 +0100
Subject: [PATCH 1/5] lib-pyldb: Avoid SEGFAUL in case we can't convert passed
 valud to py-String

Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
---
 lib/ldb/pyldb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index 78b8012..c541b72 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -1629,6 +1629,11 @@ static PyObject *py_ldb_schema_format_value(PyLdbObject *self, PyObject *args)
 	old_val.data = (uint8_t *)PyString_AsString(val);
 	old_val.length = PyString_Size(val);
 
+	if (old_val.data == NULL) {
+		PyErr_SetString(PyExc_RuntimeError, "Failed to convert passed value to String");
+		return NULL;
+	}
+
 	a = ldb_schema_attribute_by_name(pyldb_Ldb_AsLdbContext(self), element_name);
 
 	if (a == NULL) {
-- 
1.9.1


From b1964a61f69aa4eb6f67c1d2458d4abd5c8e4df9 Mon Sep 17 00:00:00 2001
From: Kamen Mazdrashki <kamenim at samba.org>
Date: Sun, 9 Nov 2014 04:31:36 +0100
Subject: [PATCH 2/5] lib-pyldb: Avoid leaking memory in error cases

Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
---
 lib/ldb/pyldb.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index c541b72..efac7b1 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -1624,8 +1624,6 @@ static PyObject *py_ldb_schema_format_value(PyLdbObject *self, PyObject *args)
 	if (!PyArg_ParseTuple(args, "sO", &element_name, &val))
 		return NULL;
 
-	mem_ctx = talloc_new(NULL);
-
 	old_val.data = (uint8_t *)PyString_AsString(val);
 	old_val.length = PyString_Size(val);
 
@@ -1640,6 +1638,12 @@ static PyObject *py_ldb_schema_format_value(PyLdbObject *self, PyObject *args)
 		Py_RETURN_NONE;
 	}
 
+	mem_ctx = talloc_new(NULL);
+	if (mem_ctx == NULL) {
+		PyErr_NoMemory();
+		return NULL;
+	}
+
 	if (a->syntax->ldif_write_fn(pyldb_Ldb_AsLdbContext(self), mem_ctx, &old_val, &new_val) != 0) {
 		talloc_free(mem_ctx);
 		Py_RETURN_NONE;
-- 
1.9.1


From fc1134c05a9e183bed07929835bfcef2ba8496d5 Mon Sep 17 00:00:00 2001
From: Kamen Mazdrashki <kamenim at samba.org>
Date: Mon, 10 Nov 2014 22:59:07 +0100
Subject: [PATCH 3/5] lib-pyldb: Throw exception when we can't create
 MessageElement object

At the moment we return an error, but no exception and it is
hard to instantly see what the problem is from Python

Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
---
 lib/ldb/pyldb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index efac7b1..40c802f 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -2352,6 +2352,8 @@ static struct ldb_message_element *PyObject_AsMessageElement(
 				(uint8_t *)PyString_AsString(obj), me->values[i].length+1);
 		}
 	} else {
+		PyErr_Format(PyExc_TypeError,
+			     "String or List type expected for '%s' attribute", attr_name);
 		talloc_free(me);
 		me = NULL;
 	}
-- 
1.9.1


From f69a5f6618f64d6c6b584d90bfe1d61a62970deb Mon Sep 17 00:00:00 2001
From: Kamen Mazdrashki <kamenim at samba.org>
Date: Tue, 11 Nov 2014 02:56:32 +0100
Subject: [PATCH 4/5] lib-ldb: Check for input parameter when searching
 attributes by name

This prevents a segfault that is hard to be tracked down from
Python bindings

Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
---
 lib/ldb/common/ldb_attributes.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/ldb/common/ldb_attributes.c b/lib/ldb/common/ldb_attributes.c
index 21a3e6e..4132bc9 100644
--- a/lib/ldb/common/ldb_attributes.c
+++ b/lib/ldb/common/ldb_attributes.c
@@ -127,6 +127,11 @@ static const struct ldb_schema_attribute *ldb_schema_attribute_by_name_internal(
 	int r;
 	const struct ldb_schema_attribute *def = &ldb_attribute_default;
 
+	/* attribute name must be valid pointer */
+	if (name == NULL) {
+		return NULL;
+	}
+
 	/* as handlers are sorted, '*' must be the first if present */
 	if (strcmp(ldb->schema.attributes[0].name, "*") == 0) {
 		def = &ldb->schema.attributes[0];
-- 
1.9.1


From be1808de8bc5232dd9b827f93ab0d3151653ff88 Mon Sep 17 00:00:00 2001
From: Kamen Mazdrashki <kamenim at samba.org>
Date: Tue, 11 Nov 2014 05:26:24 +0100
Subject: [PATCH 5/5] lib-ldb_ldif: Stop processing if we can't resolve
 ldb_message element name

This also prevent a segfault when using Python binding

Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
---
 lib/ldb/common/ldb_ldif.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/ldb/common/ldb_ldif.c b/lib/ldb/common/ldb_ldif.c
index a2e4488..9980135 100644
--- a/lib/ldb/common/ldb_ldif.c
+++ b/lib/ldb/common/ldb_ldif.c
@@ -313,6 +313,13 @@ static int ldb_ldif_write_trace(struct ldb_context *ldb,
 		const struct ldb_schema_attribute *a;
 
 		a = ldb_schema_attribute_by_name(ldb, msg->elements[i].name);
+		if (a == NULL) {
+			ldb_debug(ldb, LDB_DEBUG_ERROR, "Error: ldb_schema_attribute_by_name() "
+					"failed for Message element at %d with name %s.",
+					i, msg->elements[i].name);
+			talloc_free(mem_ctx);
+			return -1;
+		}
 
 		if (ldif->changetype == LDB_CHANGETYPE_MODIFY) {
 			switch (msg->elements[i].flags & LDB_FLAG_MOD_MASK) {
-- 
1.9.1


More information about the samba-technical mailing list