Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
GCC 7.1 fixes
Browse files Browse the repository at this point in the history
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 converts the snprintfs to a new SNPRINTF_TRUNC() macro,
which silently truncates without the compiler error.

Signed-off-by: Tony Hutter <[email protected]>
tonyhutter committed Jun 22, 2017
1 parent d9ad3fe commit 023a365
Showing 8 changed files with 70 additions and 29 deletions.
26 changes: 14 additions & 12 deletions cmd/ztest/ztest.c
Original file line number Diff line number Diff line change
@@ -126,6 +126,7 @@
#include <sys/fs/zfs.h>
#include <zfs_fletcher.h>
#include <libnvpair.h>
#include <zfs_comutil.h>
#ifdef __GLIBC__
#include <execinfo.h> /* 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);
}
21 changes: 21 additions & 0 deletions include/zfs_comutil.h
Original file line number Diff line number Diff line change
@@ -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
6 changes: 4 additions & 2 deletions lib/libspl/include/assert.h
Original file line number Diff line number Diff line change
@@ -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 = "<can't alloc>"; \
libspl_assert(__buf, __FILE__, __FUNCTION__, __LINE__); \
} \
} while (0)
22 changes: 14 additions & 8 deletions lib/libzfs/libzfs_dataset.c
Original file line number Diff line number Diff line change
@@ -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;
7 changes: 5 additions & 2 deletions lib/libzfs/libzfs_iter.c
Original file line number Diff line number Diff line change
@@ -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)
8 changes: 6 additions & 2 deletions lib/libzfs/libzfs_sendrecv.c
Original file line number Diff line number Diff line change
@@ -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);
6 changes: 4 additions & 2 deletions module/zfs/zfs_ioctl.c
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion module/zfs/zvol.c
Original file line number Diff line number Diff line change
@@ -79,6 +79,7 @@
#include <sys/zfs_znode.h>
#include <sys/spa_impl.h>
#include <sys/zvol.h>
#include <zfs_comutil.h>
#include <linux/blkdev_compat.h>

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);

0 comments on commit 023a365

Please sign in to comment.