Skip to content

Commit

Permalink
Fix improper null-byte termination handling
Browse files Browse the repository at this point in the history
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 <bass6@llnl.gov>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #2875
  • Loading branch information
nedbass authored and behlendorf committed Nov 17, 2014
1 parent 89b1cd6 commit 7b2d78a
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 16 deletions.
6 changes: 2 additions & 4 deletions module/zfs/dbuf_stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand All @@ -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);
}
Expand All @@ -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 | "
Expand Down Expand Up @@ -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);
}
Expand Down
16 changes: 4 additions & 12 deletions module/zfs/spa_stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 7b2d78a

Please sign in to comment.