[PATCH v2] trace2: prevent segfault on config collection where no value specified
- From
- Adam Murray via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Jan 10, 2025, 07:28 UTC
- Message-ID
- <pull.1814.v2.git.1736494100622.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1814.git.1730937889182.gitgitgadget@gmail.com>
From: Adam Murray <ad@canva.com>
When TRACE2 analytics is enabled, a git config option that has no value causes a segfault.
Steps to Reproduce GIT_TRACE2=true GIT_TRACE2_CONFIG_PARAMS=status.* git -c status.relativePaths version Expected Result git version 2.46.0 Actual Result zsh: segmentation fault GIT_TRACE2=true
This adds checks to prevent the segfault and instead return an empty value.
Signed-off-by: Adam Murray <ad@canva.com>
---
trace2: prevent segfault on config collection where no value specified
cc: Jeff King peff@peff.netPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1814%2Fad-murray%2Ffix-trace2-segfault-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1814/ad-murray/fix-trace2-segfault-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1814
Range-diff vs v1:
1: 24ba9db7aa1 ! 1: fd7bed52dda trace2: prevent segfault on config collection where no value specified
@@ Commit message
Actual Result
zsh: segmentation fault GIT_TRACE2=true
- This adds a null check to prevent the segfault and instead return
- the "empty config value" error.
+ This adds checks to prevent the segfault and instead return
+ an empty value.
Signed-off-by: Adam Murray <ad@canva.com>
@@ t/t0210-trace2-normal.sh: test_expect_success 'bug messages followed by BUG() ar
## trace2.c ##
@@ trace2.c: void trace2_def_param_fl(const char *file, int line, const char *param,
- int j;
- const char *redacted;
-
-- if (!trace2_enabled)
-+ if (!trace2_enabled || !value)
+ if (!trace2_enabled)
return;
- redacted = redact_arg(value);
+- redacted = redact_arg(value);
++ redacted = value ? redact_arg(value): NULL;
+
+ for_each_wanted_builtin (j, tgt_j)
+ if (tgt_j->pfn_param_fl)
+
+ ## trace2/tr2_tgt_event.c ##
+@@ trace2/tr2_tgt_event.c: static void fn_param_fl(const char *file, int line, const char *param,
+ event_fmt_prepare(event_name, file, line, NULL, &jw);
+ jw_object_string(&jw, "scope", scope_name);
+ jw_object_string(&jw, "param", param);
+- jw_object_string(&jw, "value", value);
++ if (value)
++ jw_object_string(&jw, "value", value);
+ jw_end(&jw);
+
+ tr2_dst_write_line(&tr2dst_event, &jw.json);
+
+ ## trace2/tr2_tgt_normal.c ##
+@@ trace2/tr2_tgt_normal.c: static void fn_param_fl(const char *file, int line, const char *param,
+ enum config_scope scope = kvi->scope;
+ const char *scope_name = config_scope_name(scope);
+
+- strbuf_addf(&buf_payload, "def_param scope:%s %s=%s", scope_name, param,
+- value);
++ strbuf_addf(&buf_payload, "def_param scope:%s %s", scope_name, param);
++ if (value)
++ strbuf_addf(&buf_payload, "=%s", value);
+ normal_io_write_fl(file, line, &buf_payload);
+ strbuf_release(&buf_payload);
+ }
+
+ ## trace2/tr2_tgt_perf.c ##
+@@ trace2/tr2_tgt_perf.c: static void fn_param_fl(const char *file, int line, const char *param,
+ struct strbuf scope_payload = STRBUF_INIT;
+ enum config_scope scope = kvi->scope;
+ const char *scope_name = config_scope_name(scope);
+-
+- strbuf_addf(&buf_payload, "%s:%s", param, value);
++ strbuf_addstr(&buf_payload, param);
++ if (value)
++ strbuf_addf(&buf_payload, ":%s", value);
+ strbuf_addf(&scope_payload, "%s:%s", "scope", scope_name);
+
+ perf_io_write_fl(file, line, event_name, NULL, NULL, NULL,t/t0210-trace2-normal.sh | 8 ++++++++ trace2.c | 2 +- trace2/tr2_tgt_event.c | 3 ++- trace2/tr2_tgt_normal.c | 5 +++-- trace2/tr2_tgt_perf.c | 5 +++-- 5 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/t/t0210-trace2-normal.sh b/t/t0210-trace2-normal.sh index b9adc94aab4..4047ab562a4 100755 --- a/t/t0210-trace2-normal.sh +++ b/t/t0210-trace2-normal.sh @@ -244,6 +244,14 @@ test_expect_success 'bug messages followed by BUG() are written to trace2' ' test_cmp expect actual ' +test_expect_success 'empty configuration values are handled' ' + test_when_finished "rm trace2.normal actual expect" && + echo >expect && + GIT_TRACE2="$(pwd)/trace2.normal" GIT_TRACE2_CONFIG_PARAMS=foo.empty \ + git -c foo.empty config foo.empty >actual && + test_cmp expect actual +' + sane_unset GIT_TRACE2_BRIEF # Now test without environment variables and get all Trace2 settings diff --git a/trace2.c b/trace2.c index f894532d053..49e7d1db88f 100644 --- a/trace2.c +++ b/trace2.c @@ -762,7 +762,7 @@ void trace2_def_param_fl(const char *file, int line, const char *param, if (!trace2_enabled) return; - redacted = redact_arg(value); + redacted = value ? redact_arg(value): NULL; for_each_wanted_builtin (j, tgt_j) if (tgt_j->pfn_param_fl) diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index 45b0850a5ec..8e09485c83c 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -491,7 +491,8 @@ static void fn_param_fl(const char *file, int line, const char *param, event_fmt_prepare(event_name, file, line, NULL, &jw); jw_object_string(&jw, "scope", scope_name); jw_object_string(&jw, "param", param); - jw_object_string(&jw, "value", value); + if (value) + jw_object_string(&jw, "value", value); jw_end(&jw); tr2_dst_write_line(&tr2dst_event, &jw.json); diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c index baef48aa698..924736ab360 100644 --- a/trace2/tr2_tgt_normal.c +++ b/trace2/tr2_tgt_normal.c @@ -307,8 +307,9 @@ static void fn_param_fl(const char *file, int line, const char *param, enum config_scope scope = kvi->scope; const char *scope_name = config_scope_name(scope); - strbuf_addf(&buf_payload, "def_param scope:%s %s=%s", scope_name, param, - value); + strbuf_addf(&buf_payload, "def_param scope:%s %s", scope_name, param); + if (value) + strbuf_addf(&buf_payload, "=%s", value); normal_io_write_fl(file, line, &buf_payload); strbuf_release(&buf_payload); } diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c index a6f9a8a193e..19ae7433ef8 100644 --- a/trace2/tr2_tgt_perf.c +++ b/trace2/tr2_tgt_perf.c @@ -446,8 +446,9 @@ static void fn_param_fl(const char *file, int line, const char *param, struct strbuf scope_payload = STRBUF_INIT; enum config_scope scope = kvi->scope; const char *scope_name = config_scope_name(scope); - - strbuf_addf(&buf_payload, "%s:%s", param, value); + strbuf_addstr(&buf_payload, param); + if (value) + strbuf_addf(&buf_payload, ":%s", value); strbuf_addf(&scope_payload, "%s:%s", "scope", scope_name); perf_io_write_fl(file, line, event_name, NULL, NULL, NULL, base-commit: 8f8d6eee531b3fa1a8ef14f169b0cb5035f7a772
-- gitgitgadget