From 7bf9eadc5b7fb42640bdd851807c96d35aca07f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Thu, 29 Apr 2021 22:21:41 +0200 Subject: [PATCH 1/7] libzfs: zpool_load_compat(): open feature file cloexec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As a bonus, this also passes the open flags into the open flags instead of the mode (it worked by accident because O_RDONLY is 0), correctly detects a failed map, and prefaults the entire file since we're always writing to every page Signed-off-by: Ahelenia Ziemiańska --- lib/libzfs/libzfs_pool.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index b4c9d7df90b6..d4849ee5bd9b 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/lib/libzfs/libzfs_pool.c @@ -4812,14 +4812,14 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report, file != NULL; file = strtok_r(NULL, ",", &ps)) { - boolean_t l_features[SPA_FEATURES]; + boolean_t l_features[SPA_FEATURES] = {B_FALSE}; enum { Z_SYSCONF, Z_DATA } source; /* try sysconfdir first, then datadir */ source = Z_SYSCONF; - if ((featfd = openat(sdirfd, file, 0, O_RDONLY)) < 0) { - featfd = openat(ddirfd, file, 0, O_RDONLY); + if ((featfd = openat(sdirfd, file, O_RDONLY | O_CLOEXEC)) < 0) { + featfd = openat(ddirfd, file, O_RDONLY | O_CLOEXEC); source = Z_DATA; } @@ -4835,13 +4835,18 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report, continue; } +#if !defined(MAP_POPULATE) && defined(MAP_PREFAULT_READ) +#define MAP_POPULATE MAP_PREFAULT_READ +#elif !defined(MAP_POPULATE) +#define MAP_POPULATE 0 +#endif /* private mmap() so we can strtok safely */ fc = (char *)mmap(NULL, fs.st_size, - PROT_READ|PROT_WRITE, MAP_PRIVATE, featfd, 0); + PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_POPULATE, featfd, 0); (void) close(featfd); /* map ok, and last character == newline? */ - if (fc < 0 || fc[fs.st_size - 1] != '\n') { + if (fc == MAP_FAILED || fc[fs.st_size - 1] != '\n') { (void) munmap((void *) fc, fs.st_size); strlcat(err_badfile, file, ZFS_MAXPROPLEN); strlcat(err_badfile, " ", ZFS_MAXPROPLEN); @@ -4851,9 +4856,6 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report, ret_nofiles = B_FALSE; - for (uint_t i = 0; i < SPA_FEATURES; i++) - l_features[i] = B_FALSE; - /* replace last char with NUL to ensure we have a delimiter */ fc[fs.st_size - 1] = '\0'; From e1bc5681a881408a3c68efa7972632fc3d356a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Fri, 30 Apr 2021 20:49:39 +0200 Subject: [PATCH 2/7] libzfs: zpool_load_compat(): don't free undefined pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ahelenia Ziemiańska --- lib/libzutil/os/linux/zutil_device_path_os.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/libzutil/os/linux/zutil_device_path_os.c b/lib/libzutil/os/linux/zutil_device_path_os.c index da7ffba764a8..dae5b897160b 100644 --- a/lib/libzutil/os/linux/zutil_device_path_os.c +++ b/lib/libzutil/os/linux/zutil_device_path_os.c @@ -306,9 +306,10 @@ dm_get_underlying_path(const char *dm_name) else dev_str = tmp; - size = asprintf(&tmp, "/sys/block/%s/slaves/", dev_str); - if (size == -1 || !tmp) + if ((size = asprintf(&tmp, "/sys/block/%s/slaves/", dev_str)) == -1) { + tmp = NULL; goto end; + } dp = opendir(tmp); if (dp == NULL) @@ -334,7 +335,9 @@ dm_get_underlying_path(const char *dm_name) if (!enclosure_path) continue; - size = asprintf(&path, "/dev/%s", ep->d_name); + if ((size = asprintf( + &path, "/dev/%s", ep->d_name)) == -1) + path = NULL; free(enclosure_path); break; } @@ -352,7 +355,8 @@ dm_get_underlying_path(const char *dm_name) * enclosure devices. Throw up out hands and return the first * underlying path. */ - size = asprintf(&path, "/dev/%s", first_path); + if ((size = asprintf(&path, "/dev/%s", first_path)) == -1) + path = NULL; } free(first_path); From 50f83ec2c4bde56ef110d69d4f7479ac4d769c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Mon, 3 May 2021 11:36:02 +0200 Subject: [PATCH 3/7] zpool: vdev_run_cmd(): don't free undefined pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ahelenia Ziemiańska --- cmd/zpool/zpool_iter.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/cmd/zpool/zpool_iter.c b/cmd/zpool/zpool_iter.c index d70d266699cf..3d7a0cfc35e6 100644 --- a/cmd/zpool/zpool_iter.c +++ b/cmd/zpool/zpool_iter.c @@ -494,19 +494,25 @@ vdev_run_cmd(vdev_cmd_data_t *data, char *cmd) /* Setup our custom environment variables */ rc = asprintf(&env[1], "VDEV_PATH=%s", data->path ? data->path : ""); - if (rc == -1) + if (rc == -1) { + env[1] = NULL; goto out; + } rc = asprintf(&env[2], "VDEV_UPATH=%s", data->upath ? data->upath : ""); - if (rc == -1) + if (rc == -1) { + env[2] = NULL; goto out; + } rc = asprintf(&env[3], "VDEV_ENC_SYSFS_PATH=%s", data->vdev_enc_sysfs_path ? data->vdev_enc_sysfs_path : ""); - if (rc == -1) + if (rc == -1) { + env[3] = NULL; goto out; + } /* Run the command */ rc = libzfs_run_process_get_stdout_nopath(cmd, argv, env, &lines, @@ -525,8 +531,7 @@ vdev_run_cmd(vdev_cmd_data_t *data, char *cmd) /* Start with i = 1 since env[0] was statically allocated */ for (i = 1; i < ARRAY_SIZE(env); i++) - if (env[i] != NULL) - free(env[i]); + free(env[i]); } /* From d4e5303a184ebb3fa7fc50760b6f8a85559bbb62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Mon, 3 May 2021 12:01:13 +0200 Subject: [PATCH 4/7] zutil_device_path/linux: zfs_get_enclosure_sysfs_path(): don't leak dev path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also always free tmp2 at the end Before: nabijaczleweli@tarta:~/uwu$ valgrind --leak-check=full ./blergh ==8947== Memcheck, a memory error detector ==8947== Using Valgrind-3.14.0 and LibVEX ==8947== Command: ./blergh ==8947== (null) ==8947== ==8947== HEAP SUMMARY: ==8947== in use at exit: 23 bytes in 1 blocks ==8947== total heap usage: 3 allocs, 2 frees, 1,147 bytes allocated ==8947== ==8947== 23 bytes in 1 blocks are definitely lost in loss record 1 of 1 ==8947== at 0x483577F: malloc (vg_replace_malloc.c:299) ==8947== by 0x48D74B7: vasprintf (vasprintf.c:73) ==8947== by 0x48B7833: asprintf (asprintf.c:35) ==8947== by 0x401258: zfs_get_enclosure_sysfs_path (zutil_device_path_os.c:191) ==8947== by 0x401482: main (blergh.c:107) ==8947== ==8947== LEAK SUMMARY: ==8947== definitely lost: 23 bytes in 1 blocks ==8947== indirectly lost: 0 bytes in 0 blocks ==8947== possibly lost: 0 bytes in 0 blocks ==8947== still reachable: 0 bytes in 0 blocks ==8947== suppressed: 0 bytes in 0 blocks ==8947== ==8947== For counts of detected and suppressed errors, rerun with: -v ==8947== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) nabijaczleweli@tarta:~/uwu$ sed -n 191p zutil_device_path_os.c tmpsize = asprintf(&tmp1, "/sys/block/%s/device", dev_name); After: nabijaczleweli@tarta:~/uwu$ valgrind --leak-check=full ./blergh ==9512== Memcheck, a memory error detector ==9512== Using Valgrind-3.14.0 and LibVEX ==9512== Command: ./blergh ==9512== (null) ==9512== ==9512== HEAP SUMMARY: ==9512== in use at exit: 0 bytes in 0 blocks ==9512== total heap usage: 3 allocs, 3 frees, 1,147 bytes allocated ==9512== ==9512== All heap blocks were freed -- no leaks are possible ==9512== ==9512== For counts of detected and suppressed errors, rerun with: -v ==9512== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) Signed-off-by: Ahelenia Ziemiańska --- lib/libzutil/os/linux/zutil_device_path_os.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/libzutil/os/linux/zutil_device_path_os.c b/lib/libzutil/os/linux/zutil_device_path_os.c index dae5b897160b..71134a5383ab 100644 --- a/lib/libzutil/os/linux/zutil_device_path_os.c +++ b/lib/libzutil/os/linux/zutil_device_path_os.c @@ -195,10 +195,8 @@ zfs_get_enclosure_sysfs_path(const char *dev_name) } dp = opendir(tmp1); - if (dp == NULL) { - tmp1 = NULL; /* To make free() at the end a NOP */ + if (dp == NULL) goto end; - } /* * Look though all sysfs entries in /sys/block//device for @@ -216,11 +214,8 @@ zfs_get_enclosure_sysfs_path(const char *dev_name) size = readlink(tmp2, buf, sizeof (buf)); /* Did readlink fail or crop the link name? */ - if (size == -1 || size >= sizeof (buf)) { - free(tmp2); - tmp2 = NULL; /* To make free() at the end a NOP */ + if (size == -1 || size >= sizeof (buf)) break; - } /* * We got a valid link. readlink() doesn't terminate strings From a0ca4979a6c3bfa91d73a48990d7f1d2c4b03f26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Mon, 3 May 2021 12:13:20 +0200 Subject: [PATCH 5/7] zutil_device_path/linux: zfs_get_enclosure_sysfs_path(): don't free undefined pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ahelenia Ziemiańska --- lib/libzutil/os/linux/zutil_device_path_os.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/libzutil/os/linux/zutil_device_path_os.c b/lib/libzutil/os/linux/zutil_device_path_os.c index 71134a5383ab..2a6f4ae2a222 100644 --- a/lib/libzutil/os/linux/zutil_device_path_os.c +++ b/lib/libzutil/os/linux/zutil_device_path_os.c @@ -207,9 +207,10 @@ zfs_get_enclosure_sysfs_path(const char *dev_name) if (strstr(ep->d_name, "enclosure_device") == NULL) continue; - if (asprintf(&tmp2, "%s/%s", tmp1, ep->d_name) == -1 || - tmp2 == NULL) + if (asprintf(&tmp2, "%s/%s", tmp1, ep->d_name) == -1) { + tmp2 = NULL; break; + } size = readlink(tmp2, buf, sizeof (buf)); From 431a021a8fdad69c3d3565e35e18813eabd474c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Mon, 3 May 2021 12:11:30 +0200 Subject: [PATCH 6/7] libzfsbootenv: lzbe_set_boot_device(): don't free undefined pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ahelenia Ziemiańska --- lib/libzfsbootenv/lzbe_device.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/libzfsbootenv/lzbe_device.c b/lib/libzfsbootenv/lzbe_device.c index b366bd9c7f57..2d8833b4fff2 100644 --- a/lib/libzfsbootenv/lzbe_device.c +++ b/lib/libzfsbootenv/lzbe_device.c @@ -88,12 +88,11 @@ lzbe_set_boot_device(const char *pool, lzbe_flags_t flag, const char *device) if (strncmp(device, "zfs:", 4) == 0) { fnvlist_add_string(nv, OS_BOOTONCE, device); } else { - descriptor = NULL; - if (asprintf(&descriptor, "zfs:%s:", device) > 0) + if (asprintf(&descriptor, "zfs:%s:", device) > 0) { fnvlist_add_string(nv, OS_BOOTONCE, descriptor); - else + free(descriptor); + } else rv = ENOMEM; - free(descriptor); } } From a44f40547a0b7101f3bd52a9288452cba127bfe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Mon, 3 May 2021 12:30:16 +0200 Subject: [PATCH 7/7] libzfs: zfs_asprintf(): don't return undefined pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ahelenia Ziemiańska --- lib/libzfs/libzfs_util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/libzfs/libzfs_util.c b/lib/libzfs/libzfs_util.c index 8038f7fa343e..7840e3590a57 100644 --- a/lib/libzfs/libzfs_util.c +++ b/lib/libzfs/libzfs_util.c @@ -782,8 +782,10 @@ zfs_asprintf(libzfs_handle_t *hdl, const char *fmt, ...) va_end(ap); - if (err < 0) + if (err < 0) { (void) no_memory(hdl); + ret = NULL; + } return (ret); }