[PATCH] cpukit/libblock/src: Add better memory handling in flashdisk.c

[Chris Johns]

On 5/6/21 8:11 am, Gedare Bloom wrote:
> On Fri, Jun 4, 2021 at 1:17 PM Joel Sherrill <joel at rtems.org> wrote:
>>
>> On the surface, this looks OK to me. But I remember looking at this one
>> and wondering if there was any cleanup required because of the various
>> subroutine calls as you work down through this method.
>>
>> Did you look into each of the subroutines called and make sure they
>> didn't do further allocations?
>>
> We hadn't. Now, we took a closer look at this also, and see some more
> problems that need to be addressed. The fd variable is a loop
> iterator, so we can't just free() it. Should this function cleanup
> everything that was attempted and not to return with a partial
> initialization in case of a failure? Or, should it allow the partial
> initialization of some of the fd's, and just cleanup the last one that
> fails (due to out of memory)?

I think there is one that pops up on a regular basis where you cannot delete
things because they are registered with something that has not unregister.

I am not sure this is the case. It might pay to check before pushing.

> 
> 
>> --joel
>>
>> On Fri, Jun 4, 2021 at 1:27 PM Harrison Edward Gerber <gerberhe11 at gmail.com> wrote:
>>>
>>> See also CID 1439298
>>>
>>> Closes #3570
>>> ---
>>>  cpukit/libblock/src/flashdisk.c | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/cpukit/libblock/src/flashdisk.c b/cpukit/libblock/src/flashdisk.c
>>> index 91f99e0d52..4de6ecd807 100644
>>> --- a/cpukit/libblock/src/flashdisk.c
>>> +++ b/cpukit/libblock/src/flashdisk.c
>>> @@ -2486,17 +2486,29 @@ rtems_fdisk_initialize (rtems_device_major_number major,
>>>       */
>>>      fd->copy_buffer = malloc (c->block_size);
>>>      if (!fd->copy_buffer)
>>> +    {
>>> +      free(fd);

Missing space to be consistent? More of these below.

Chris

>>>        return RTEMS_NO_MEMORY;
>>> +    }
>>>
>>>      fd->blocks = calloc (blocks, sizeof (rtems_fdisk_block_ctl));
>>>      if (!fd->blocks)
>>> +    {
>>> +      free(fd->copy_buffer);
>>> +      free(fd);
>>>        return RTEMS_NO_MEMORY;
>>> +    }
>>>
>>>      fd->block_count = blocks;
>>>
>>>      fd->devices = calloc (c->device_count, sizeof (rtems_fdisk_device_ctl));
>>>      if (!fd->devices)
>>> +    {
>>> +      free(fd->blocks);
>>> +      free(fd->copy_buffer);
>>> +      free(fd);
>>>        return RTEMS_NO_MEMORY;
>>> +    }
>>>
>>>      rtems_mutex_init (&fd->lock, "Flash Disk");
>>>
>>> @@ -2508,6 +2520,7 @@ rtems_fdisk_initialize (rtems_device_major_number major,
>>>        free (fd->copy_buffer);
>>>        free (fd->blocks);
>>>        free (fd->devices);
>>> +      free(fd);
>>>        rtems_fdisk_error ("disk create phy failed");
>>>        return sc;
>>>      }
>>> @@ -2529,6 +2542,7 @@ rtems_fdisk_initialize (rtems_device_major_number major,
>>>          free (fd->copy_buffer);
>>>          free (fd->blocks);
>>>          free (fd->devices);
>>> +        free(fd);
>>>          return RTEMS_NO_MEMORY;
>>>        }
>>>
>>> @@ -2564,6 +2578,7 @@ rtems_fdisk_initialize (rtems_device_major_number major,
>>>        free (fd->copy_buffer);
>>>        free (fd->blocks);
>>>        free (fd->devices);
>>> +      free(fd);
>>>        rtems_fdisk_error ("recovery of disk failed: %s (%d)",
>>>                           strerror (ret), ret);
>>>        return ret;
>>> @@ -2577,6 +2592,7 @@ rtems_fdisk_initialize (rtems_device_major_number major,
>>>        free (fd->copy_buffer);
>>>        free (fd->blocks);
>>>        free (fd->devices);
>>> +      free(fd);
>>>        rtems_fdisk_error ("compacting of disk failed: %s (%d)",
>>>                           strerror (ret), ret);
>>>        return ret;
>>> --
>>> 2.25.1
>>>
>>> _______________________________________________
>>> devel mailing list
>>> devel at rtems.org
>>> http://lists.rtems.org/mailman/listinfo/devel
>>
>> _______________________________________________
>> devel mailing list
>> devel at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>