From b88f4d7ba7ed8ad2463d8e56dd91b9719056788b 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/zvol.c | 7 ++----- 8 files changed, 70 insertions(+), 25 deletions(-) create mode 100644 config/user-no-format-truncation.m4 diff --git a/cmd/ztest/Makefile.am b/cmd/ztest/Makefile.am index ef4d99f288..7008f4ed81 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) DEFAULT_INCLUDES += \ -I$(top_srcdir)/include \ diff --git a/config/user-no-format-truncation.m4 b/config/user-no-format-truncation.m4 new file mode 100644 index 0000000000..4426907eeb --- /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 e1c1408bec..bb7393cb9d 100644 --- a/config/user.m4 +++ b/config/user.m4 @@ -15,6 +15,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 dnl # dnl # Checks for library functions AC_CHECK_FUNCS([mlockall]) diff --git a/lib/libspl/include/assert.h b/lib/libspl/include/assert.h index 6226872e5d..b251ace7b8 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 dde1a9a0fc..88d7733ea4 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -3315,8 +3315,9 @@ zfs_check_snap_cb(zfs_handle_t *zhp, void *arg) char name[ZFS_MAXNAMELEN]; 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); @@ -3534,8 +3535,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); @@ -4257,8 +4259,9 @@ zfs_hold_one(zfs_handle_t *zhp, void *arg) char name[ZFS_MAXNAMELEN]; 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); @@ -4377,8 +4380,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 5c1cf966a5..cb30ad8099 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 735d08c9d9..62945a9cc1 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -1487,9 +1487,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 = cksum_and_write(&drr, sizeof (drr), &zc, outfd); diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index 0bb68f9b85..cfdc709864 100644 --- a/module/zfs/zvol.c +++ b/module/zfs/zvol.c @@ -1615,14 +1615,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); @@ -1638,16 +1636,15 @@ 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(&zvol_state_lock); - - kmem_free(name, MAXNAMELEN); } typedef struct zvol_snapdev_cb_arg {