ldb bug fixes

[Volker Lendecke]

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