Syslinux - Sep 2015 - [PATCH v1 1/1] extlinux: fix file descriptors leak

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);
         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;
 }
 
-- 
1.9.1

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

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.