From 850adcf5ef0338aa7c3e3222fc39f0aa5a68a6bb 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 f200b928bc6d..3f11445f74c8 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;