<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 15, 2021, 6:10 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">On 15/3/21 2:21 pm, Joel Sherrill wrote:<br>
> On Sun, Mar 14, 2021, 9:27 PM Chris Johns <<a href="mailto:chrisj@rtems.org" target="_blank" rel="noreferrer">chrisj@rtems.org</a><br>
> <mailto:<a href="mailto:chrisj@rtems.org" target="_blank" rel="noreferrer">chrisj@rtems.org</a>>> wrote:<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>
> ++++++++++++++++++++++++++++++-----------<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<br>
> 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<br>
> 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?<br>
> <br>
> Does the change make it exactly the opposite order of creation or do you see it<br>
> not being in inverse order?<br>
<br>
If there is no reason to change the order the blocks are freed then I suggest<br>
not changing the order. It avoids adding noise to the change.<br>
<br>
> This was a hard one. It was missing a LOT of cleanup.<br>
> <br>
> <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>
<br>
I do not know. I have not looked.<br>
<br>
> I didn't recognise that as an allocation. What's the destroy call for that?<br>
<br>
If the block dev holds the pointer and you have freed it bad things will happen.<br>
<br>
I have a funny feeling there was no block dev destroy when the code was written<br>
and why there are no free's for this allocation.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Is this one we should leave a version of the patch on the ticket, link to this threadz and walk away from? Or add block dev destroy? What concerns me about that is that it is another layer in the onion we (Ryan and I) don't fully understand the interactions.</div><div dir="auto"><br></div><div dir="auto">Is blocked destroy going to be just resources or will it have to interrupt outstanding work, etc?</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>
Chris<br>
</blockquote></div></div></div>