<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Mar 14, 2021, 9:27 PM Chris Johns <<a href="mailto:chrisj@rtems.org">chrisj@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
On 13/3/21 2:18 am, Ryan Long wrote:<br>
> CID 1439298: Resource leak in rtems_fdisk_initialize().<br>
> <br>
> Closes #4299<br>
> ---<br>
> cpukit/libblock/src/flashdisk.c | 42 ++++++++++++++++++++++++++++++-----------<br>
> 1 file changed, 31 insertions(+), 11 deletions(-)<br>
> <br>
> diff --git a/cpukit/libblock/src/flashdisk.c b/cpukit/libblock/src/flashdisk.c<br>
> index 91f99e0..c4bac82 100644<br>
> --- a/cpukit/libblock/src/flashdisk.c<br>
> +++ b/cpukit/libblock/src/flashdisk.c<br>
> @@ -2463,6 +2463,7 @@ rtems_fdisk_initialize (rtems_device_major_number major,<br>
> {<br>
> char name[] = RTEMS_FLASHDISK_DEVICE_BASE_NAME "a";<br>
> uint32_t device;<br>
> + uint32_t device_to_free;<br>
> uint32_t blocks = 0;<br>
> int ret;<br>
> <br>
> @@ -2485,18 +2486,27 @@ rtems_fdisk_initialize (rtems_device_major_number major,<br>
> * One copy buffer of a page size.<br>
> */<br>
> fd->copy_buffer = malloc (c->block_size);<br>
> - if (!fd->copy_buffer)<br>
> + if (!fd->copy_buffer) {<br>
> + free(fd);<br>
> return RTEMS_NO_MEMORY;<br>
> + }<br>
> <br>
> fd->blocks = calloc (blocks, sizeof (rtems_fdisk_block_ctl));<br>
> - if (!fd->blocks)<br>
> + if (!fd->blocks) {<br>
> + free(fd->copy_buffer);<br>
> + free(fd);<br>
> return RTEMS_NO_MEMORY;<br>
> + }<br>
> <br>
> fd->block_count = blocks;<br>
> <br>
> fd->devices = calloc (c->device_count, sizeof (rtems_fdisk_device_ctl));<br>
> - if (!fd->devices)<br>
> + if (!fd->devices) {<br>
> + free (fd->blocks);<br>
> + free (fd->copy_buffer);<br>
> + free (fd);<br>
> return RTEMS_NO_MEMORY;<br>
> + }<br>
> <br>
> rtems_mutex_init (&fd->lock, "Flash Disk");<br>
> <br>
> @@ -2505,9 +2515,10 @@ rtems_fdisk_initialize (rtems_device_major_number major,<br>
> if (sc != RTEMS_SUCCESSFUL)<br>
> {<br>
> rtems_mutex_destroy (&fd->lock);<br>
> - free (fd->copy_buffer);<br>
> - free (fd->blocks);<br>
> free (fd->devices);<br>
> + free (fd->blocks);<br>
> + free (fd->copy_buffer);<br>
<br>
Why the order change?</blockquote></div></div><div dir="auto"><br></div><div dir="auto">Does the change make it exactly the opposite order of creation or do you see it not being in inverse order?</div><div dir="auto"><br></div><div dir="auto">This was a hard one. It was missing a LOT of cleanup.</div><div dir="auto"></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> + free (fd);<br>
<br>
What happens to the created blkdev the fd is passed into? Does that need to be<br>
destroyed before this is released?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">I didn't recognise that as an allocation. What's the destroy call for that?</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Same for the other cases below?<br>
<br>
Chris<br>
<br>
> rtems_fdisk_error ("disk create phy failed");<br>
> return sc;<br>
> }<br>
> @@ -2524,11 +2535,14 @@ rtems_fdisk_initialize (rtems_device_major_number major,<br>
> sizeof (rtems_fdisk_segment_ctl));<br>
> if (!fd->devices[device].segments)<br>
> {<br>
> + for (device_to_free = device - 1; device_to_free >= 0; device_to_free--)<br>
> + free(fd->devices[device_to_free].segments);<br>
> unlink (name);<br>
> rtems_mutex_destroy (&fd->lock);<br>
> - free (fd->copy_buffer);<br>
> - free (fd->blocks);<br>
> free (fd->devices);<br>
> + free (fd->blocks);<br>
> + free (fd->copy_buffer);<br>
> + free (fd);<br>
> return RTEMS_NO_MEMORY;<br>
> }<br>
> <br>
> @@ -2559,11 +2573,14 @@ rtems_fdisk_initialize (rtems_device_major_number major,<br>
> ret = rtems_fdisk_recover_block_mappings (fd);<br>
> if (ret)<br>
> {<br>
> + for (device_to_free = device - 1; device_to_free >= 0; device_to_free--)<br>
> + free(fd->devices[device_to_free].segments);<br>
> unlink (name);<br>
> rtems_mutex_destroy (&fd->lock);<br>
> - free (fd->copy_buffer);<br>
> - free (fd->blocks);<br>
> free (fd->devices);<br>
> + free (fd->blocks);<br>
> + free (fd->copy_buffer);<br>
> + free (fd);<br>
> rtems_fdisk_error ("recovery of disk failed: %s (%d)",<br>
> strerror (ret), ret);<br>
> return ret;<br>
> @@ -2572,11 +2589,14 @@ rtems_fdisk_initialize (rtems_device_major_number major,<br>
> ret = rtems_fdisk_compact (fd);<br>
> if (ret)<br>
> {<br>
> + for (device_to_free = device - 1; device_to_free >= 0; device_to_free--)<br>
> + free(fd->devices[device_to_free].segments);<br>
> unlink (name);<br>
> rtems_mutex_destroy (&fd->lock);<br>
> - free (fd->copy_buffer);<br>
> - free (fd->blocks);<br>
> free (fd->devices);<br>
> + free (fd->blocks);<br>
> + free (fd->copy_buffer);<br>
> + free (fd);<br>
> rtems_fdisk_error ("compacting of disk failed: %s (%d)",<br>
> strerror (ret), ret);<br>
> return ret;<br>
> <br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" target="_blank" rel="noreferrer">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div></div></div>