libmultipath: fix memory leaks from scandir() use
authorMartin Wilck <mwilck@suse.com>
Fri, 2 Nov 2018 12:21:06 +0000 (13:21 +0100)
committerChristophe Varoqui <christophe.varoqui@opensvc.com>
Wed, 14 Nov 2018 07:21:14 +0000 (08:21 +0100)
scandir() users must not only free the resulting dirent* array,
but also every member. Add a cleanup function, and fix the
existing users of scandir() in libmultipath.

Add a small helper macro for casting function pointers to the
type pthread_cleanup_push() expects.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
libmultipath/config.c
libmultipath/foreign.c
libmultipath/foreign/nvme.c
libmultipath/sysfs.c
libmultipath/util.c
libmultipath/util.h

index 0aef186..5af7af5 100644 (file)
@@ -639,17 +639,13 @@ free_config (struct config * conf)
        FREE(conf);
 }
 
-static void free_namelist(void *nl)
-{
-       free(nl);
-}
-
 /* if multipath fails to process the config directory, it should continue,
  * with just a warning message */
 static void
 process_config_dir(struct config *conf, vector keywords, char *dir)
 {
        struct dirent **namelist;
+       struct scandir_result sr;
        int i, n;
        char path[LINE_MAX];
        int old_hwtable_size;
@@ -669,7 +665,9 @@ process_config_dir(struct config *conf, vector keywords, char *dir)
                return;
        } else if (n == 0)
                return;
-       pthread_cleanup_push(free_namelist, namelist);
+       sr.di = namelist;
+       sr.n = n;
+       pthread_cleanup_push_cast(free_scandir_result, &sr);
        for (i = 0; i < n; i++) {
                if (!strstr(namelist[i]->d_name, ".conf"))
                        continue;
index 80b399b..48e8d24 100644 (file)
@@ -115,6 +115,7 @@ static int _init_foreign(const char *multipath_dir)
 {
        char pathbuf[PATH_MAX];
        struct dirent **di;
+       struct scandir_result sr;
        int r, i;
 
        foreigns = vector_alloc();
@@ -135,7 +136,9 @@ static int _init_foreign(const char *multipath_dir)
                return -r;
        }
 
-       pthread_cleanup_push(free, di);
+       sr.di = di;
+       sr.n = r;
+       pthread_cleanup_push_cast(free_scandir_result, &sr);
        for (i = 0; i < r; i++) {
                const char *msg, *fn, *c;
                struct foreign *fgn;
index 8887a75..c753a74 100644 (file)
@@ -27,6 +27,7 @@
 #include <dirent.h>
 #include <errno.h>
 #include <ctype.h>
+#include "util.h"
 #include "vector.h"
 #include "generic.h"
 #include "foreign.h"
@@ -534,6 +535,7 @@ static void _find_controllers(struct context *ctx, struct nvme_map *map)
 {
        char pathbuf[PATH_MAX], realbuf[PATH_MAX];
        struct dirent **di = NULL;
+       struct scandir_result sr;
        struct udev_device *subsys;
        struct nvme_path *path;
        int r, i, n;
@@ -568,7 +570,9 @@ static void _find_controllers(struct context *ctx, struct nvme_map *map)
                return;
        }
 
-       pthread_cleanup_push(free, di);
+       sr.di = di;
+       sr.n = r;
+       pthread_cleanup_push_cast(free_scandir_result, &sr);
        for (i = 0; i < r; i++) {
                char *fn = di[i]->d_name;
                struct udev_device *ctrl, *udev;
index b7dacaa..558c8d6 100644 (file)
@@ -303,6 +303,7 @@ static void close_fd(void *arg)
 bool sysfs_is_multipathed(const struct path *pp)
 {
        char pathbuf[PATH_MAX];
+       struct scandir_result sr;
        struct dirent **di;
        int n, r, i;
        bool found = false;
@@ -323,7 +324,9 @@ bool sysfs_is_multipathed(const struct path *pp)
                return false;
        }
 
-       pthread_cleanup_push(free, di);
+       sr.di = di;
+       sr.n = r;
+       pthread_cleanup_push_cast(free_scandir_result, &sr);
        for (i = 0; i < r && !found; i++) {
                long fd;
                int nr;
index d08112d..66c4761 100644 (file)
@@ -496,3 +496,12 @@ void set_max_fds(int max_fds)
                }
        }
 }
+
+void free_scandir_result(struct scandir_result *res)
+{
+       int i;
+
+       for (i = 0; i < res->n; i++)
+               FREE(res->di[i]);
+       FREE(res->di);
+}
index c246295..a818e29 100644 (file)
@@ -30,4 +30,13 @@ void set_max_fds(int max_fds);
 #define safe_snprintf(var, size, format, args...)      \
        snprintf(var, size, format, ##args) >= size
 
+#define pthread_cleanup_push_cast(f, arg)              \
+       pthread_cleanup_push(((void (*)(void *))&f), (arg))
+
+struct scandir_result {
+       struct dirent **di;
+       int n;
+};
+void free_scandir_result(struct scandir_result *);
+
 #endif /* _UTIL_H */