[Freeipa-devel] [PATCHES] [bind-dyndb-ldap] Two patches for minor Coverity issues

[Adam Tkac]

On Tue, Jan 04, 2011 at 03:41:12PM -0500, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Patch 0001: Fix missing varargs cleanup
> 
> The CHECK() macro may cause execution to skip down to the cleanup
> tag. If this happens, it would mean that we never called va_end()
> on "backup".
> 
> This patch reorganizes the code slightly to ensure that va_end()
> is always called.
> 
> 
> Patch 0002: Fix potential out-of-bounds write
> 
> If there are exactly LD_MAX_SPLITS entries resulting from this
> split, the mandatory trailing NULL entry will be written to one
> entry past the end of the static arrayof LD_MAX_SPLITS size.

Both patches look fine for me, ack. Please push them.

Regards, Adam

> - -- 
> Stephen Gallagher
> RHCE 804006346421761
> 
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAk0jhegACgkQeiVVYja6o6PGlwCgnO1jSmW1VhO3kJh3C818655M
> DaEAoK5b0f4VLiRkkKgMaJnGrjRoHv9+
> =XJeu
> -----END PGP SIGNATURE-----

> From 4cc3a923c1e26ac4c286afd47df1d823920ef56b Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Tue, 4 Jan 2011 15:28:46 -0500
> Subject: [PATCH 1/2] Fix missing varargs cleanup
> 
> The CHECK() macro may cause execution to skip down to the cleanup
> tag. If this happens, it would mean that we never called va_end()
> on "backup".
> 
> This patch reorganizes the code slightly to ensure that va_end()
> is always called.
> ---
>  src/str.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/str.c b/src/str.c
> index b975aac7ba8c1028a71ac499dfe39530aba4e61f..611ae2028ec06d2e8e9e270eb6a6e0eaa37adcae 100644
> --- a/src/str.c
> +++ b/src/str.c
> @@ -431,16 +431,16 @@ str_vsprintf(ld_string_t *dest, const char *format, va_list ap)
>  		CHECK(str_alloc(dest, len));
>  		len = vsnprintf(dest->data, dest->allocated, format, backup);
>  	}
> -	va_end(backup);
>  
>  	if (len < 0) {
>  		result = ISC_R_FAILURE;
>  		goto cleanup;
>  	}
>  
> -	return ISC_R_SUCCESS;
> +	result = ISC_R_SUCCESS;
>  
>  cleanup:
> +	va_end(backup);
>  	return result;
>  }
>  
> -- 
> 1.7.3.4
> 

> From 93d709e47444ba38c314b4cece980a829c4f23b9 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Tue, 4 Jan 2011 15:33:02 -0500
> Subject: [PATCH 2/2] Fix potential out-of-bounds write
> 
> If there are exactly LD_MAX_SPLITS entries resulting from this
> split, the mandatory trailing NULL entry will be written to one
> entry past the end of the static arrayof LD_MAX_SPLITS size.
> ---
>  src/str.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/str.c b/src/str.c
> index 611ae2028ec06d2e8e9e270eb6a6e0eaa37adcae..56faa12dce3c7c7bde59d947b69907b9f63d315d 100644
> --- a/src/str.c
> +++ b/src/str.c
> @@ -570,7 +570,7 @@ str_split(const ld_string_t *src, const char delimiter, ld_split_t *split)
>  	current_pos = 0;
>  	save = 1;
>  	for (unsigned int i = 0;
> -	     i < split->allocated && current_pos < LD_MAX_SPLITS;
> +	     i < split->allocated && current_pos < LD_MAX_SPLITS - 1;
>  	     i++) {
>  		if (save && split->data[i] != '\0') {
>  			split->splits[current_pos] = split->data + i;
> -- 
> 1.7.3.4
> 



> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel


-- 
Adam Tkac, Red Hat, Inc.