On 28 September 2011 07:57, <a...@redhat.com> wrote:
> From: Alex Jia <a...@redhat.com>
>
> Haven't released memory of 'array' and 'host_mb' in failure paths.
>
> Signed-off-by: Alex Jia <a...@redhat.com>
> ---
> linux-user/syscall.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 7735008..922c2a0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2523,8 +2523,10 @@ static inline abi_long do_semctl(int semid, int
> semnum, int cmd,
> case GETALL:
> case SETALL:
> err = target_to_host_semarray(semid, &array, target_su.array);
> - if (err)
> + if (err) {
> + free(array);
> return err;
> + }
> arg.array = array;
> ret = get_errno(semctl(semid, semnum, cmd, arg));
> err = host_to_target_semarray(semid, target_su.array, &array);
This is the wrong place to try to fix this. If target_to_host_semarray
fails it should free() the buffer it malloc()ed itself, not rely on
its caller to do the cleanup.
> @@ -2779,9 +2781,9 @@ static inline abi_long do_msgrcv(int msqid, abi_long
> msgp,
> }
>
> target_mb->mtype = tswapl(host_mb->mtype);
> - free(host_mb);
>
> end:
> + free(host_mb);
> if (target_mb)
> unlock_user_struct(target_mb, msgp, 1);
> return ret;
This change is OK.
Also I note that target_to_host_semarray is doing a plain malloc()
and not checking the return value. You should fix that while you're
doing fixes in this area.
-- PMM
