git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] trace2: prevent segfault on config collection where no value specified

From
AGAdam 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.net
Published-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
Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 11 in “trace2: prevent segfault on config collection where no value specified”
  1. trace2: prevent segfault on config collection where no value specifiedAdam Murray via GitGitGadget, Nov 7, 2024
  2. Jeff KingNov 7, 2024
  3. Junio C HamanoNov 7, 2024
  4. trace2: prevent segfault on config collection where no value specifiedAdam Murray via GitGitGadget, Jan 10, 2025
  5. Junio C HamanoJan 10, 2025
  6. Johannes SchindelinJan 22, 2025
  7. Junio C HamanoJan 22, 2025
  8. Junio C HamanoJan 22, 2025
  9. D. Ben KnobleJan 23, 2025
  10. Junio C HamanoJan 23, 2025
  11. D. Ben KnobleJan 23, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.