On Sun, Mar 14, 2021, 9:27 PM Chris Johns <chr...@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@rtems.org > http://lists.rtems.org/mailman/listinfo/devel >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel