From d71d69326116756e69b2d7bee4582f00de27ec72 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Thu, 27 Oct 2022 12:45:26 -0400 Subject: [PATCH] Fix too few arguments to formatting function 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 Signed-off-by: Richard Yao Closes #14098 --- module/zfs/zcp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/module/zfs/zcp.c b/module/zfs/zcp.c index 2b79f0a2a0..5ebf1bbbc8 100644 --- a/module/zfs/zcp.c +++ b/module/zfs/zcp.c @@ -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;