diff --git a/cmd/ztest/ztest.c b/cmd/ztest/ztest.c index 39fbfd993adf..3a72901114da 100644 --- a/cmd/ztest/ztest.c +++ b/cmd/ztest/ztest.c @@ -126,6 +126,7 @@ #include #include #include +#include #ifdef __GLIBC__ #include /* for backtrace() */ #endif @@ -3540,8 +3541,9 @@ ztest_snapshot_destroy(char *osname, uint64_t id) char snapname[ZFS_MAX_DATASET_NAME_LEN]; int error; - (void) snprintf(snapname, sizeof (snapname), "%s@%llu", osname, - (u_longlong_t)id); + if (snprintf(snapname, sizeof (snapname), "%s@%llu", osname, + (u_longlong_t)id) >= sizeof (snapname)) + return (B_FALSE); error = dsl_destroy_snapshot(snapname, B_FALSE); if (error != 0 && error != ENOENT) @@ -3565,8 +3567,8 @@ ztest_dmu_objset_create_destroy(ztest_ds_t *zd, uint64_t id) (void) rw_rdlock(&ztest_name_lock); - (void) snprintf(name, sizeof (name), "%s/temp_%llu", - ztest_opts.zo_pool, (u_longlong_t)id); + SNPRINTF_TRUNC(name, sizeof (name), "%s/temp_%llu", ztest_opts.zo_pool, + (u_longlong_t)id); /* * If this dataset exists from a previous run, process its replay log @@ -3744,15 +3746,15 @@ ztest_dsl_dataset_promote_busy(ztest_ds_t *zd, uint64_t id) ztest_dsl_dataset_cleanup(osname, id); - (void) snprintf(snap1name, ZFS_MAX_DATASET_NAME_LEN, + SNPRINTF_TRUNC(snap1name, ZFS_MAX_DATASET_NAME_LEN, "%s@s1_%llu", osname, (u_longlong_t)id); - (void) snprintf(clone1name, ZFS_MAX_DATASET_NAME_LEN, + SNPRINTF_TRUNC(clone1name, ZFS_MAX_DATASET_NAME_LEN, "%s/c1_%llu", osname, (u_longlong_t)id); - (void) snprintf(snap2name, ZFS_MAX_DATASET_NAME_LEN, + SNPRINTF_TRUNC(snap2name, ZFS_MAX_DATASET_NAME_LEN, "%s@s2_%llu", clone1name, (u_longlong_t)id); - (void) snprintf(clone2name, ZFS_MAX_DATASET_NAME_LEN, + SNPRINTF_TRUNC(clone2name, ZFS_MAX_DATASET_NAME_LEN, "%s/c2_%llu", osname, (u_longlong_t)id); - (void) snprintf(snap3name, ZFS_MAX_DATASET_NAME_LEN, + SNPRINTF_TRUNC(snap3name, ZFS_MAX_DATASET_NAME_LEN, "%s@s3_%llu", clone1name, (u_longlong_t)id); error = dmu_objset_snapshot_one(osname, strchr(snap1name, '@') + 1); @@ -6120,7 +6122,7 @@ ztest_thread(void *arg) static void ztest_dataset_name(char *dsname, char *pool, int d) { - (void) snprintf(dsname, ZFS_MAX_DATASET_NAME_LEN, "%s/ds_%d", pool, d); + SNPRINTF_TRUNC(dsname, ZFS_MAX_DATASET_NAME_LEN, "%s/ds_%d", pool, d); } static void @@ -6420,7 +6422,7 @@ ztest_run(ztest_shared_t *zs) */ if (ztest_random(2) == 0) { char name[ZFS_MAX_DATASET_NAME_LEN]; - (void) snprintf(name, sizeof (name), "%s_import", + SNPRINTF_TRUNC(name, sizeof (name), "%s_import", ztest_opts.zo_pool); ztest_spa_import_export(ztest_opts.zo_pool, name); ztest_spa_import_export(name, ztest_opts.zo_pool); @@ -7000,7 +7002,7 @@ main(int argc, char **argv) char tmpname[ZFS_MAX_DATASET_NAME_LEN]; kernel_fini(); kernel_init(FREAD | FWRITE); - (void) snprintf(tmpname, sizeof (tmpname), "%s_tmp", + SNPRINTF_TRUNC(tmpname, sizeof (tmpname), "%s_tmp", ztest_opts.zo_pool); (void) spa_rename(tmpname, ztest_opts.zo_pool); } diff --git a/include/zfs_comutil.h b/include/zfs_comutil.h index f89054388a4d..c4c1fd1e5a6e 100644 --- a/include/zfs_comutil.h +++ b/include/zfs_comutil.h @@ -41,6 +41,27 @@ extern int zfs_spa_version_map(int zpl_version); #define ZFS_NUM_LEGACY_HISTORY_EVENTS 41 extern const char *zfs_history_event_names[ZFS_NUM_LEGACY_HISTORY_EVENTS]; +/* + * Truncating snprintf() without compiler warnings + * + * GCC 7.1 with "-Wall" will warn in cases where we're not checking the + * snprintf() return code when the buffer could be truncated: + * + * libzfs_iter.c: In function ‘zfs_iter_bookmarks’: + * libzfs_iter.c:207:40: error: ‘snprintf’ output may be truncated + * before the last format character [-Werror=format-truncation=] + * (void) snprintf(name, sizeof (name), "%s#%s", zhp->zfs_name, + * + * In many cases we don't care and just want to silently truncate the buffer. + * + * This hack fools the compiler by making the size volatile, so that it isn't + * able to compute the truncation at compile time, and thus no error. + */ +#define SNPRINTF_TRUNC(str, size, ...) { \ + volatile int s = size; \ + snprintf(str, s, __VA_ARGS__); \ +} + #ifdef __cplusplus } #endif diff --git a/lib/libspl/include/assert.h b/lib/libspl/include/assert.h index bd89ad94fa1c..657150eab643 100644 --- a/lib/libspl/include/assert.h +++ b/lib/libspl/include/assert.h @@ -56,10 +56,12 @@ do { \ const TYPE __left = (TYPE)(LEFT); \ const TYPE __right = (TYPE)(RIGHT); \ if (!(__left OP __right)) { \ - char *__buf = alloca(256); \ - (void) snprintf(__buf, 256, \ + char *__buf = NULL; \ + int rc = asprintf(&__buf, \ "%s %s %s (0x%llx %s 0x%llx)", #LEFT, #OP, #RIGHT, \ (u_longlong_t)__left, #OP, (u_longlong_t)__right); \ + if (rc == -1) \ + __buf = ""; \ libspl_assert(__buf, __FILE__, __FUNCTION__, __LINE__); \ } \ } while (0) diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index f1346b69c6f4..46a40f7896e5 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -3566,8 +3566,9 @@ zfs_check_snap_cb(zfs_handle_t *zhp, void *arg) char name[ZFS_MAX_DATASET_NAME_LEN]; int rv = 0; - (void) snprintf(name, sizeof (name), - "%s@%s", zhp->zfs_name, dd->snapname); + if (snprintf(name, sizeof (name), "%s@%s", zhp->zfs_name, + dd->snapname) >= sizeof (name)) + return (EINVAL); if (lzc_exists(name)) verify(nvlist_add_boolean(dd->nvl, name) == 0); @@ -3788,8 +3789,9 @@ zfs_snapshot_cb(zfs_handle_t *zhp, void *arg) int rv = 0; if (zfs_prop_get_int(zhp, ZFS_PROP_INCONSISTENT) == 0) { - (void) snprintf(name, sizeof (name), - "%s@%s", zfs_get_name(zhp), sd->sd_snapname); + if (snprintf(name, sizeof (name), "%s@%s", zfs_get_name(zhp), + sd->sd_snapname) >= sizeof (name)) + return (EINVAL); fnvlist_add_boolean(sd->sd_nvl, name); @@ -4533,8 +4535,9 @@ zfs_hold_one(zfs_handle_t *zhp, void *arg) char name[ZFS_MAX_DATASET_NAME_LEN]; int rv = 0; - (void) snprintf(name, sizeof (name), - "%s@%s", zhp->zfs_name, ha->snapname); + if (snprintf(name, sizeof (name), "%s@%s", zhp->zfs_name, + ha->snapname) >= sizeof (name)) + return (EINVAL); if (lzc_exists(name)) fnvlist_add_string(ha->nvl, name, ha->tag); @@ -4653,8 +4656,11 @@ zfs_release_one(zfs_handle_t *zhp, void *arg) int rv = 0; nvlist_t *existing_holds; - (void) snprintf(name, sizeof (name), - "%s@%s", zhp->zfs_name, ha->snapname); + if (snprintf(name, sizeof (name), "%s@%s", zhp->zfs_name, + ha->snapname) >= sizeof (name)) { + ha->error = EINVAL; + rv = EINVAL; + } if (lzc_get_holds(name, &existing_holds) != 0) { ha->error = ENOENT; diff --git a/lib/libzfs/libzfs_iter.c b/lib/libzfs/libzfs_iter.c index d78c757a58b7..57ebdd89d1b8 100644 --- a/lib/libzfs/libzfs_iter.c +++ b/lib/libzfs/libzfs_iter.c @@ -204,8 +204,11 @@ zfs_iter_bookmarks(zfs_handle_t *zhp, zfs_iter_f func, void *data) bmark_name = nvpair_name(pair); bmark_props = fnvpair_value_nvlist(pair); - (void) snprintf(name, sizeof (name), "%s#%s", zhp->zfs_name, - bmark_name); + if (snprintf(name, sizeof (name), "%s#%s", zhp->zfs_name, + bmark_name) >= sizeof (name)) { + err = EINVAL; + goto out; + } nzhp = make_bookmark_handle(zhp, name, bmark_props); if (nzhp == NULL) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 71ee8faaeaa3..ff909f1e3574 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -1867,9 +1867,13 @@ zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap, drr_versioninfo, DMU_COMPOUNDSTREAM); DMU_SET_FEATUREFLAGS(drr.drr_u.drr_begin. drr_versioninfo, featureflags); - (void) snprintf(drr.drr_u.drr_begin.drr_toname, + if (snprintf(drr.drr_u.drr_begin.drr_toname, sizeof (drr.drr_u.drr_begin.drr_toname), - "%s@%s", zhp->zfs_name, tosnap); + "%s@%s", zhp->zfs_name, tosnap) >= + sizeof (drr.drr_u.drr_begin.drr_toname)) { + err = EINVAL; + goto stderr_out; + } drr.drr_payloadlen = buflen; err = dump_record(&drr, packbuf, buflen, &zc, outfd); diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index 4fda36c7f047..ec1754dab397 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -3627,8 +3627,10 @@ zfs_ioc_destroy(zfs_cmd_t *zc) */ char namebuf[ZFS_MAX_DATASET_NAME_LEN + 6]; - (void) snprintf(namebuf, sizeof (namebuf), - "%s/%s", zc->zc_name, recv_clone_name); + if (snprintf(namebuf, sizeof (namebuf), "%s/%s", + zc->zc_name, recv_clone_name) >= + sizeof (namebuf)) + return (EINVAL); /* * Try to remove the hidden child (%recv) and after diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index 8890aaaf78a3..6b25037c44b5 100644 --- a/module/zfs/zvol.c +++ b/module/zfs/zvol.c @@ -79,6 +79,7 @@ #include #include #include +#include #include unsigned int zvol_inhibit_dev = 0; @@ -2086,7 +2087,7 @@ zvol_rename_minors_impl(const char *oldname, const char *newname) } else if (strncmp(zv->zv_name, oldname, oldnamelen) == 0 && (zv->zv_name[oldnamelen] == '/' || zv->zv_name[oldnamelen] == '@')) { - snprintf(name, MAXNAMELEN, "%s%c%s", newname, + SNPRINTF_TRUNC(name, MAXNAMELEN, "%s%c%s", newname, zv->zv_name[oldnamelen], zv->zv_name + oldnamelen + 1); zvol_rename_minor(zv, name);