From 21a4f5cc867b4f166704df84985edc048caa2578 Mon Sep 17 00:00:00 2001 From: Tony Hutter Date: Wed, 4 Apr 2018 10:16:47 -0700 Subject: [PATCH] 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 Signed-off-by: Tony Hutter Closes #7361 Closes #7368 --- cmd/zvol_id/zvol_id_main.c | 15 ++++--- lib/libspl/include/umem.h | 2 +- lib/libzfs/libzfs_dataset.c | 14 +++++-- lib/libzfs/libzfs_sendrecv.c | 2 +- module/zfs/dsl_scan.c | 3 +- .../cmd/devname2devid/devname2devid.c | 3 +- tests/zfs-tests/cmd/mkbusy/mkbusy.c | 16 +++----- tests/zfs-tests/cmd/mktree/mktree.c | 8 +++- tests/zfs-tests/cmd/xattrtest/xattrtest.c | 41 +++++++++++++++---- .../functional/cli_root/zfs_diff/socket.c | 7 +++- 10 files changed, 77 insertions(+), 34 deletions(-) diff --git a/cmd/zvol_id/zvol_id_main.c b/cmd/zvol_id/zvol_id_main.c index 6bd5113f1e..4a2d74cc20 100644 --- 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 c63026ceed..5b10ac801d 100644 --- 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 17c17fa798..8ec5b7af4b 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -1032,10 +1032,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) { @@ -1092,17 +1093,24 @@ zfs_valid_proplist(libzfs_handle_t *hdl, zfs_type_t type, nvlist_t *nvl, * userquota@-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 1623a75b4b..b138745b84 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -3758,7 +3758,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/zfs/dsl_scan.c b/module/zfs/dsl_scan.c index dba3bf2454..90534b4fa3 100644 --- a/module/zfs/dsl_scan.c +++ b/module/zfs/dsl_scan.c @@ -1176,14 +1176,15 @@ dsl_scan_check_suspend(dsl_scan_t *scn, const zbookmark_phys_t *zb) (longlong_t)zb->zb_blkid); scn->scn_phys.scn_bookmark = *zb; } else { +#ifdef ZFS_DEBUG dsl_scan_phys_t *scnp = &scn->scn_phys; - dprintf("suspending at at DDT bookmark " "%llx/%llx/%llx/%llx\n", (longlong_t)scnp->scn_ddt_bookmark.ddb_class, (longlong_t)scnp->scn_ddt_bookmark.ddb_type, (longlong_t)scnp->scn_ddt_bookmark.ddb_checksum, (longlong_t)scnp->scn_ddt_bookmark.ddb_cursor); +#endif } scn->scn_suspending = B_TRUE; return (B_TRUE); diff --git a/tests/zfs-tests/cmd/devname2devid/devname2devid.c b/tests/zfs-tests/cmd/devname2devid/devname2devid.c index 59fbcdddb5..91e59c589f 100644 --- 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 1e4567488d..9634904f0d 100644 --- 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 bf0ec5e0cc..02d4974d78 100644 --- 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 dd3f2a6c97..32a6b1d95b 100644 --- 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 ? "" : verify_value; value_string = value_is_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); diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_diff/socket.c b/tests/zfs-tests/tests/functional/cli_root/zfs_diff/socket.c index 75e6e1ecf8..2fe9de77ce 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_diff/socket.c +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_diff/socket.c @@ -30,13 +30,16 @@ main(int argc, char *argv[]) struct sockaddr_un sock; int fd; char *path; - + size_t size; if (argc != 2) { fprintf(stderr, "usage: %s /path/to/socket\n", argv[0]); exit(1); } path = argv[1]; - strncpy(sock.sun_path, (char *)path, sizeof (sock.sun_path)); + size = sizeof (sock.sun_path); + strncpy(sock.sun_path, (char *)path, size - 1); + sock.sun_path[size - 1] = '\0'; + sock.sun_family = AF_UNIX; if ((fd = socket(AF_UNIX, SOCK_DGRAM, 0)) == -1) { perror("socket");