[syslinux] [PATCH v1 1/1] extlinux: fix file descriptors leak

Paulo Alcantara pcacjr at zytor.com
Wed Sep 9 11:06:28 PDT 2015


On Wed,  9 Sep 2015 14:00:09 +0300
Imran Zaman via Syslinux <syslinux at zytor.com> wrote:

> file descriptors are closed when not in use
> 
> Signed-off-by: Imran Zaman <imran.zaman at intel.com>
> ---
>  extlinux/main.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/extlinux/main.c b/extlinux/main.c
> index 09740bd..55a1495 100644
> --- a/extlinux/main.c
> +++ b/extlinux/main.c
> @@ -580,6 +580,7 @@ int ext2_fat_install_file(const char *path, int
> devfd, struct stat *rst) goto bail;
>      }
>  
> +    close(fd);
>      free(file);
>      free(oldfile);
>      free(c32file);
> @@ -778,7 +779,7 @@ static char * get_default_subvol(char * rootdir,
> char * subvol) struct btrfs_ioctl_search_key *sk = &args.key;
>      struct btrfs_ioctl_search_header *sh;
>      int ret, i;
> -    int fd;
> +    int fd = -1;
>      struct btrfs_root_ref *ref;
>      struct btrfs_dir_item *dir_item;
>      unsigned long off = 0;
> @@ -797,6 +798,8 @@ static char * get_default_subvol(char * rootdir,
> char * subvol) }
>      if (ret <= 0) {
>          subvol[0] = '\0';
> +        if (fd >= 0)
> +            close(fd);

How about to define a label like this in the end of function:

bail:
    if (ret < 0) {
        if (fd >= 0)
            close(fd);
        subvol[0] = '\0';
        return NULL;
    }

    return subvol;

and use it where appropriate -- I tell this because there are at least
3 bailing conditions like this one.

Other than that, the patch looks good to me. Good catch!

Thanks,

Paulo

>          return NULL;
>      }
>  
> @@ -831,6 +834,8 @@ static char * get_default_subvol(char * rootdir,
> char * subvol) if (ret < 0) {
>             fprintf(stderr, "ERROR: can't perform the search\n");
>             subvol[0] = '\0';
> +           if (fd >= 0)
> +               close(fd);
>             return NULL;
>         }
>         /* the ioctl returns the number of item it found in nr_items
> */ @@ -891,6 +896,8 @@ static char * get_default_subvol(char *
> rootdir, char * subvol) 
>     if (defaultsubvolid == 0) {
>         subvol[0] = '\0';
> +       if (fd >= 0)
> +           close(fd);
>         return NULL;
>     }
>  
> @@ -922,6 +929,8 @@ static char * get_default_subvol(char * rootdir,
> char * subvol) if (ret < 0) {
>             fprintf(stderr, "ERROR: can't perform the search\n");
>             subvol[0] = '\0';
> +           if (fd >= 0)
> +               close(fd);
>             return NULL;
>         }
>         /* the ioctl returns the number of item it found in nr_items
> */ @@ -975,6 +984,9 @@ static char * get_default_subvol(char *
> rootdir, char * subvol) } else
>             break;
>     }
> +
> +   if (fd >= 0)
> +       close(fd);
>     return subvol;
>  }
>  


-- 
Paulo Alcantara, C.E.S.A.R
Speaking for myself only.


More information about the Syslinux mailing list