[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] linux-user: fix memory leak in failure path

From: Alex Jia
Subject: Re: [Qemu-devel] [PATCH] linux-user: fix memory leak in failure path
Date: Wed, 28 Sep 2011 16:27:36 +0800
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110830 Red Hat/3.1.12-2.el6_1 Thunderbird/3.1.12

On 09/28/2011 03:55 PM, Peter Maydell wrote:
On 28 September 2011 07:57,<address@hidden>  wrote:
From: Alex Jia<address@hidden>

Haven't released memory of 'array' and 'host_mb' in failure paths.

Signed-off-by: Alex Jia<address@hidden>
  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.

Yeah, caller shouldn't do this.
@@ -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);

+    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.
Yeah, for return value check of malloc(), it seems many places haven't
do it.

-- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]