Skip to content

Commit

Permalink
Fix too few arguments to formatting function
Browse files Browse the repository at this point in the history
CodeQL reported that when the VERIFY3U condition is false, we do not
pass enough arguments to `spl_panic()`. This is because the format
string from `snprintf()` was concatenated into the format string for
`spl_panic()`, which causes us to have an unexpected format specifier.

A CodeQL developer suggested fixing the macro to have a `%s` format
string that takes a stringified RIGHT argument, which would fix this.
However, upon inspection, the VERIFY3U check was never necessary in the
first place, so we remove it in favor of just calling `snprintf()`.

Lastly, it is interesting that every other static analyzer run on the
codebase did not catch this, including some that made an effort to catch
such things. Presumably, all of them relied on header annotations, which
we have not yet done on `spl_panic()`. CodeQL apparently is able to
track the flow of arguments on their way to annotated functions, which
llowed it to catch this when others did not. A future patch that I have
in development should annotate `spl_panic()`, so the others will catch
this too.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes openzfs#14098
  • Loading branch information
ryao authored and tonyhutter committed Nov 30, 2022
1 parent 663f6b1 commit 850adcf
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions module/zfs/zcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,9 @@ zcp_table_to_nvlist(lua_State *state, int index, int depth)
}
break;
case LUA_TNUMBER:
VERIFY3U(sizeof (buf), >,
snprintf(buf, sizeof (buf), "%lld",
(longlong_t)lua_tonumber(state, -2)));
(void) snprintf(buf, sizeof (buf), "%lld",
(longlong_t)lua_tonumber(state, -2));

key = buf;
if (saw_str_could_collide) {
key_could_collide = B_TRUE;
Expand Down

0 comments on commit 850adcf

Please sign in to comment.