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

[Gedare Bloom]

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)?


> --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);
>>        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