On 28 September 2011 09:24, <a...@redhat.com> wrote:
> From: Alex Jia <a...@redhat.com>
>
> Haven't released memory of 'host_mb' in failure path, and calling malloc
> allocate
> memory to 'host_array', however, memory hasn't been freed before the function
> target_to_host_semarray returns.
>
> Signed-off-by: Alex Jia <a...@redhat.com>
> ---
> linux-user/syscall.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 7735008..22d4fcc 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2466,6 +2466,7 @@ static inline abi_long target_to_host_semarray(int
> semid, unsigned short **host_
> for(i=0; i<nsems; i++) {
> __get_user((*host_array)[i], &array[i]);
> }
> + free(*host_array);
> unlock_user(array, target_addr, 0);
>
> return 0;
This is wrong -- you're freeing the array in the exit-success
path, not the error path. And you're still not checking the
return value from malloc().
In fact, on closer examination, this code is pretty nasty:
we allocate the array in target_to_host_semarray and then
free it in host_to_target_semarray, and in both functions
we make a syscall purely to figure out the length of the
array -- so we end up doing that twice when we only need
do it once. And the error exit cases in host_to_target_semarray
don't free the host array either. It could probably be
refactored to make it less ugly: have the code at the
top level do the "find out size of array, malloc it, free"
work, and have host_to_target_semarray and
target_to_host_semarray only do the copying work (this
would match other target_to_host* helpers.)
-- PMM
