Skip to content

Commit

Permalink
Cleanup dump_bookmarks()
Browse files Browse the repository at this point in the history
Assertions are meant to check assumptions, but the way that this
assertion is written does not check an assumption, since it is provably
always true. Removing the assertion will cause a compiler warning (made
into an error by -Werror) about printing up to 512 bytes to a 256-byte
buffer, so instead, we change the assertion to verify the assumption
that we never do a snprintf() that is truncated to avoid overrunning the
256-byte buffer.

This was caught by an audit of the codebase to look for misuse of
`snprintf()` after CodeQL reported that we had misused `snprintf()`. An
explanation of how snprintf() can be misused is here:

https://www.redhat.com/en/blog/trouble-snprintf

This particular instance did not misuse `snprintf()`, but it was caught
by the audit anyway.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes openzfs#14098
  • Loading branch information
ryao authored and datacore-rm committed Aug 14, 2023
1 parent a3ca8b1 commit 834c5b2
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions cmd/zdb/zdb.c
Original file line number Diff line number Diff line change
Expand Up @@ -2824,9 +2824,11 @@ dump_bookmarks(objset_t *os, int verbosity)
zap_cursor_advance(&zc)) {
char osname[ZFS_MAX_DATASET_NAME_LEN];
char buf[ZFS_MAX_DATASET_NAME_LEN];
int len;
dmu_objset_name(os, osname);
VERIFY3S(0, <=, snprintf(buf, sizeof (buf), "%s#%s", osname,
attr.za_name));
len = snprintf(buf, sizeof (buf), "%s#%s", osname,
attr.za_name);
VERIFY3S(len, <, ZFS_MAX_DATASET_NAME_LEN);
(void) dump_bookmark(dp, buf, verbosity >= 5, verbosity >= 6);
}
zap_cursor_fini(&zc);
Expand Down

0 comments on commit 834c5b2

Please sign in to comment.