[Toybox] [PATCH] losetup: Fix memory leaks in loopback_setup()

[James McMechan]

Slight glitch for humans, the line "if (FLAG(s)) " after the free(s);
Even though that 's' is a flag name, it looks at first and maybe second
glance like a use after free. Either a comment explaining that or choosing
a variable other than 's' for the allocation might be clearer.


On Mon, Oct 21, 2019, 03:22 Alessio Balsini via Toybox <
toybox at lists.landley.net> wrote:

> The function loopback_setup() uses xabspath() to get the loopback path.
> This function allocates dynamic memory which should be freed by the
> function caller.
> But there are early return cases where the dynamic memory is not freed.
> Besides the special cases of perror_exit(), for which the "early" free
> operation is simply used to silence memory analysis tools, the
>
>   if (racy && errno == EBUSY) return 1;
>
> branch may be a real cause of memory leak.
>
> Fix by adding a new free() in the racy+EBUSY branch and anticipating the
> existing free().
>
> Signed-off-by: Alessio Balsini <balsini at android.com>
> ---
>  toys/other/losetup.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/toys/other/losetup.c b/toys/other/losetup.c
> index 917e64ea..7f91ba1f 100644
> --- a/toys/other/losetup.c
> +++ b/toys/other/losetup.c
> @@ -107,15 +107,16 @@ static int loopback_setup(char *device, char *file)
>
>      if (!s) perror_exit("file"); // already opened, but if deleted
> since...
>      if (ioctl(lfd, LOOP_SET_FD, ffd)) {
> +      free(s);
>        if (racy && errno == EBUSY) return 1;
>        perror_exit("%s=%s", device, file);
>      }
> +    xstrncpy((char *)loop->lo_file_name, s, LO_NAME_SIZE);
> +    free(s);
>      loop->lo_offset = TT.o;
>      loop->lo_sizelimit = TT.S;
> -    xstrncpy((char *)loop->lo_file_name, s, LO_NAME_SIZE);
>      if (ioctl(lfd, LOOP_SET_STATUS64, loop)) perror_exit("%s=%s", device,
> file);
>      if (FLAG(s)) puts(device);
> -    free(s);
>    }
>    else {
>      xprintf("%s: [%lld]:%llu (%s)", device, (long long)loop->lo_device,
> --
> 2.23.0.866.gb869b98d4c-goog
>
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20191021/dac09d5e/attachment-0001.htm>