From feb04e66802ef96aa77951c43d4b632b376041ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Sat, 22 May 2021 17:19:14 +0200 Subject: [PATCH] Forbid basename(3) and dirname(3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are at least two interpretations of basename(3), in addition to both functions being allowed to /both/ return a static buffer (unsuitable in multi-threaded environments) /and/ raze the input (which encourages overallocations, at best) Reviewed-by: John Kennedy Reviewed-by: Brian Behlendorf Reviewed-by: Ryan Moeller Signed-off-by: Ahelenia Ziemiańska Closes #12105 --- cmd/zed/agents/zfs_retire.c | 3 +- config/Rules.am | 3 ++ lib/libzfs/libzfs_pool.c | 2 +- lib/libzpool/kernel.c | 18 +++------ lib/libzutil/zutil_import.c | 37 ++++++++++++++----- .../tests/functional/ctime/Makefile.am | 2 + .../zfs-tests/tests/functional/ctime/ctime.c | 13 +++---- 7 files changed, 45 insertions(+), 33 deletions(-) diff --git a/cmd/zed/agents/zfs_retire.c b/cmd/zed/agents/zfs_retire.c index 1c4cc885b5..1563f5d279 100644 --- a/cmd/zed/agents/zfs_retire.c +++ b/cmd/zed/agents/zfs_retire.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -240,7 +241,7 @@ replace_with_spare(fmd_hdl_t *hdl, zpool_handle_t *zhp, nvlist_t *vdev) ZPOOL_CONFIG_CHILDREN, &spares[s], 1); fmd_hdl_debug(hdl, "zpool_vdev_replace '%s' with spare '%s'", - dev_name, basename(spare_name)); + dev_name, zfs_basename(spare_name)); if (zpool_vdev_attach(zhp, dev_name, spare_name, replacement, B_TRUE, rebuild) == 0) { diff --git a/config/Rules.am b/config/Rules.am index ef10d49389..8fe2fa9ca8 100644 --- a/config/Rules.am +++ b/config/Rules.am @@ -54,6 +54,9 @@ if BUILD_FREEBSD AM_CPPFLAGS += -DTEXT_DOMAIN=\"zfs-freebsd-user\" endif AM_CPPFLAGS += -D"strtok(...)=strtok(__VA_ARGS__) __attribute__((deprecated(\"Use strtok_r(3) instead!\")))" +AM_CPPFLAGS += -D"__xpg_basename(...)=__xpg_basename(__VA_ARGS__) __attribute__((deprecated(\"basename(3) is underspecified. Use zfs_basename() instead!\")))" +AM_CPPFLAGS += -D"basename(...)=basename(__VA_ARGS__) __attribute__((deprecated(\"basename(3) is underspecified. Use zfs_basename() instead!\")))" +AM_CPPFLAGS += -D"dirname(...)=dirname(__VA_ARGS__) __attribute__((deprecated(\"dirname(3) is underspecified. Use zfs_dirnamelen() instead!\")))" AM_LDFLAGS = $(DEBUG_LDFLAGS) AM_LDFLAGS += $(ASAN_LDFLAGS) diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index adc36c47f2..c0bf9d067d 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/lib/libzfs/libzfs_pool.c @@ -4309,7 +4309,7 @@ zfs_save_arguments(int argc, char **argv, char *string, int len) { int i; - (void) strlcpy(string, basename(argv[0]), len); + (void) strlcpy(string, zfs_basename(argv[0]), len); for (i = 1; i < argc; i++) { (void) strlcat(string, " ", len); (void) strlcat(string, argv[i], len); diff --git a/lib/libzpool/kernel.c b/lib/libzpool/kernel.c index cc8e534e7e..836eb176e1 100644 --- a/lib/libzpool/kernel.c +++ b/lib/libzpool/kernel.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -541,19 +542,10 @@ void __dprintf(boolean_t dprint, const char *file, const char *func, int line, const char *fmt, ...) { - const char *newfile; + /* Get rid of annoying "../common/" prefix to filename. */ + const char *newfile = zfs_basename(file); + va_list adx; - - /* - * Get rid of annoying "../common/" prefix to filename. - */ - newfile = strrchr(file, '/'); - if (newfile != NULL) { - newfile = newfile + 1; /* Get rid of leading / */ - } else { - newfile = file; - } - if (dprint) { /* dprintf messages are printed immediately */ @@ -1040,7 +1032,7 @@ zfs_file_open(const char *path, int flags, int mode, zfs_file_t **fpp) if (vn_dumpdir != NULL) { char *dumppath = umem_zalloc(MAXPATHLEN, UMEM_NOFAIL); - char *inpath = basename((char *)(uintptr_t)path); + const char *inpath = zfs_basename(path); (void) snprintf(dumppath, MAXPATHLEN, "%s/%s", vn_dumpdir, inpath); diff --git a/lib/libzutil/zutil_import.c b/lib/libzutil/zutil_import.c index 871a75ab23..9bd12973fc 100644 --- a/lib/libzutil/zutil_import.c +++ b/lib/libzutil/zutil_import.c @@ -154,6 +154,17 @@ zutil_strdup(libpc_handle_t *hdl, const char *str) return (ret); } +static char * +zutil_strndup(libpc_handle_t *hdl, const char *str, size_t n) +{ + char *ret; + + if ((ret = strndup(str, n)) == NULL) + (void) zutil_no_memory(hdl); + + return (ret); +} + /* * Intermediate structures used to gather configuration information. */ @@ -1272,20 +1283,22 @@ zpool_find_import_scan_path(libpc_handle_t *hdl, pthread_mutex_t *lock, { int error = 0; char path[MAXPATHLEN]; - char *d, *b; - char *dpath, *name; + char *d = NULL; + ssize_t dl; + const char *dpath, *name; /* - * Separate the directory part and last part of the - * path. We do this so that we can get the realpath of + * Separate the directory and the basename. + * We do this so that we can get the realpath of * the directory. We don't get the realpath on the * whole path because if it's a symlink, we want the * path of the symlink not where it points to. */ - d = zutil_strdup(hdl, dir); - b = zutil_strdup(hdl, dir); - dpath = dirname(d); - name = basename(b); + name = zfs_basename(dir); + if ((dl = zfs_dirnamelen(dir)) == -1) + dpath = "."; + else + dpath = d = zutil_strndup(hdl, dir, dl); if (realpath(dpath, path) == NULL) { error = errno; @@ -1303,7 +1316,6 @@ zpool_find_import_scan_path(libpc_handle_t *hdl, pthread_mutex_t *lock, zpool_find_import_scan_add_slice(hdl, lock, cache, path, name, order); out: - free(b); free(d); return (error); } @@ -1506,6 +1518,7 @@ discover_cached_paths(libpc_handle_t *hdl, nvlist_t *nv, avl_tree_t *cache, pthread_mutex_t *lock) { char *path = NULL; + ssize_t dl; uint_t children; nvlist_t **child; @@ -1521,8 +1534,12 @@ discover_cached_paths(libpc_handle_t *hdl, nvlist_t *nv, * our directory cache. */ if (nvlist_lookup_string(nv, ZPOOL_CONFIG_PATH, &path) == 0) { + if ((dl = zfs_dirnamelen(path)) == -1) + path = "."; + else + path[dl] = '\0'; return (zpool_find_import_scan_dir(hdl, lock, cache, - dirname(path), 0)); + path, 0)); } return (0); } diff --git a/tests/zfs-tests/tests/functional/ctime/Makefile.am b/tests/zfs-tests/tests/functional/ctime/Makefile.am index e7479ae810..3174f78c62 100644 --- a/tests/zfs-tests/tests/functional/ctime/Makefile.am +++ b/tests/zfs-tests/tests/functional/ctime/Makefile.am @@ -11,3 +11,5 @@ pkgexecdir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/ctime pkgexec_PROGRAMS = ctime ctime_SOURCES = ctime.c + +ctime_LDADD = $(abs_top_builddir)/lib/libzfs_core/libzfs_core.la diff --git a/tests/zfs-tests/tests/functional/ctime/ctime.c b/tests/zfs-tests/tests/functional/ctime/ctime.c index d01fa0d4ed..2d515d957a 100644 --- a/tests/zfs-tests/tests/functional/ctime/ctime.c +++ b/tests/zfs-tests/tests/functional/ctime/ctime.c @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -149,22 +150,18 @@ static int do_link(const char *pfile) { int ret = 0; - char link_file[BUFSIZ] = { 0 }; - char pfile_copy[BUFSIZ] = { 0 }; - char *dname; + char link_file[BUFSIZ + 16] = { 0 }; if (pfile == NULL) { return (-1); } - strncpy(pfile_copy, pfile, sizeof (pfile_copy)-1); - pfile_copy[sizeof (pfile_copy) - 1] = '\0'; /* * Figure out source file directory name, and create * the link file in the same directory. */ - dname = dirname((char *)pfile_copy); - (void) snprintf(link_file, BUFSIZ, "%s/%s", dname, "link_file"); + (void) snprintf(link_file, sizeof (link_file), + "%.*s/%s", (int)zfs_dirnamelen(pfile), pfile, "link_file"); if (link(pfile, link_file) == -1) { (void) fprintf(stderr, "link(%s, %s) failed with errno %d\n", @@ -321,7 +318,7 @@ main(int argc, char *argv[]) (void) snprintf(tfile, sizeof (tfile), "%s/%s", penv[0], penv[1]); /* - * If the test file is exists, remove it first. + * If the test file exists, remove it first. */ if (access(tfile, F_OK) == 0) { (void) unlink(tfile);