* [Qemu-devel] [PATCH v2] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0()
@ 2015-10-01 5:32 Harmandeep Kaur
2015-10-01 15:57 ` Eric Blake
2015-10-01 16:03 ` Peter Maydell
0 siblings, 2 replies; 6+ messages in thread
From: Harmandeep Kaur @ 2015-10-01 5:32 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Riku Voipio
[-- Attachment #1: Type: text/plain, Size: 4266 bytes --]
Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
in linux-user/syscall.c file
v1->v2 convert the free() call in host_to_target_semarray()
to g_free() and calls g_try_malloc(count) instead of
g_try_malloc(sizeof(count))
Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
---
linux-user/syscall.c | 38 +++++++++++---------------------------
1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d1d3eb2..c79e862 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1554,12 +1554,7 @@ set_timeout:
}
fprog.len = tswap16(tfprog->len);
- filter = malloc(fprog.len * sizeof(*filter));
- if (filter == NULL) {
- unlock_user_struct(tfilter, tfprog->filter, 1);
- unlock_user_struct(tfprog, optval_addr, 1);
- return -TARGET_ENOMEM;
- }
+ filter = g_malloc(fprog.len * sizeof(*filter));
for (i = 0; i < fprog.len; i++) {
filter[i].code = tswap16(tfilter[i].code);
filter[i].jt = tfilter[i].jt;
@@ -1570,7 +1565,7 @@ set_timeout:
ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
SO_ATTACH_FILTER, &fprog, sizeof(fprog)));
- free(filter);
+ g_free(filter);
unlock_user_struct(tfilter, tfprog->filter, 1);
unlock_user_struct(tfprog, optval_addr, 1);
@@ -1881,11 +1876,7 @@ static struct iovec *lock_iovec(int type, abi_ulong
target_addr,
return NULL;
}
- vec = calloc(count, sizeof(struct iovec));
- if (vec == NULL) {
- errno = ENOMEM;
- return NULL;
- }
+ vec = g_new0(struct iovec, count);
target_vec = lock_user(VERIFY_READ, target_addr,
count * sizeof(struct target_iovec), 1);
@@ -1945,7 +1936,7 @@ static struct iovec *lock_iovec(int type, abi_ulong
target_addr,
}
unlock_user(target_vec, target_addr, 0);
fail2:
- free(vec);
+ g_free(vec);
errno = err;
return NULL;
}
@@ -2672,14 +2663,11 @@ static inline abi_long target_to_host_semarray(int
semid, unsigned short **host_
nsems = semid_ds.sem_nsems;
- *host_array = malloc(nsems*sizeof(unsigned short));
- if (!*host_array) {
- return -TARGET_ENOMEM;
- }
+ *host_array = g_malloc(nsems*sizeof(unsigned short));
array = lock_user(VERIFY_READ, target_addr,
nsems*sizeof(unsigned short), 1);
if (!array) {
- free(*host_array);
+ g_free(*host_array);
return -TARGET_EFAULT;
}
@@ -2716,7 +2704,7 @@ static inline abi_long host_to_target_semarray(int
semid, abi_ulong target_addr,
for(i=0; i<nsems; i++) {
__put_user((*host_array)[i], &array[i]);
}
- free(*host_array);
+ g_free(*host_array);
unlock_user(array, target_addr, 1);
return 0;
@@ -2975,15 +2963,11 @@ static inline abi_long do_msgsnd(int msqid,
abi_long msgp,
if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
return -TARGET_EFAULT;
- host_mb = malloc(msgsz+sizeof(long));
- if (!host_mb) {
- unlock_user_struct(target_mb, msgp, 0);
- return -TARGET_ENOMEM;
- }
+ host_mb = g_malloc(msgsz+sizeof(long));
host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
memcpy(host_mb->mtext, target_mb->mtext, msgsz);
ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
- free(host_mb);
+ g_free(host_mb);
unlock_user_struct(target_mb, msgp, 0);
return ret;
@@ -7625,7 +7609,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
struct linux_dirent *dirp;
abi_long count = arg3;
- dirp = malloc(count);
+ dirp = g_try_malloc(count);
if (!dirp) {
ret = -TARGET_ENOMEM;
goto fail;
@@ -7662,7 +7646,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = count1;
unlock_user(target_dirp, arg2, ret);
}
- free(dirp);
+ g_free(dirp);
}
#else
{
--
1.9.1
[-- Attachment #2: Type: text/html, Size: 5526 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0()
2015-10-01 5:32 [Qemu-devel] [PATCH v2] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0() Harmandeep Kaur
@ 2015-10-01 15:57 ` Eric Blake
2015-10-01 16:00 ` Harmandeep Kaur
2015-10-01 16:03 ` Peter Maydell
1 sibling, 1 reply; 6+ messages in thread
From: Eric Blake @ 2015-10-01 15:57 UTC (permalink / raw)
To: Harmandeep Kaur, qemu-devel; +Cc: Stefan Hajnoczi, Riku Voipio
[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]
On 09/30/2015 11:32 PM, Harmandeep Kaur wrote:
> Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
> in linux-user/syscall.c file
This part is fine,
>
> v1->v2 convert the free() call in host_to_target_semarray()
> to g_free() and calls g_try_malloc(count) instead of
> g_try_malloc(sizeof(count))
but this part belongs...
>
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> ---
...here, after the --- separator. It is useful information to mail
reviewers, but worthless in the qemu.git history (a year from now, we
won't care how many versions it went through on the list, only the
version that got checked in).
> +++ b/linux-user/syscall.c
> @@ -1554,12 +1554,7 @@ set_timeout:
> }
>
> fprog.len = tswap16(tfprog->len);
> - filter = malloc(fprog.len * sizeof(*filter));
> - if (filter == NULL) {
> - unlock_user_struct(tfilter, tfprog->filter, 1);
> - unlock_user_struct(tfprog, optval_addr, 1);
> - return -TARGET_ENOMEM;
> - }
> + filter = g_malloc(fprog.len * sizeof(*filter));
This should use g_new(TYPE, fprog.len) to avoid overflow issues.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0()
2015-10-01 15:57 ` Eric Blake
@ 2015-10-01 16:00 ` Harmandeep Kaur
0 siblings, 0 replies; 6+ messages in thread
From: Harmandeep Kaur @ 2015-10-01 16:00 UTC (permalink / raw)
To: Eric Blake; +Cc: Stefan Hajnoczi, Riku Voipio, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]
Thank you Eric for guiding me in the right direction.
I look forward to implement this correctly.
On Thu, Oct 1, 2015 at 9:27 PM, Eric Blake <eblake@redhat.com> wrote:
> On 09/30/2015 11:32 PM, Harmandeep Kaur wrote:
> > Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
> > in linux-user/syscall.c file
>
> This part is fine,
>
> >
> > v1->v2 convert the free() call in host_to_target_semarray()
> > to g_free() and calls g_try_malloc(count) instead of
> > g_try_malloc(sizeof(count))
>
> but this part belongs...
>
> >
> > Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> > ---
>
> ...here, after the --- separator. It is useful information to mail
> reviewers, but worthless in the qemu.git history (a year from now, we
> won't care how many versions it went through on the list, only the
> version that got checked in).
>
>
> > +++ b/linux-user/syscall.c
> > @@ -1554,12 +1554,7 @@ set_timeout:
> > }
> >
> > fprog.len = tswap16(tfprog->len);
> > - filter = malloc(fprog.len * sizeof(*filter));
> > - if (filter == NULL) {
> > - unlock_user_struct(tfilter, tfprog->filter, 1);
> > - unlock_user_struct(tfprog, optval_addr, 1);
> > - return -TARGET_ENOMEM;
> > - }
> > + filter = g_malloc(fprog.len * sizeof(*filter));
>
> This should use g_new(TYPE, fprog.len) to avoid overflow issues.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>
[-- Attachment #2: Type: text/html, Size: 2459 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0()
2015-10-01 5:32 [Qemu-devel] [PATCH v2] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0() Harmandeep Kaur
2015-10-01 15:57 ` Eric Blake
@ 2015-10-01 16:03 ` Peter Maydell
2015-10-01 16:12 ` Harmandeep Kaur
1 sibling, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2015-10-01 16:03 UTC (permalink / raw)
To: Harmandeep Kaur; +Cc: Stefan Hajnoczi, Riku Voipio, qemu-devel
On 1 October 2015 at 06:32, Harmandeep Kaur <write.harmandeep@gmail.com> wrote:
> Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
> in linux-user/syscall.c file
>
> v1->v2 convert the free() call in host_to_target_semarray()
> to g_free() and calls g_try_malloc(count) instead of
> g_try_malloc(sizeof(count))
>
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> ---
> linux-user/syscall.c | 38 +++++++++++---------------------------
> 1 file changed, 11 insertions(+), 27 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d1d3eb2..c79e862 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1554,12 +1554,7 @@ set_timeout:
> }
>
> fprog.len = tswap16(tfprog->len);
> - filter = malloc(fprog.len * sizeof(*filter));
> - if (filter == NULL) {
> - unlock_user_struct(tfilter, tfprog->filter, 1);
> - unlock_user_struct(tfprog, optval_addr, 1);
> - return -TARGET_ENOMEM;
> - }
>
> + filter = g_malloc(fprog.len * sizeof(*filter));
fprog.len comes from the guest -- you can't use g_malloc.
> for (i = 0; i < fprog.len; i++) {
> filter[i].code = tswap16(tfilter[i].code);
> filter[i].jt = tfilter[i].jt;
> @@ -1570,7 +1565,7 @@ set_timeout:
>
> ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
> SO_ATTACH_FILTER, &fprog, sizeof(fprog)));
> - free(filter);
> + g_free(filter);
>
> unlock_user_struct(tfilter, tfprog->filter, 1);
> unlock_user_struct(tfprog, optval_addr, 1);
> @@ -1881,11 +1876,7 @@ static struct iovec *lock_iovec(int type, abi_ulong
> target_addr,
> return NULL;
> }
>
> - vec = calloc(count, sizeof(struct iovec));
> - if (vec == NULL) {
> - errno = ENOMEM;
> - return NULL;
> - }
> + vec = g_new0(struct iovec, count);
count comes from the guest -- you can't use g_new0.
>
> target_vec = lock_user(VERIFY_READ, target_addr,
> count * sizeof(struct target_iovec), 1);
> @@ -1945,7 +1936,7 @@ static struct iovec *lock_iovec(int type, abi_ulong
> target_addr,
> }
> unlock_user(target_vec, target_addr, 0);
> fail2:
> - free(vec);
> + g_free(vec);
> errno = err;
> return NULL;
> }
> @@ -2672,14 +2663,11 @@ static inline abi_long target_to_host_semarray(int
> semid, unsigned short **host_
>
> nsems = semid_ds.sem_nsems;
>
> - *host_array = malloc(nsems*sizeof(unsigned short));
> - if (!*host_array) {
> - return -TARGET_ENOMEM;
> - }
> + *host_array = g_malloc(nsems*sizeof(unsigned short));
> array = lock_user(VERIFY_READ, target_addr,
> nsems*sizeof(unsigned short), 1);
I don't think we can guarantee that nsems is small -- you can't
use g_malloc.
> if (!array) {
> - free(*host_array);
> + g_free(*host_array);
> return -TARGET_EFAULT;
> }
>
> @@ -2716,7 +2704,7 @@ static inline abi_long host_to_target_semarray(int
> semid, abi_ulong target_addr,
> for(i=0; i<nsems; i++) {
> __put_user((*host_array)[i], &array[i]);
> }
> - free(*host_array);
> + g_free(*host_array);
> unlock_user(array, target_addr, 1);
>
> return 0;
> @@ -2975,15 +2963,11 @@ static inline abi_long do_msgsnd(int msqid, abi_long
> msgp,
>
> if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
> return -TARGET_EFAULT;
> - host_mb = malloc(msgsz+sizeof(long));
> - if (!host_mb) {
> - unlock_user_struct(target_mb, msgp, 0);
> - return -TARGET_ENOMEM;
> - }
> + host_mb = g_malloc(msgsz+sizeof(long));
msgsz comes from the guest -- you can't use g_malloc.
> host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
> memcpy(host_mb->mtext, target_mb->mtext, msgsz);
> ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
> - free(host_mb);
> + g_free(host_mb);
> unlock_user_struct(target_mb, msgp, 0);
>
> return ret;
> @@ -7625,7 +7609,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> struct linux_dirent *dirp;
> abi_long count = arg3;
>
> - dirp = malloc(count);
> + dirp = g_try_malloc(count);
> if (!dirp) {
> ret = -TARGET_ENOMEM;
> goto fail;
> @@ -7662,7 +7646,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> ret = count1;
> unlock_user(target_dirp, arg2, ret);
> }
> - free(dirp);
> + g_free(dirp);
> }
> #else
> {
> --
> 1.9.1
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0()
2015-10-01 16:03 ` Peter Maydell
@ 2015-10-01 16:12 ` Harmandeep Kaur
2015-10-01 16:23 ` Harmandeep Kaur
0 siblings, 1 reply; 6+ messages in thread
From: Harmandeep Kaur @ 2015-10-01 16:12 UTC (permalink / raw)
To: Peter Maydell; +Cc: Stefan Hajnoczi, Riku Voipio, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5411 bytes --]
Hi Peter,
Can you please tell me if I should use other glib
functions, or implement it in any other way?
I am newbie here :)
Thank you for reviewing my patch.
On Thu, Oct 1, 2015 at 9:33 PM, Peter Maydell <peter.maydell@linaro.org>
wrote:
> On 1 October 2015 at 06:32, Harmandeep Kaur <write.harmandeep@gmail.com>
> wrote:
> > Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
> > in linux-user/syscall.c file
> >
> > v1->v2 convert the free() call in host_to_target_semarray()
> > to g_free() and calls g_try_malloc(count) instead of
> > g_try_malloc(sizeof(count))
> >
> > Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> > ---
> > linux-user/syscall.c | 38 +++++++++++---------------------------
> > 1 file changed, 11 insertions(+), 27 deletions(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index d1d3eb2..c79e862 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -1554,12 +1554,7 @@ set_timeout:
> > }
> >
> > fprog.len = tswap16(tfprog->len);
> > - filter = malloc(fprog.len * sizeof(*filter));
> > - if (filter == NULL) {
> > - unlock_user_struct(tfilter, tfprog->filter, 1);
> > - unlock_user_struct(tfprog, optval_addr, 1);
> > - return -TARGET_ENOMEM;
> > - }
> >
> > + filter = g_malloc(fprog.len * sizeof(*filter));
>
> fprog.len comes from the guest -- you can't use g_malloc.
>
> > for (i = 0; i < fprog.len; i++) {
> > filter[i].code = tswap16(tfilter[i].code);
> > filter[i].jt = tfilter[i].jt;
> > @@ -1570,7 +1565,7 @@ set_timeout:
> >
> > ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
> > SO_ATTACH_FILTER, &fprog,
> sizeof(fprog)));
> > - free(filter);
> > + g_free(filter);
> >
> > unlock_user_struct(tfilter, tfprog->filter, 1);
> > unlock_user_struct(tfprog, optval_addr, 1);
> > @@ -1881,11 +1876,7 @@ static struct iovec *lock_iovec(int type,
> abi_ulong
> > target_addr,
> > return NULL;
> > }
> >
> > - vec = calloc(count, sizeof(struct iovec));
> > - if (vec == NULL) {
> > - errno = ENOMEM;
> > - return NULL;
> > - }
> > + vec = g_new0(struct iovec, count);
>
> count comes from the guest -- you can't use g_new0.
>
> >
> > target_vec = lock_user(VERIFY_READ, target_addr,
> > count * sizeof(struct target_iovec), 1);
> > @@ -1945,7 +1936,7 @@ static struct iovec *lock_iovec(int type, abi_ulong
> > target_addr,
> > }
> > unlock_user(target_vec, target_addr, 0);
> > fail2:
> > - free(vec);
> > + g_free(vec);
> > errno = err;
> > return NULL;
> > }
> > @@ -2672,14 +2663,11 @@ static inline abi_long
> target_to_host_semarray(int
> > semid, unsigned short **host_
> >
> > nsems = semid_ds.sem_nsems;
> >
> > - *host_array = malloc(nsems*sizeof(unsigned short));
> > - if (!*host_array) {
> > - return -TARGET_ENOMEM;
> > - }
> > + *host_array = g_malloc(nsems*sizeof(unsigned short));
> > array = lock_user(VERIFY_READ, target_addr,
> > nsems*sizeof(unsigned short), 1);
>
> I don't think we can guarantee that nsems is small -- you can't
> use g_malloc.
>
> > if (!array) {
> > - free(*host_array);
> > + g_free(*host_array);
> > return -TARGET_EFAULT;
> > }
> >
> > @@ -2716,7 +2704,7 @@ static inline abi_long host_to_target_semarray(int
> > semid, abi_ulong target_addr,
> > for(i=0; i<nsems; i++) {
> > __put_user((*host_array)[i], &array[i]);
> > }
> > - free(*host_array);
> > + g_free(*host_array);
> > unlock_user(array, target_addr, 1);
> >
> > return 0;
> > @@ -2975,15 +2963,11 @@ static inline abi_long do_msgsnd(int msqid,
> abi_long
> > msgp,
> >
> > if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
> > return -TARGET_EFAULT;
> > - host_mb = malloc(msgsz+sizeof(long));
> > - if (!host_mb) {
> > - unlock_user_struct(target_mb, msgp, 0);
> > - return -TARGET_ENOMEM;
> > - }
> > + host_mb = g_malloc(msgsz+sizeof(long));
>
> msgsz comes from the guest -- you can't use g_malloc.
>
> > host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
> > memcpy(host_mb->mtext, target_mb->mtext, msgsz);
> > ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
> > - free(host_mb);
> > + g_free(host_mb);
> > unlock_user_struct(target_mb, msgp, 0);
> >
> > return ret;
> > @@ -7625,7 +7609,7 @@ abi_long do_syscall(void *cpu_env, int num,
> abi_long
> > arg1,
> > struct linux_dirent *dirp;
> > abi_long count = arg3;
> >
> > - dirp = malloc(count);
> > + dirp = g_try_malloc(count);
> > if (!dirp) {
> > ret = -TARGET_ENOMEM;
> > goto fail;
> > @@ -7662,7 +7646,7 @@ abi_long do_syscall(void *cpu_env, int num,
> abi_long
> > arg1,
> > ret = count1;
> > unlock_user(target_dirp, arg2, ret);
> > }
> > - free(dirp);
> > + g_free(dirp);
> > }
> > #else
> > {
> > --
> > 1.9.1
>
> thanks
> -- PMM
>
[-- Attachment #2: Type: text/html, Size: 7339 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0()
2015-10-01 16:12 ` Harmandeep Kaur
@ 2015-10-01 16:23 ` Harmandeep Kaur
0 siblings, 0 replies; 6+ messages in thread
From: Harmandeep Kaur @ 2015-10-01 16:23 UTC (permalink / raw)
To: Peter Maydell; +Cc: Stefan Hajnoczi, Riku Voipio, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 6132 bytes --]
I meant last query just for following part of the code.
> @@ -2975,15 +2963,11 @@ static inline abi_long do_msgsnd(int msqid,
abi_long
> msgp,
>
> if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
> return -TARGET_EFAULT;
> - host_mb = malloc(msgsz+sizeof(long));
> - if (!host_mb) {
> - unlock_user_struct(target_mb, msgp, 0);
> - return -TARGET_ENOMEM;
> - }
> + host_mb = g_malloc(msgsz+sizeof(long));
On Thu, Oct 1, 2015 at 9:42 PM, Harmandeep Kaur <write.harmandeep@gmail.com>
wrote:
> Hi Peter,
>
> Can you please tell me if I should use other glib
> functions, or implement it in any other way?
> I am newbie here :)
>
> Thank you for reviewing my patch.
>
> On Thu, Oct 1, 2015 at 9:33 PM, Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On 1 October 2015 at 06:32, Harmandeep Kaur <write.harmandeep@gmail.com>
>> wrote:
>> > Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
>> > in linux-user/syscall.c file
>> >
>> > v1->v2 convert the free() call in host_to_target_semarray()
>> > to g_free() and calls g_try_malloc(count) instead of
>> > g_try_malloc(sizeof(count))
>> >
>> > Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
>> > ---
>> > linux-user/syscall.c | 38 +++++++++++---------------------------
>> > 1 file changed, 11 insertions(+), 27 deletions(-)
>> >
>> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> > index d1d3eb2..c79e862 100644
>> > --- a/linux-user/syscall.c
>> > +++ b/linux-user/syscall.c
>> > @@ -1554,12 +1554,7 @@ set_timeout:
>> > }
>> >
>> > fprog.len = tswap16(tfprog->len);
>> > - filter = malloc(fprog.len * sizeof(*filter));
>> > - if (filter == NULL) {
>> > - unlock_user_struct(tfilter, tfprog->filter, 1);
>> > - unlock_user_struct(tfprog, optval_addr, 1);
>> > - return -TARGET_ENOMEM;
>> > - }
>> >
>> > + filter = g_malloc(fprog.len * sizeof(*filter));
>>
>> fprog.len comes from the guest -- you can't use g_malloc.
>>
>> > for (i = 0; i < fprog.len; i++) {
>> > filter[i].code = tswap16(tfilter[i].code);
>> > filter[i].jt = tfilter[i].jt;
>> > @@ -1570,7 +1565,7 @@ set_timeout:
>> >
>> > ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
>> > SO_ATTACH_FILTER, &fprog,
>> sizeof(fprog)));
>> > - free(filter);
>> > + g_free(filter);
>> >
>> > unlock_user_struct(tfilter, tfprog->filter, 1);
>> > unlock_user_struct(tfprog, optval_addr, 1);
>> > @@ -1881,11 +1876,7 @@ static struct iovec *lock_iovec(int type,
>> abi_ulong
>> > target_addr,
>> > return NULL;
>> > }
>> >
>> > - vec = calloc(count, sizeof(struct iovec));
>> > - if (vec == NULL) {
>> > - errno = ENOMEM;
>> > - return NULL;
>> > - }
>> > + vec = g_new0(struct iovec, count);
>>
>> count comes from the guest -- you can't use g_new0.
>>
>> >
>> > target_vec = lock_user(VERIFY_READ, target_addr,
>> > count * sizeof(struct target_iovec), 1);
>> > @@ -1945,7 +1936,7 @@ static struct iovec *lock_iovec(int type,
>> abi_ulong
>> > target_addr,
>> > }
>> > unlock_user(target_vec, target_addr, 0);
>> > fail2:
>> > - free(vec);
>> > + g_free(vec);
>> > errno = err;
>> > return NULL;
>> > }
>> > @@ -2672,14 +2663,11 @@ static inline abi_long
>> target_to_host_semarray(int
>> > semid, unsigned short **host_
>> >
>> > nsems = semid_ds.sem_nsems;
>> >
>> > - *host_array = malloc(nsems*sizeof(unsigned short));
>> > - if (!*host_array) {
>> > - return -TARGET_ENOMEM;
>> > - }
>> > + *host_array = g_malloc(nsems*sizeof(unsigned short));
>> > array = lock_user(VERIFY_READ, target_addr,
>> > nsems*sizeof(unsigned short), 1);
>>
>> I don't think we can guarantee that nsems is small -- you can't
>> use g_malloc.
>>
>> > if (!array) {
>> > - free(*host_array);
>> > + g_free(*host_array);
>> > return -TARGET_EFAULT;
>> > }
>> >
>> > @@ -2716,7 +2704,7 @@ static inline abi_long host_to_target_semarray(int
>> > semid, abi_ulong target_addr,
>> > for(i=0; i<nsems; i++) {
>> > __put_user((*host_array)[i], &array[i]);
>> > }
>> > - free(*host_array);
>> > + g_free(*host_array);
>> > unlock_user(array, target_addr, 1);
>> >
>> > return 0;
>> > @@ -2975,15 +2963,11 @@ static inline abi_long do_msgsnd(int msqid,
>> abi_long
>> > msgp,
>> >
>> > if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
>> > return -TARGET_EFAULT;
>> > - host_mb = malloc(msgsz+sizeof(long));
>> > - if (!host_mb) {
>> > - unlock_user_struct(target_mb, msgp, 0);
>> > - return -TARGET_ENOMEM;
>> > - }
>> > + host_mb = g_malloc(msgsz+sizeof(long));
>>
>> msgsz comes from the guest -- you can't use g_malloc.
>>
>> > host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
>> > memcpy(host_mb->mtext, target_mb->mtext, msgsz);
>> > ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
>> > - free(host_mb);
>> > + g_free(host_mb);
>> > unlock_user_struct(target_mb, msgp, 0);
>> >
>> > return ret;
>> > @@ -7625,7 +7609,7 @@ abi_long do_syscall(void *cpu_env, int num,
>> abi_long
>> > arg1,
>> > struct linux_dirent *dirp;
>> > abi_long count = arg3;
>> >
>> > - dirp = malloc(count);
>> > + dirp = g_try_malloc(count);
>> > if (!dirp) {
>> > ret = -TARGET_ENOMEM;
>> > goto fail;
>> > @@ -7662,7 +7646,7 @@ abi_long do_syscall(void *cpu_env, int num,
>> abi_long
>> > arg1,
>> > ret = count1;
>> > unlock_user(target_dirp, arg2, ret);
>> > }
>> > - free(dirp);
>> > + g_free(dirp);
>> > }
>> > #else
>> > {
>> > --
>> > 1.9.1
>>
>> thanks
>> -- PMM
>>
>
>
[-- Attachment #2: Type: text/html, Size: 8363 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-01 16:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-01 5:32 [Qemu-devel] [PATCH v2] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0() Harmandeep Kaur
2015-10-01 15:57 ` Eric Blake
2015-10-01 16:00 ` Harmandeep Kaur
2015-10-01 16:03 ` Peter Maydell
2015-10-01 16:12 ` Harmandeep Kaur
2015-10-01 16:23 ` Harmandeep Kaur
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).