[apparmor] [PATCH v2 45/42] libapparmor: Don't leak memory after a realloc(3) failure

[Seth Arnold]

On Tue, Mar 24, 2015 at 05:06:37PM -0500, Tyler Hicks wrote:
> realloc() returns NULL when it fails. Using the same pointer to specify
> the buffer to reallocate *and* to store realloc()'s return value will
> result in a leak of the previously allocated buffer upon error.
> 
> These issues were discovered by cppcheck.
> 
> Note that 'buffer' in write_policy_fd_to_iface() has the autofree
> attribute so it must not be manually freed if the realloc(3) fails as
> it'll be automatically freed.
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>

Acked-by: Seth Arnold <seth.arnold at canonical.com>

Thanks

> ---
>  libraries/libapparmor/src/kernel.c           | 18 ++++++++++++++----
>  libraries/libapparmor/src/kernel_interface.c |  6 ++++--
>  2 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/libraries/libapparmor/src/kernel.c b/libraries/libapparmor/src/kernel.c
> index de856f7..9d5f45d 100644
> --- a/libraries/libapparmor/src/kernel.c
> +++ b/libraries/libapparmor/src/kernel.c
> @@ -288,10 +288,15 @@ int aa_getprocattr(pid_t tid, const char *attr, char **label, char **mode)
>  	}
>  
>  	do {
> +		char *tmp;
> +
>  		size <<= 1;
> -		buffer = realloc(buffer, size);
> -		if (!buffer)
> +		tmp = realloc(buffer, size);
> +		if (!tmp) {
> +			free(buffer);
>  			return -1;
> +		}
> +		buffer = tmp;
>  		memset(buffer, 0, size);
>  
>  		rc = aa_getprocattr_raw(tid, attr, buffer, size, mode);
> @@ -645,10 +650,15 @@ int aa_getpeercon(int fd, char **label, char **mode)
>  	}
>  
>  	do {
> +		char *tmp;
> +
>  		last_size = size;
> -		buffer = realloc(buffer, size);
> -		if (!buffer)
> +		tmp = realloc(buffer, size);
> +		if (!tmp) {
> +			free(buffer);
>  			return -1;
> +		}
> +		buffer = tmp;
>  		memset(buffer, 0, size);
>  
>  		rc = aa_getpeercon_raw(fd, buffer, &size, mode);
> diff --git a/libraries/libapparmor/src/kernel_interface.c b/libraries/libapparmor/src/kernel_interface.c
> index 24239ce..6ab20ea 100644
> --- a/libraries/libapparmor/src/kernel_interface.c
> +++ b/libraries/libapparmor/src/kernel_interface.c
> @@ -159,13 +159,15 @@ static int write_policy_fd_to_iface(aa_kernel_interface *kernel_interface,
>  
>  	do {
>  		if (asize - size == 0) {
> -			buffer = realloc(buffer, chunksize);
> +			char *tmp = realloc(buffer, chunksize);
> +
>  			asize = chunksize;
>  			chunksize <<= 1;
> -			if (!buffer) {
> +			if (!tmp) {
>  				errno = ENOMEM;
>  				return -1;
>  			}
> +			buffer = tmp;
>  		}
>  
>  		rsize = read(fd, buffer + size, asize - size);
> -- 
> 2.1.4
> 
> 
> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150324/fb648f18/attachment.pgp>