[PATCH 4/5] flashdisk.c: Fix Resource leak (CID #1439298)

[Joel Sherrill]

On Sun, Mar 14, 2021, 9:27 PM Chris Johns <chrisj at rtems.org> wrote:

>
>
> On 13/3/21 2:18 am, Ryan Long wrote:
> > CID 1439298: Resource leak in rtems_fdisk_initialize().
> >
> > Closes #4299
> > ---
> >  cpukit/libblock/src/flashdisk.c | 42
> ++++++++++++++++++++++++++++++-----------
> >  1 file changed, 31 insertions(+), 11 deletions(-)
> >
> > diff --git a/cpukit/libblock/src/flashdisk.c
> b/cpukit/libblock/src/flashdisk.c
> > index 91f99e0..c4bac82 100644
> > --- a/cpukit/libblock/src/flashdisk.c
> > +++ b/cpukit/libblock/src/flashdisk.c
> > @@ -2463,6 +2463,7 @@ rtems_fdisk_initialize (rtems_device_major_number
> major,
> >    {
> >      char     name[] = RTEMS_FLASHDISK_DEVICE_BASE_NAME "a";
> >      uint32_t device;
> > +    uint32_t device_to_free;
> >      uint32_t blocks = 0;
> >      int      ret;
> >
> > @@ -2485,18 +2486,27 @@ rtems_fdisk_initialize
> (rtems_device_major_number major,
> >       * One copy buffer of a page size.
> >       */
> >      fd->copy_buffer = malloc (c->block_size);
> > -    if (!fd->copy_buffer)
> > +    if (!fd->copy_buffer) {
> > +      free(fd);
> >        return RTEMS_NO_MEMORY;
> > +    }
> >
> >      fd->blocks = calloc (blocks, sizeof (rtems_fdisk_block_ctl));
> > -    if (!fd->blocks)
> > +    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)
> > +    if (!fd->devices) {
> > +      free (fd->blocks);
> > +      free (fd->copy_buffer);
> > +      free (fd);
> >        return RTEMS_NO_MEMORY;
> > +    }
> >
> >      rtems_mutex_init (&fd->lock, "Flash Disk");
> >
> > @@ -2505,9 +2515,10 @@ rtems_fdisk_initialize (rtems_device_major_number
> major,
> >      if (sc != RTEMS_SUCCESSFUL)
> >      {
> >        rtems_mutex_destroy (&fd->lock);
> > -      free (fd->copy_buffer);
> > -      free (fd->blocks);
> >        free (fd->devices);
> > +      free (fd->blocks);
> > +      free (fd->copy_buffer);
>
> Why the order change?


Does the change make it exactly the opposite order of creation or do you
see it not being in inverse order?

This was a hard one. It was missing a LOT of cleanup.

>
> > +      free (fd);
>
> What happens to the created blkdev the fd is passed into? Does that need
> to be
> destroyed before this is released?
>

I didn't recognise that as an allocation. What's the destroy call for that?

>
> Same for the other cases below?
>
> Chris
>
> >        rtems_fdisk_error ("disk create phy failed");
> >        return sc;
> >      }
> > @@ -2524,11 +2535,14 @@ rtems_fdisk_initialize
> (rtems_device_major_number major,
> >                                               sizeof
> (rtems_fdisk_segment_ctl));
> >        if (!fd->devices[device].segments)
> >        {
> > +        for (device_to_free = device - 1; device_to_free >= 0;
> device_to_free--)
> > +       free(fd->devices[device_to_free].segments);
> >          unlink (name);
> >          rtems_mutex_destroy (&fd->lock);
> > -        free (fd->copy_buffer);
> > -        free (fd->blocks);
> >          free (fd->devices);
> > +        free (fd->blocks);
> > +        free (fd->copy_buffer);
> > +        free (fd);
> >          return RTEMS_NO_MEMORY;
> >        }
> >
> > @@ -2559,11 +2573,14 @@ rtems_fdisk_initialize
> (rtems_device_major_number major,
> >      ret = rtems_fdisk_recover_block_mappings (fd);
> >      if (ret)
> >      {
> > +      for (device_to_free = device - 1; device_to_free >= 0;
> device_to_free--)
> > +        free(fd->devices[device_to_free].segments);
> >        unlink (name);
> >        rtems_mutex_destroy (&fd->lock);
> > -      free (fd->copy_buffer);
> > -      free (fd->blocks);
> >        free (fd->devices);
> > +      free (fd->blocks);
> > +      free (fd->copy_buffer);
> > +      free (fd);
> >        rtems_fdisk_error ("recovery of disk failed: %s (%d)",
> >                           strerror (ret), ret);
> >        return ret;
> > @@ -2572,11 +2589,14 @@ rtems_fdisk_initialize
> (rtems_device_major_number major,
> >      ret = rtems_fdisk_compact (fd);
> >      if (ret)
> >      {
> > +      for (device_to_free = device - 1; device_to_free >= 0;
> device_to_free--)
> > +        free(fd->devices[device_to_free].segments);
> >        unlink (name);
> >        rtems_mutex_destroy (&fd->lock);
> > -      free (fd->copy_buffer);
> > -      free (fd->blocks);
> >        free (fd->devices);
> > +      free (fd->blocks);
> > +      free (fd->copy_buffer);
> > +      free (fd);
> >        rtems_fdisk_error ("compacting of disk failed: %s (%d)",
> >                           strerror (ret), ret);
> >        return ret;
> >
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210314/af97146e/attachment-0001.html>