[lttng-dev] [PATCH lttng-tools 1/2] Fix: lttng-crash: fd leak

[Jérémie Galarneau]

Merged with modifications.

While this does correct the leak in the common case, it will also leak
when the "error" label is jumped to or close an uninitialized file
descriptor. The edited version I have merged takes care of both
issues.

Thanks!
Jérémie

On Thu, Sep 3, 2015 at 5:52 PM, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> At the same time make sure to have on one exit-point.
>
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/bin/lttng-crash/lttng-crash.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/src/bin/lttng-crash/lttng-crash.c b/src/bin/lttng-crash/lttng-crash.c
> index 4f1efe9..8e57034 100644
> --- a/src/bin/lttng-crash/lttng-crash.c
> +++ b/src/bin/lttng-crash/lttng-crash.c
> @@ -103,7 +103,7 @@ enum rb_modes {
>
>  struct crash_abi_unknown {
>         uint8_t magic[RB_CRASH_DUMP_ABI_MAGIC_LEN];
> -       uint64_t mmap_length;   /* Overall lenght of crash record */
> +       uint64_t mmap_length;   /* Overall length of crash record */
>         uint16_t endian;        /*
>                                  * { 0x12, 0x34 }: big endian
>                                  * { 0x34, 0x12 }: little endian
> @@ -326,26 +326,30 @@ int copy_file(const char *file_dest, const char *file_src)
>         int fd_src, fd_dest;
>         ssize_t readlen, writelen;
>         char buf[COPY_BUFLEN];
> +       int ret;
>
>         DBG("Copy metadata file '%s' into '%s'", file_src, file_dest);
>
>         fd_src = open(file_src, O_RDONLY);
>         if (fd_src < 0) {
>                 PERROR("Error opening %s for reading", file_src);
> -               return fd_src;
> +               ret = fd_src;
> +               goto error;
>         }
>         fd_dest = open(file_dest, O_RDWR | O_CREAT | O_EXCL,
>                         S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
>         if (fd_dest < 0) {
>                 PERROR("Error opening %s for writing", file_dest);
> -               return fd_dest;
> +               ret = fd_dest;
> +               goto error;
>         }
>
>         for (;;) {
>                 readlen = lttng_read(fd_src, buf, COPY_BUFLEN);
>                 if (readlen < 0) {
>                         PERROR("Error reading input file");
> -                       return -1;
> +                       ret = -1;
> +                       goto error;
>                 }
>                 if (!readlen) {
>                         break;
> @@ -353,10 +357,26 @@ int copy_file(const char *file_dest, const char *file_src)
>                 writelen = lttng_write(fd_dest, buf, readlen);
>                 if (writelen < readlen) {
>                         PERROR("Error writing to output file");
> -                       return -1;
> +                       ret = -1;
> +                       goto error;
>                 }
>         }
> +
> +       ret = close(fd_src);
> +       if (ret < 0) {
> +               PERROR("Error closing %s", file_src);
> +               goto error;
> +       }
> +
> +       ret = close(fd_dest);
> +       if (ret < 0) {
> +               PERROR("Error closing %s", file_dest);
> +               goto error;
> +       }
> +
>         return 0;
> +error:
> +       return ret;
>  }
>
>  static
> --
> 2.1.4
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com