Thank you for the review. I will definitely make
the necessary changes.
Have a great day.
On Thu, Oct 1, 2015 at 12:49 AM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 4:32 PM, Harmandeep Kaur
> <write.harmand...@gmail.com> wrote:
> > @@ -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;
> > }
>
> This free covers the error case, but in the success case the pointer
> is available to the caller after this function returns.
>
> You also need to convert the free() call in host_to_target_semarray()
> to g_free().
>
> Looking at host_to_target_semarray(), it seems to me there is a
> separate issue in that function. host_array is leaked if an error
> occurs during host_to_target_semarray(). This would be a good bug to
> fix in a separate patch.
>
> > @@ -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(sizeof(count));
> > if (!dirp) {
>
> It should be g_try_malloc(count) because we're trying to allocate
> count bytes, not sizeof(abi_long) bytes.
>
> Also, please check the other malloc/calloc calls you converted to see
> if byte count is a value passed in from the program being emulated.
> If yes, then we don't control the count and it could be huge. In that
> case g_try_malloc() needs to be used and the NULL return value must be
> handled.
>
> Stefan
>
