[Qemu-devel] [PATCH] fix memory leak in aio_write_f

[Qemu-devel] [PATCH] fix memory leak in aio_write_f

[ajia]

From: Alex Jia <ajia@redhat.com>

Haven't released memory of 'ctx' before return.

Signed-off-by: Alex Jia <ajia@redhat.com>
---
 qemu-io.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index e91af37..c45a413 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -1248,6 +1248,7 @@ static int aio_write_f(int argc, char **argv)
         case 'P':
             pattern = parse_pattern(optarg);
             if (pattern < 0) {
+                free(ctx);
                 return 0;
             }
             break;
-- 
1.7.1

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

[ajia]

From: Alex Jia <ajia@redhat.com>

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

Signed-off-by: Alex Jia <ajia@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);
@@ -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;
-- 
1.7.1

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

[Peter Maydell]

On 28 September 2011 07:57,  <ajia@redhat.com> wrote:
> From: Alex Jia <ajia@redhat.com>
>
> Haven't released memory of 'array' and 'host_mb' in failure paths.
>
> Signed-off-by: Alex Jia <ajia@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

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

[Alex Jia]

On 09/28/2011 03:55 PM, Peter Maydell wrote:
> On 28 September 2011 07:57,<ajia@redhat.com>  wrote:
>> From: Alex Jia<ajia@redhat.com>
>>
>> Haven't released memory of 'array' and 'host_mb' in failure paths.
>>
>> Signed-off-by: Alex Jia<ajia@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.
>
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);
>>
>>   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.
Yeah, for return value check of malloc(), it seems many places haven't
do it.

Thanks,
Alex
> -- PMM

Re: [Qemu-devel] [PATCH] fix memory leak in aio_write_f

[Kevin Wolf]

Am 28.09.2011 08:57, schrieb ajia@redhat.com:
> From: Alex Jia <ajia@redhat.com>
> 
> Haven't released memory of 'ctx' before return.
> 
> Signed-off-by: Alex Jia <ajia@redhat.com>

Thanks, applied to the block branch.

Kevin