[PATCH] lib/ldb minor fixes
Kamen Mazdrashki
kamenim at samba.org
Tue Nov 11 17:28:45 MST 2014
Hi Andrew,
I have updated the patches according our discussion. See inline.
Also, there is one more commit that allows for adding MessageElements
from one Message object to another - at the moment such attempts lead
to crashes due to "Bad talloc magic value" error
As usual, attached is the whole patchset.
A pull request for convenient review is here:
https://github.com/kamenim/samba/pull/4
On Tue, Nov 11, 2014 at 10:29 AM, Kamen Mazdrashki <kamenim at samba.org>
wrote:
> Hi Andrew,
>
> On Tue, Nov 11, 2014 at 9:14 AM, Andrew Bartlett <abartlet at samba.org>
> wrote:
>
>> On Tue, 2014-11-11 at 05:29 +0100, Kamen Mazdrashki wrote:
>> > I have update the last patch to this:
>> >
>> https://github.com/kamenim/samba/commit/be1808de8bc5232dd9b827f93ab0d3151653ff88
>> > Also attached the whole set to this mail.
>>
>>
>> I still want it to instead check for
>>
>> if (msg->elements[i].name == NULL)
>>
>
> I have considered this also.
> I have opted out for this check to be done
> in ldb_schema_attribute_by_name_internal()
> though, as every call for finding attribute by name end up being handled
> by this function.
> Hence the sanity check handles much more code paths.
> I see the benefit of such check here though, so we can get a much better
> error message
>
>
Please take a look at updated patch here:
https://github.com/kamenim/samba/commit/2966a44cd900f597aed232c963693ab85bd33f4b
Let me know if this is what you had in mind too.
Cheers,
Kamen
-------------- next part --------------
From 9703d46a43ab5997a4c048e783cc05df3bf12fa7 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/6] 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 80708719197a39c7846580306e0cd12417967854 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/6] 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 af49a2ece923b76371c2d2edb816919a860fdd24 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/6] 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 194658167d65f0f96e7150b1f5e5652175830999 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/6] 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 2966a44cd900f597aed232c963693ab85bd33f4b Mon Sep 17 00:00:00 2001
From: Kamen Mazdrashki <kamenim at samba.org>
Date: Wed, 12 Nov 2014 01:12:31 +0100
Subject: [PATCH 5/6] lib-ldb_ldif: Stop processing if we can't resolve
ldb_message element name
I have hit this while using Python bindings for testing and
forgot to pass 'name' argument to MessageElement constructor
Perhaps there are other ways to exploit this crash too.
Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
---
lib/ldb/common/ldb_ldif.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/lib/ldb/common/ldb_ldif.c b/lib/ldb/common/ldb_ldif.c
index a2e4488..3855c44 100644
--- a/lib/ldb/common/ldb_ldif.c
+++ b/lib/ldb/common/ldb_ldif.c
@@ -312,7 +312,21 @@ static int ldb_ldif_write_trace(struct ldb_context *ldb,
for (i=0;i<msg->num_elements;i++) {
const struct ldb_schema_attribute *a;
+ if (msg->elements[i].name == NULL) {
+ ldb_debug(ldb, LDB_DEBUG_ERROR,
+ "Error: Invalid element name (NULL) at position %d", i);
+ talloc_free(mem_ctx);
+ return -1;
+ }
+
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
From 3df6829a4e4e06ca32dca3fe9f77ad82b224b224 Mon Sep 17 00:00:00 2001
From: Kamen Mazdrashki <kamenim at samba.org>
Date: Wed, 12 Nov 2014 01:17:56 +0100
Subject: [PATCH 6/6] lib-pyldb: Avoid crash when copying MessageElements
between Python Message objects
This patch allows for following snipets in Python:
res = ldb.search(...)
m_from = res[0]
m_to = Message()
m_to.add(m_from["attrName"])
The problem previously is that we are trying to reference a
ldb_message_element that may not be a memory context on its own.
For instance, when search request from above example returns
Messages with more than one attribute, this leads immediately
to "Bad talloc magic value" crash, every message element beside
the first one is not a memory context
Signed-off-by: Kamen Mazdrashki <kamenim at samba.org>
---
lib/ldb/pyldb.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index 40c802f..842a878 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -2781,15 +2781,25 @@ static PyObject *py_ldb_msg_add(PyLdbMessageObject *self, PyObject *args)
if (!PyArg_ParseTuple(args, "O!", &PyLdbMessageElement, &py_element))
return NULL;
- el = talloc_reference(msg, py_element->el);
+ el = py_element->el;
if (el == NULL) {
- PyErr_NoMemory();
+ PyErr_SetString(PyExc_ValueError, "Invalid MessageElement object");
return NULL;
}
ret = ldb_msg_add(msg, el, el->flags);
PyErr_LDB_ERROR_IS_ERR_RAISE(PyExc_LdbError, ret, NULL);
+ el = &(msg->elements[msg->num_elements - 1]);
+ if (talloc_reference(msg->elements, el->name) == NULL) {
+ PyErr_NoMemory();
+ return NULL;
+ }
+ if (talloc_reference(msg->elements, el->values) == NULL) {
+ PyErr_NoMemory();
+ return NULL;
+ }
+
Py_RETURN_NONE;
}
--
1.9.1
More information about the samba-technical
mailing list