ldb bug fixes

[Adrian Cochrane]

It turns out that wouldn't be an easier fix because 
/lib/talloc/talloc_guide.txt specifically states:
>   talloc_realloc(context, ptr, type, 0) ==> talloc_free(ptr);

However I did take your advice and incorporated it into this updated 
patch.

On Thu, Nov 19, 2015 at 8:25 PM, Volker Lendecke 
<Volker.Lendecke at SerNet.DE> wrote:
> On Thu, Nov 19, 2015 at 04:18:16PM +1300, Adrian Cochrane wrote:
>>  I caught and fixed a bug in LDB. This bug corrupted LDB database
>>  searches when that database contained an empty DN record.
>> 
>>  I've attached my fix, please review.
> 
>>  From 34cf7347a9d35254f70bc7455c4ac2a591573652 Mon Sep 17 00:00:00 
>> 2001
>>  From: Adrian Cochrane <adrianc at catalyst.net.nz>
>>  Date: Wed, 18 Nov 2015 15:25:20 +1300
>>  Subject: [PATCH] ldb: Fix bug triggered by having an empty message 
>> in database
>>   during search.
>> 
>>  Previously if the message had 0 elements, Talloc would reallocate 
>> the projected
>>  array to NULL, fooling LDB into thinking that it failed to 
>> reallocate. This fix
> 
> Does talloc_array(ptr, size, 0) really return NULL? I think
> it should not.
> 
>>  corrects LDB to be able to handle the case where the message has no 
>> attributes
>>  in common with the filter.
>> 
>>  Also the realloc call resized the array to the number of elements 
>> in the message,
>>  not the number of elements in common with the filter -- it 
>> essentially did nothing.
>> 
>>  Signed-off-by: Adrian Cochrane <adrianc at catalyst.net.nz>
>>  ---
>>   lib/ldb/ldb_tdb/ldb_search.c | 14 +++++++++++---
>>   lib/ldb/tests/python/api.py  | 29 +++++++++++++++++++++++++----
>>   2 files changed, 36 insertions(+), 7 deletions(-)
>> 
>>  diff --git a/lib/ldb/ldb_tdb/ldb_search.c 
>> b/lib/ldb/ldb_tdb/ldb_search.c
>>  index 1e7e7ea..7055af1 100644
>>  --- a/lib/ldb/ldb_tdb/ldb_search.c
>>  +++ b/lib/ldb/ldb_tdb/ldb_search.c
>>  @@ -407,10 +407,18 @@ int ltdb_filter_attrs(struct ldb_message 
>> *msg, const char * const *attrs)
>>   	}
>> 
>>   	talloc_free(msg->elements);
>>  -	msg->elements = talloc_realloc(msg, el2, struct 
>> ldb_message_element, msg->num_elements);
>>  -	if (msg->elements == NULL) {
>>  -		return -1;
>>  +
>>  +	if (num_elements == 0) {
>>  +		msg->elements = NULL;
>>  +		talloc_free(el2);
> 
> Do you really want to set msg->elements to NULL? This is in
> line with num_elements==0, but a non-NULL pointer might be
> safer. On the other hand, a NULL pointer will point out bugs
> quicker.
> 
>>  +	} else if (num_elements > 0) {
> 
> num_elements is unsigned, isn't this if clause redundant?
> 
>>  +		msg->elements = talloc_realloc(msg, el2, struct 
>> ldb_message_element,
>>  +					       num_elements);
>>  +		if (msg->elements == NULL) {
>>  +			return -1;
>>  +		}
>>   	}
>>  +
> 
> A simpler patch might be a one-liner to just realloc to
> num_elements instead of msg->num_elements.
> 
>>   	msg->num_elements = num_elements;
> 
> ... or move this line up a bit :-)
> 
> Volker
> 
> --
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ldb-Fix-bug-triggered-by-having-an-empty-message-in-.patch
Type: text/x-patch
Size: 4045 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20151123/57e77b87/0001-ldb-Fix-bug-triggered-by-having-an-empty-message-in-.bin>