Fix improper null-byte termination handling

Fix a few cases where null-byte termination of strings was done
unnecessarily or incorrectly.

- The snprintf() function always produces a null-byte terminated string
  for non-negative return values, so it is not necessary to write out a
  null-byte as a separate step.

- Also, it is unsafe to use the return value of snprintf() as an offset
  for placing a null-byte, because if the output was truncated the return
  value is the number of bytes that _would_ have been written had enough
  space been available. Therefore the return value may index beyond the
  array boundaries.

- Finally, snprintf() accounts for the null-byte when limiting its output
  size, so there is no need to pass it a size parameter that is one less
  than the buffer size.

Signed-off-by: Ned Bass <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#2875

module/zfs/dbuf_stats.c

@@ -45,7 +45,7 @@ static dbuf_stats_t dbuf_stats_hash_table;
 static int
 dbuf_stats_hash_table_headers(char *buf, size_t size)
 {
-	size = snprintf(buf, size - 1,
+	(void) snprintf(buf, size,
 	    "%-88s | %-124s | %s\n"
 	    "%-16s %-8s %-8s %-8s %-8s %-8s %-8s %-5s %-5s %5s | "
 	    "%-5s %-5s %-6s %-8s %-6s %-8s %-12s "
@@ -57,7 +57,6 @@ dbuf_stats_hash_table_headers(char *buf, size_t size)
 	    "mru", "gmru", "mfu", "gmfu", "l2", "l2_dattr", "l2_asize",
 	    "l2_comp", "aholds", "dtype", "btype", "data_bs", "meta_bs",
 	    "bsize", "lvls", "dholds", "blocks", "dsize");
-	buf[size] = '\0';
 
 	return (0);
 }
@@ -75,7 +74,7 @@ __dbuf_stats_hash_table_data(char *buf, size_t size, dmu_buf_impl_t *db)
 	if (dn)
 		__dmu_object_info_from_dnode(dn, &doi);
 
-	size = snprintf(buf, size - 1,
+	(void) snprintf(buf, size,
 	    "%-16s %-8llu %-8lld %-8lld %-8lld %-8llu %-8llu %-5d %-5d %-5lu | "
 	    "%-5d %-5d %-6lld 0x%-6x %-6lu %-8llu %-12llu "
 	    "%-6lu %-6lu %-6lu %-6lu %-6lu %-8llu %-8llu %-8d %-5lu | "
@@ -118,7 +117,6 @@ __dbuf_stats_hash_table_data(char *buf, size_t size, dmu_buf_impl_t *db)
 	    (ulong_t)refcount_count(&dn->dn_holds),
 	    (u_longlong_t)doi.doi_fill_count,
 	    (u_longlong_t)doi.doi_max_offset);
-	buf[size] = '\0';
 
 	return (size);
 }

module/zfs/spa_stats.c

@@ -63,10 +63,9 @@ typedef struct spa_read_history {
 static int
 spa_read_history_headers(char *buf, size_t size)
 {
-	size = snprintf(buf, size - 1, "%-8s %-16s %-8s %-8s %-8s %-8s %-8s "
+	(void) snprintf(buf, size, "%-8s %-16s %-8s %-8s %-8s %-8s %-8s "
 	    "%-24s %-8s %-16s\n", "UID", "start", "objset", "object",
 	    "level", "blkid", "aflags", "origin", "pid", "process");
-	buf[size] = '\0';
 
 	return (0);
 }
@@ -76,13 +75,12 @@ spa_read_history_data(char *buf, size_t size, void *data)
 {
 	spa_read_history_t *srh = (spa_read_history_t *)data;
 
-	size = snprintf(buf, size - 1, "%-8llu %-16llu 0x%-6llx "
+	(void) snprintf(buf, size, "%-8llu %-16llu 0x%-6llx "
 	    "%-8lli %-8lli %-8lli 0x%-6x %-24s %-8i %-16s\n",
 	    (u_longlong_t)srh->uid, srh->start,
 	    (longlong_t)srh->objset, (longlong_t)srh->object,
 	    (longlong_t)srh->level, (longlong_t)srh->blkid,
 	    srh->aflags, srh->origin, srh->pid, srh->comm);
-	buf[size] = '\0';
 
 	return (0);
 }
@@ -150,7 +148,6 @@ spa_read_history_init(spa_t *spa)
 	ssh->private = NULL;
 
 	(void) snprintf(name, KSTAT_STRLEN, "zfs/%s", spa_name(spa));
-	name[KSTAT_STRLEN-1] = '\0';
 
 	ksp = kstat_create(name, 0, "reads", "misc",
 	    KSTAT_TYPE_RAW, 0, KSTAT_FLAG_VIRTUAL);
@@ -256,11 +253,10 @@ typedef struct spa_txg_history {
 static int
 spa_txg_history_headers(char *buf, size_t size)
 {
-	size = snprintf(buf, size - 1, "%-8s %-16s %-5s %-12s %-12s %-12s "
+	(void) snprintf(buf, size, "%-8s %-16s %-5s %-12s %-12s %-12s "
 	    "%-8s %-8s %-12s %-12s %-12s %-12s\n", "txg", "birth", "state",
 	    "ndirty", "nread", "nwritten", "reads", "writes",
 	    "otime", "qtime", "wtime", "stime");
-	buf[size] = '\0';
 
 	return (0);
 }
@@ -298,15 +294,14 @@ spa_txg_history_data(char *buf, size_t size, void *data)
 		sync = sth->times[TXG_STATE_SYNCED] -
 		    sth->times[TXG_STATE_WAIT_FOR_SYNC];
 
-	size = snprintf(buf, size - 1, "%-8llu %-16llu %-5c %-12llu "
+	(void) snprintf(buf, size, "%-8llu %-16llu %-5c %-12llu "
 	    "%-12llu %-12llu %-8llu %-8llu %-12llu %-12llu %-12llu %-12llu\n",
 	    (longlong_t)sth->txg, sth->times[TXG_STATE_BIRTH], state,
 	    (u_longlong_t)sth->ndirty,
 	    (u_longlong_t)sth->nread, (u_longlong_t)sth->nwritten,
 	    (u_longlong_t)sth->reads, (u_longlong_t)sth->writes,
 	    (u_longlong_t)open, (u_longlong_t)quiesce, (u_longlong_t)wait,
 	    (u_longlong_t)sync);
-	buf[size] = '\0';
 
 	return (0);
 }
@@ -376,7 +371,6 @@ spa_txg_history_init(spa_t *spa)
 	ssh->private = NULL;
 
 	(void) snprintf(name, KSTAT_STRLEN, "zfs/%s", spa_name(spa));
-	name[KSTAT_STRLEN-1] = '\0';
 
 	ksp = kstat_create(name, 0, "txgs", "misc",
 	    KSTAT_TYPE_RAW, 0, KSTAT_FLAG_VIRTUAL);
@@ -562,7 +556,6 @@ spa_tx_assign_init(spa_t *spa)
 	ssh->private = kmem_alloc(ssh->size, KM_SLEEP);
 
 	(void) snprintf(name, KSTAT_STRLEN, "zfs/%s", spa_name(spa));
-	name[KSTAT_STRLEN-1] = '\0';
 
 	for (i = 0; i < ssh->count; i++) {
 		ks = &((kstat_named_t *)ssh->private)[i];
@@ -637,7 +630,6 @@ spa_io_history_init(spa_t *spa)
 	mutex_init(&ssh->lock, NULL, MUTEX_DEFAULT, NULL);
 
 	(void) snprintf(name, KSTAT_STRLEN, "zfs/%s", spa_name(spa));
-	name[KSTAT_STRLEN-1] = '\0';
 
 	ksp = kstat_create(name, 0, "io", "disk", KSTAT_TYPE_IO, 1, 0);
 	ssh->kstat = ksp;