[systemd-devel] [PATCH] systemd-bootchart: Prevent leaking file descriptors in open-fdopen combination

[Daniel Mack]

On 03/29/2015 08:44 PM, Alexander Sverdlin wrote:
> Correctly handle the potential failure of fdopen() (because of OOM, for instance)
> after potentially successful open(). Prevent leaking open fd in such case.

Applied, thanks!

> ---
>  src/bootchart/store.c | 12 ++++++++++--
>  src/bootchart/svg.c   |  9 +++++++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/src/bootchart/store.c b/src/bootchart/store.c
> index 3900936..8e9a62f 100644
> --- a/src/bootchart/store.c
> +++ b/src/bootchart/store.c
> @@ -330,9 +330,13 @@ schedstat_next:
>                          /* ppid */
>                          sprintf(filename, "%d/stat", pid);
>                          fd = openat(procfd, filename, O_RDONLY);
> +                        if (fd == -1)
> +                                continue;
>                          st = fdopen(fd, "r");
> -                        if (!st)
> +                        if (!st) {
> +                                close(fd);
>                                  continue;
> +                        }
>                          if (!fscanf(st, "%*s %*s %*s %i", &p)) {
>                                  continue;
>                          }
> @@ -432,9 +436,13 @@ schedstat_next:
>                  if (!ps->smaps) {
>                          sprintf(filename, "%d/smaps", pid);
>                          fd = openat(procfd, filename, O_RDONLY);
> +                        if (fd == -1)
> +                                continue;
>                          ps->smaps = fdopen(fd, "r");
> -                        if (!ps->smaps)
> +                        if (!ps->smaps) {
> +                                close(fd);
>                                  continue;
> +                        }
>                          setvbuf(ps->smaps, smaps_buf, _IOFBF, sizeof(smaps_buf));
>                  }
>                  else {
> diff --git a/src/bootchart/svg.c b/src/bootchart/svg.c
> index e1fc531..5412915 100644
> --- a/src/bootchart/svg.c
> +++ b/src/bootchart/svg.c
> @@ -170,6 +170,9 @@ static void svg_title(const char *build) {
>                  if (!fgets(cmdline, 255, f))
>                          sprintf(cmdline, "Unknown");
>                  fclose(f);
> +        } else {
> +                if (fd >= 0)
> +                        close(fd);
>          }
> 
>          /* extract root fs so we can find disk model name in sysfs */
> @@ -185,6 +188,9 @@ static void svg_title(const char *build) {
>                          if (!fgets(model, 255, f))
>                                  fprintf(stderr, "Error reading disk model for %s\n", rootbdev);
>                          fclose(f);
> +                } else {
> +                        if (fd >= 0)
> +                                close(fd);
>                  }
>          }
> 
> @@ -208,6 +214,9 @@ static void svg_title(const char *build) {
>                          }
>                  }
>                  fclose(f);
> +        } else {
> +                if (fd >= 0)
> +                        close(fd);
>          }
> 
>          svg("<text class=\"t1\" x=\"0\" y=\"30\">Bootchart for %s - %s</text>\n",
>