From 682ce104cdd80db4b67eea09eb0a90324c5f98ee Mon Sep 17 00:00:00 2001 From: Tony Hutter Date: Wed, 28 Jun 2017 10:05:16 -0700 Subject: [PATCH] GCC 7.1 fixes GCC 7.1 with will warn when we're not checking the snprintf() return code in cases where the buffer could be truncated. This patch either checks the snprintf return code (where applicable), or simply disables the warnings (ztest.c). Reviewed-by: Chunwei Chen Reviewed-by: Brian Behlendorf Signed-off-by: Tony Hutter Closes #6253 --- cmd/ztest/Makefile.am | 4 +++- config/user-no-format-truncation.m4 | 22 ++++++++++++++++++++++ config/user.m4 | 1 + lib/libspl/include/assert.h | 24 +++++++++++++++++------- lib/libzfs/libzfs_dataset.c | 22 ++++++++++++++-------- lib/libzfs/libzfs_iter.c | 7 +++++-- lib/libzfs/libzfs_sendrecv.c | 8 ++++++-- module/zfs/zfs_ioctl.c | 6 ++++-- module/zfs/zvol.c | 7 ++----- 9 files changed, 74 insertions(+), 27 deletions(-) create mode 100644 config/user-no-format-truncation.m4 diff --git a/cmd/ztest/Makefile.am b/cmd/ztest/Makefile.am index a2f3b5a6b8a2..5167d0c1d86e 100644 --- a/cmd/ztest/Makefile.am +++ b/cmd/ztest/Makefile.am @@ -1,6 +1,8 @@ include $(top_srcdir)/config/Rules.am -AM_CFLAGS += $(DEBUG_STACKFLAGS) $(FRAME_LARGER_THAN) +# -Wnoformat-truncation to get rid of compiler warning for unchecked +# truncating snprintfs on gcc 7.1.1. +AM_CFLAGS += $(DEBUG_STACKFLAGS) $(FRAME_LARGER_THAN) $(NO_FORMAT_TRUNCATION) AM_CPPFLAGS += -DDEBUG DEFAULT_INCLUDES += \ diff --git a/config/user-no-format-truncation.m4 b/config/user-no-format-truncation.m4 new file mode 100644 index 000000000000..4426907eeb4d --- /dev/null +++ b/config/user-no-format-truncation.m4 @@ -0,0 +1,22 @@ +dnl # +dnl # Check if gcc supports -Wno-format-truncation option. +dnl # +AC_DEFUN([ZFS_AC_CONFIG_USER_NO_FORMAT_TRUNCATION], [ + AC_MSG_CHECKING([for -Wno-format-truncation support]) + + saved_flags="$CFLAGS" + CFLAGS="$CFLAGS -Wno-format-truncation" + + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [])], + [ + NO_FORMAT_TRUNCATION=-Wno-format-truncation + AC_MSG_RESULT([yes]) + ], + [ + NO_FORMAT_TRUNCATION= + AC_MSG_RESULT([no]) + ]) + + CFLAGS="$saved_flags" + AC_SUBST([NO_FORMAT_TRUNCATION]) +]) diff --git a/config/user.m4 b/config/user.m4 index 079e2e73b6e5..21ff7143a56b 100644 --- a/config/user.m4 +++ b/config/user.m4 @@ -17,6 +17,7 @@ AC_DEFUN([ZFS_AC_CONFIG_USER], [ ZFS_AC_CONFIG_USER_RUNSTATEDIR ZFS_AC_CONFIG_USER_MAKEDEV_IN_SYSMACROS ZFS_AC_CONFIG_USER_MAKEDEV_IN_MKDEV + ZFS_AC_CONFIG_USER_NO_FORMAT_TRUNCATION ZFS_AC_TEST_FRAMEWORK diff --git a/lib/libspl/include/assert.h b/lib/libspl/include/assert.h index bd89ad94fa1c..6237d6bcffd9 100644 --- a/lib/libspl/include/assert.h +++ b/lib/libspl/include/assert.h @@ -40,6 +40,20 @@ libspl_assert(const char *buf, const char *file, const char *func, int line) abort(); } +/* printf version of libspl_assert */ +static inline void +libspl_assertf(const char *file, const char *func, int line, char *format, ...) +{ + va_list args; + + va_start(args, format); + vfprintf(stderr, format, args); + fprintf(stderr, "\n"); + fprintf(stderr, "ASSERT at %s:%d:%s()", file, line, func); + va_end(args); + abort(); +} + #ifdef verify #undef verify #endif @@ -55,13 +69,9 @@ libspl_assert(const char *buf, const char *file, const char *func, int line) do { \ const TYPE __left = (TYPE)(LEFT); \ const TYPE __right = (TYPE)(RIGHT); \ - if (!(__left OP __right)) { \ - char *__buf = alloca(256); \ - (void) snprintf(__buf, 256, \ - "%s %s %s (0x%llx %s 0x%llx)", #LEFT, #OP, #RIGHT, \ - (u_longlong_t)__left, #OP, (u_longlong_t)__right); \ - libspl_assert(__buf, __FILE__, __FUNCTION__, __LINE__); \ - } \ + if (!(__left OP __right)) \ + libspl_assertf(__FILE__, __FUNCTION__, __LINE__, \ + "%s %s %s (0x%llx %s 0x%llx)", #LEFT, #OP, #RIGHT); \ } while (0) #define VERIFY3S(x, y, z) VERIFY3_IMPL(x, y, z, int64_t) diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index bc630919a6a6..51c168ad7902 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -3567,8 +3567,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); @@ -3782,8 +3783,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); @@ -4527,8 +4529,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); @@ -4647,8 +4650,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 8cdfe31a2f7f..a2f7f045f1f6 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -3628,8 +3628,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 (SET_ERROR(EINVAL)); /* * Try to remove the hidden child (%recv) and after diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index 75ed06e03300..528acbc227f3 100644 --- a/module/zfs/zvol.c +++ b/module/zfs/zvol.c @@ -2154,14 +2154,12 @@ zvol_rename_minors_impl(const char *oldname, const char *newname) { zvol_state_t *zv, *zv_next; int oldnamelen, newnamelen; - char *name; if (zvol_inhibit_dev) return; oldnamelen = strlen(oldname); newnamelen = strlen(newname); - name = kmem_alloc(MAXNAMELEN, KM_SLEEP); mutex_enter(&zvol_state_lock); @@ -2181,18 +2179,17 @@ 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, + char *name = kmem_asprintf("%s%c%s", newname, zv->zv_name[oldnamelen], zv->zv_name + oldnamelen + 1); zvol_rename_minor(zv, name); + kmem_free(name, strlen(name + 1)); } mutex_exit(&zv->zv_state_lock); } mutex_exit(&zvol_state_lock); - - kmem_free(name, MAXNAMELEN); } typedef struct zvol_snapdev_cb_arg {