Fedora 28: Fix misc bounds check compiler warnings

Fix a bunch of (mostly) sprintf/snprintf truncation compiler
warnings that show up on Fedora 28 (GCC 8.0.1).

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes #7361
Closes #7368

diff --git a/cmd/zpios/zpios_main.c b/cmd/zpios/zpios_main.c
index 01f2ce1971c1a19150fc5ff525f09776fe44d5b7..4be26d64f67021ecda3740f6d06bbbabd2761e33 100644 (file)
--- a/cmd/zpios/zpios_main.c
+++ b/cmd/zpios/zpios_main.c
@@ -525,10 +525,11 @@ run_one(cmd_args_t *args, uint32_t id, uint32_t T, uint32_t N,
 
        memset(cmd, 0, cmd_size);
        cmd->cmd_magic = ZPIOS_CMD_MAGIC;
-       strncpy(cmd->cmd_pool, args->pool, ZPIOS_NAME_SIZE - 1);
-       strncpy(cmd->cmd_pre, args->pre, ZPIOS_PATH_SIZE - 1);
-       strncpy(cmd->cmd_post, args->post, ZPIOS_PATH_SIZE - 1);
-       strncpy(cmd->cmd_log, args->log, ZPIOS_PATH_SIZE - 1);
+       snprintf(cmd->cmd_pool, sizeof (cmd->cmd_pool), "%s", args->pool);
+       snprintf(cmd->cmd_pre, sizeof (cmd->cmd_pre), "%s", args->pre);
+       snprintf(cmd->cmd_post, sizeof (cmd->cmd_post), "%s", args->post);
+       snprintf(cmd->cmd_log, sizeof (cmd->cmd_log), "%s", args->log);
+
        cmd->cmd_id = id;
        cmd->cmd_chunk_size = C;
        cmd->cmd_thread_count = T;

diff --git a/cmd/zvol_id/zvol_id_main.c b/cmd/zvol_id/zvol_id_main.c
index 6bd5113f1ea7cbff4cc6f10613e57d0740999dae..4a2d74cc203cfa394a8096c07bb4bc1c8d46d273 100644 (file)
--- a/cmd/zvol_id/zvol_id_main.c
+++ b/cmd/zvol_id/zvol_id_main.c
@@ -55,11 +55,12 @@ main(int argc, char **argv)
 {
        int fd, error = 0;
        char zvol_name[ZFS_MAX_DATASET_NAME_LEN];
-       char zvol_name_part[ZFS_MAX_DATASET_NAME_LEN];
+       char *zvol_name_part = NULL;
        char *dev_name;
        struct stat64 statbuf;
        int dev_minor, dev_part;
        int i;
+       int rc;
 
        if (argc < 2) {
                printf("Usage: %s /dev/zvol_device_node\n", argv[0]);
@@ -88,11 +89,13 @@ main(int argc, char **argv)
                return (errno);
        }
        if (dev_part > 0)
-               snprintf(zvol_name_part, ZFS_MAX_DATASET_NAME_LEN,
-                   "%s-part%d", zvol_name, dev_part);
+               rc = asprintf(&zvol_name_part, "%s-part%d", zvol_name,
+                   dev_part);
        else
-               snprintf(zvol_name_part, ZFS_MAX_DATASET_NAME_LEN,
-                   "%s", zvol_name);
+               rc = asprintf(&zvol_name_part, "%s", zvol_name);
+
+       if (rc == -1 || zvol_name_part == NULL)
+               goto error;
 
        for (i = 0; i < strlen(zvol_name_part); i++) {
                if (isblank(zvol_name_part[i]))
@@ -100,6 +103,8 @@ main(int argc, char **argv)
        }
 
        printf("%s\n", zvol_name_part);
+       free(zvol_name_part);
+error:
        close(fd);
        return (error);
 }

diff --git a/lib/libspl/include/umem.h b/lib/libspl/include/umem.h
index c63026ceed1a8a7f272ee3271a447e5a64750f95..5b10ac801de7b052b3934db3dfec7aaf53738732 100644 (file)
--- a/lib/libspl/include/umem.h
+++ b/lib/libspl/include/umem.h
@@ -146,7 +146,7 @@ umem_cache_create(
 
        cp = umem_alloc(sizeof (umem_cache_t), UMEM_DEFAULT);
        if (cp) {
-               strncpy(cp->cache_name, name, UMEM_CACHE_NAMELEN);
+               strlcpy(cp->cache_name, name, UMEM_CACHE_NAMELEN);
                cp->cache_bufsize = bufsize;
                cp->cache_align = align;
                cp->cache_constructor = constructor;

diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c
index b65dbc826854a766c912c52d184d4db6c7a4f205..892f058b2936a53574e5323d0a3d96bce9ef9a35 100644 (file)
--- a/lib/libzfs/libzfs_dataset.c
+++ b/lib/libzfs/libzfs_dataset.c
@@ -1030,10 +1030,11 @@ zfs_valid_proplist(libzfs_handle_t *hdl, zfs_type_t type, nvlist_t *nvl,
 
                if (prop == ZPROP_INVAL && zfs_prop_userquota(propname)) {
                        zfs_userquota_prop_t uqtype;
-                       char newpropname[128];
+                       char *newpropname = NULL;
                        char domain[128];
                        uint64_t rid;
                        uint64_t valary[3];
+                       int rc;
 
                        if (userquota_propname_decode(propname, zoned,
                            &uqtype, domain, sizeof (domain), &rid) != 0) {
@@ -1088,17 +1089,24 @@ zfs_valid_proplist(libzfs_handle_t *hdl, zfs_type_t type, nvlist_t *nvl,
                         * userquota@<hex-rid>-domain, to make it easy
                         * for the kernel to decode.
                         */
-                       (void) snprintf(newpropname, sizeof (newpropname),
-                           "%s%llx-%s", zfs_userquota_prop_prefixes[uqtype],
+                       rc = asprintf(&newpropname, "%s%llx-%s",
+                           zfs_userquota_prop_prefixes[uqtype],
                            (longlong_t)rid, domain);
+                       if (rc == -1 || newpropname == NULL) {
+                               (void) no_memory(hdl);
+                               goto error;
+                       }
+
                        valary[0] = uqtype;
                        valary[1] = rid;
                        valary[2] = intval;
                        if (nvlist_add_uint64_array(ret, newpropname,
                            valary, 3) != 0) {
+                               free(newpropname);
                                (void) no_memory(hdl);
                                goto error;
                        }
+                       free(newpropname);
                        continue;
                } else if (prop == ZPROP_INVAL && zfs_prop_written(propname)) {
                        zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,

diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c
index 66d89067bcee984207a0db215799f59958e49142..5490581abe38ef79e28a2e078f5de389479c69c4 100644 (file)
--- a/lib/libzfs/libzfs_sendrecv.c
+++ b/lib/libzfs/libzfs_sendrecv.c
@@ -3380,7 +3380,7 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
         * Determine the name of the origin snapshot.
         */
        if (originsnap) {
-               (void) strncpy(origin, originsnap, sizeof (origin));
+               (void) strlcpy(origin, originsnap, sizeof (origin));
                if (flags->verbose)
                        (void) printf("using provided clone origin %s\n",
                            origin);

diff --git a/module/zpios/pios.c b/module/zpios/pios.c
index c70c0d6f1c9e402052ae6d617bad9e05db7ab6e0..6ce96e67d9056fa1a096b405e4374dab0f9b3a26 100644 (file)
--- a/module/zpios/pios.c
+++ b/module/zpios/pios.c
@@ -289,10 +289,11 @@ zpios_setup_run(run_args_t **run_args, zpios_cmd_t *kcmd, struct file *file)
        ra = vmem_zalloc(size, KM_SLEEP);
 
        *run_args = ra;
-       strncpy(ra->pool, kcmd->cmd_pool, ZPIOS_NAME_SIZE - 1);
-       strncpy(ra->pre, kcmd->cmd_pre, ZPIOS_PATH_SIZE - 1);
-       strncpy(ra->post, kcmd->cmd_post, ZPIOS_PATH_SIZE - 1);
-       strncpy(ra->log, kcmd->cmd_log, ZPIOS_PATH_SIZE - 1);
+       snprintf(ra->pool, sizeof (ra->pool), "%s", kcmd->cmd_pool);
+       snprintf(ra->pre, sizeof (ra->pre), "%s", kcmd->cmd_pre);
+       snprintf(ra->post, sizeof (ra->post), "%s", kcmd->cmd_post);
+       snprintf(ra->log, sizeof (ra->log), "%s", kcmd->cmd_log);
+
        ra->id                  = kcmd->cmd_id;
        ra->chunk_size          = kcmd->cmd_chunk_size;
        ra->thread_count        = kcmd->cmd_thread_count;

diff --git a/tests/zfs-tests/cmd/devname2devid/devname2devid.c b/tests/zfs-tests/cmd/devname2devid/devname2devid.c
index 59fbcdddb571ac26e05033e8b36084c4f9687c7b..91e59c589fd5b87d73ff26f9d9f3144afe10729a 100644 (file)
--- a/tests/zfs-tests/cmd/devname2devid/devname2devid.c
+++ b/tests/zfs-tests/cmd/devname2devid/devname2devid.c
@@ -83,7 +83,8 @@ udev_device_get_devid(struct udev_device *dev, char *bufptr, size_t buflen)
                name = udev_list_entry_get_name(entry);
                if (strncmp(name, devbyid, strlen(devbyid)) == 0) {
                        name += strlen(DEV_BYID_PATH);
-                       (void) stpncpy(bufptr, name, buflen);
+                       (void) stpncpy(bufptr, name, buflen - 1);
+                       bufptr[buflen - 1] = '\0';
                        return (0);
                }
                entry = udev_list_entry_get_next(entry);

diff --git a/tests/zfs-tests/cmd/mkbusy/mkbusy.c b/tests/zfs-tests/cmd/mkbusy/mkbusy.c
index 1e4567488d98be997bbb343b710b3531c3252abe..9634904f0d56890cfa05239f7c124c205cd1b6dc 100644 (file)
--- a/tests/zfs-tests/cmd/mkbusy/mkbusy.c
+++ b/tests/zfs-tests/cmd/mkbusy/mkbusy.c
@@ -98,8 +98,9 @@ main(int argc, char *argv[])
 
        if ((ret = stat(argv[0], &sbuf)) != 0) {
                char    *arg, *dname, *fname;
-               int     arglen, dlen, flen;
+               int     arglen;
                char    *slash;
+               int     rc;
 
                /*
                 * The argument supplied doesn't exist. Copy the path, and
@@ -126,23 +127,18 @@ main(int argc, char *argv[])
                free(arg);
                if (dname == NULL || fname == NULL)
                        fail("strdup", 1);
-               dlen = strlen(dname);
-               flen = strlen(fname);
 
                /* The directory portion of the path must exist */
                if ((ret = stat(dname, &sbuf)) != 0 || !(sbuf.st_mode &
                    S_IFDIR))
                        usage(prog);
 
-               if ((fpath = (char *)malloc(dlen + 1 + flen + 1)) == NULL)
-                       fail("malloc", 1);
-               (void) memset(fpath, '\0', dlen + 1 + flen + 1);
-
-               (void) strncpy(fpath, dname, dlen);
-               fpath[dlen] = '/';
-               (void) strncat(fpath, fname, flen);
+               rc = asprintf(&fpath, "%s/%s", dname, fname);
                free(dname);
                free(fname);
+               if (rc == -1 || fpath == NULL)
+                       fail("asprintf", 1);
+
        } else if ((sbuf.st_mode & S_IFMT) == S_IFREG ||
            (sbuf.st_mode & S_IFMT) == S_IFLNK ||
            (sbuf.st_mode & S_IFMT) == S_IFCHR ||

diff --git a/tests/zfs-tests/cmd/mktree/mktree.c b/tests/zfs-tests/cmd/mktree/mktree.c
index bf0ec5e0cc73a8f3cf5149b72fd774f6ad6d218a..02d4974d783ffb8ad37ee5d82d73296e7e329b96 100644 (file)
--- a/tests/zfs-tests/cmd/mktree/mktree.c
+++ b/tests/zfs-tests/cmd/mktree/mktree.c
@@ -137,8 +137,12 @@ mktree(char *pdir, int level)
 static char *
 getfdname(char *pdir, char type, int level, int dir, int file)
 {
-       (void) snprintf(fdname, sizeof (fdname),
-           "%s/%c-l%dd%df%d", pdir, type, level, dir, file);
+       size_t size = sizeof (fdname);
+       if (snprintf(fdname, size, "%s/%c-l%dd%df%d", pdir, type, level, dir,
+           file) >= size) {
+               (void) fprintf(stderr, "fdname truncated\n");
+               exit(EINVAL);
+       }
        return (fdname);
 }
 

diff --git a/tests/zfs-tests/cmd/xattrtest/xattrtest.c b/tests/zfs-tests/cmd/xattrtest/xattrtest.c
index dd3f2a6c975de9608215eed7638aecd41f424937..32a6b1d95b10069af3edf942874602d6786617e0 100644 (file)
--- a/tests/zfs-tests/cmd/xattrtest/xattrtest.c
+++ b/tests/zfs-tests/cmd/xattrtest/xattrtest.c
@@ -367,8 +367,10 @@ create_files(void)
        char *file = NULL;
        struct timeval start, stop;
        double seconds;
+       size_t fsize;
 
-       file = malloc(PATH_MAX);
+       fsize = PATH_MAX;
+       file = malloc(fsize);
        if (file == NULL) {
                rc = ENOMEM;
                ERROR("Error %d: malloc(%d) bytes for file name\n", rc,
@@ -379,7 +381,11 @@ create_files(void)
        (void) gettimeofday(&start, NULL);
 
        for (i = 1; i <= files; i++) {
-               (void) sprintf(file, "%s/file-%d", path, i);
+               if (snprintf(file, fsize, "%s/file-%d", path, i) >= fsize) {
+                       rc = EINVAL;
+                       ERROR("Error %d: path too long\n", rc);
+                       goto out;
+               }
 
                if (nth && ((i % nth) == 0))
                        fprintf(stdout, "create: %s\n", file);
@@ -452,6 +458,7 @@ setxattrs(void)
        char *file = NULL;
        struct timeval start, stop;
        double seconds;
+       size_t fsize;
 
        value = malloc(XATTR_SIZE_MAX);
        if (value == NULL) {
@@ -461,7 +468,8 @@ setxattrs(void)
                goto out;
        }
 
-       file = malloc(PATH_MAX);
+       fsize = PATH_MAX;
+       file = malloc(fsize);
        if (file == NULL) {
                rc = ENOMEM;
                ERROR("Error %d: malloc(%d) bytes for file name\n", rc,
@@ -472,7 +480,11 @@ setxattrs(void)
        (void) gettimeofday(&start, NULL);
 
        for (i = 1; i <= files; i++) {
-               (void) sprintf(file, "%s/file-%d", path, i);
+               if (snprintf(file, fsize, "%s/file-%d", path, i) >= fsize) {
+                       rc = EINVAL;
+                       ERROR("Error %d: path too long\n", rc);
+                       goto out;
+               }
 
                if (nth && ((i % nth) == 0))
                        fprintf(stdout, "setxattr: %s\n", file);
@@ -523,6 +535,7 @@ getxattrs(void)
        char *file = NULL;
        struct timeval start, stop;
        double seconds;
+       size_t fsize;
 
        verify_value = malloc(XATTR_SIZE_MAX);
        if (verify_value == NULL) {
@@ -543,7 +556,9 @@ getxattrs(void)
        verify_string = value_is_random ? "<random>" : verify_value;
        value_string = value_is_random ? "<random>" : value;
 
-       file = malloc(PATH_MAX);
+       fsize = PATH_MAX;
+       file = malloc(fsize);
+
        if (file == NULL) {
                rc = ENOMEM;
                ERROR("Error %d: malloc(%d) bytes for file name\n", rc,
@@ -554,7 +569,11 @@ getxattrs(void)
        (void) gettimeofday(&start, NULL);
 
        for (i = 1; i <= files; i++) {
-               (void) sprintf(file, "%s/file-%d", path, i);
+               if (snprintf(file, fsize, "%s/file-%d", path, i) >= fsize) {
+                       rc = EINVAL;
+                       ERROR("Error %d: path too long\n", rc);
+                       goto out;
+               }
 
                if (nth && ((i % nth) == 0))
                        fprintf(stdout, "getxattr: %s\n", file);
@@ -615,8 +634,10 @@ unlink_files(void)
        char *file = NULL;
        struct timeval start, stop;
        double seconds;
+       size_t fsize;
 
-       file = malloc(PATH_MAX);
+       fsize = PATH_MAX;
+       file = malloc(fsize);
        if (file == NULL) {
                rc = ENOMEM;
                ERROR("Error %d: malloc(%d) bytes for file name\n",
@@ -627,7 +648,11 @@ unlink_files(void)
        (void) gettimeofday(&start, NULL);
 
        for (i = 1; i <= files; i++) {
-               (void) sprintf(file, "%s/file-%d", path, i);
+               if (snprintf(file, fsize, "%s/file-%d", path, i) >= fsize) {
+                       rc = EINVAL;
+                       ERROR("Error %d: path too long\n", rc);
+                       goto out;
+               }
 
                if (nth && ((i % nth) == 0))
                        fprintf(stdout, "unlink: %s\n", file);