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

[PATCH 7/7] fsck: handle NULL value when parsing message config

From
Jeff King <peff@peff.net>
Date
Dec 7, 2023, 07:11 UTC
Message-ID
<20231207071135.GG1276005@coredump.intra.peff.net>
In-Reply-To
<20231207071030.GA1275835@coredump.intra.peff.net>

When parsing fsck.*, receive.fsck.*, or fetch.fsck.*, we don't check for an implicit bool. So any of:

  [fsck]
  badTree
  [receive "fsck"]
  badTree
  [fetch "fsck"]
  badTree

will cause us to segfault. We can fix it with config_error_nonbool() in the usual way, but we have to make a few more changes to get good error messages. The problem is that all three spots do:

  if (skip_prefix(var, "fsck.", &var))

to match and parse the actual message id. But that means that "var" now just says "badTree" instead of "receive.fsck.badTree", making the resulting message confusing. We can fix that by storing the parsed message id in its own separate variable.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/receive-pack.c | 11 +++++++----
 fetch-pack.c           | 12 ++++++++----
 fsck.c                 |  8 ++++++--
 3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 8c4f0cb90a..ccf9738bce 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -142,6 +142,7 @@ static enum deny_action parse_deny_action(const char *var, const char *value)
 static int receive_pack_config(const char *var, const char *value,
 			       const struct config_context *ctx, void *cb)
 {
+	const char *msg_id;
 	int status = parse_hide_refs_config(var, value, "receive", &hidden_refs);
 
 	if (status)
@@ -178,12 +179,14 @@ static int receive_pack_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (skip_prefix(var, "receive.fsck.", &var)) {
-		if (is_valid_msg_type(var, value))
+	if (skip_prefix(var, "receive.fsck.", &msg_id)) {
+		if (!value)
+			return config_error_nonbool(var);
+		if (is_valid_msg_type(msg_id, value))
 			strbuf_addf(&fsck_msg_types, "%c%s=%s",
-				fsck_msg_types.len ? ',' : '=', var, value);
+				fsck_msg_types.len ? ',' : '=', msg_id, value);
 		else
-			warning("skipping unknown msg id '%s'", var);
+			warning("skipping unknown msg id '%s'", msg_id);
 		return 0;
 	}
 
diff --git a/fetch-pack.c b/fetch-pack.c
index 26999e3b65..31a72d43de 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1862,6 +1862,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 static int fetch_pack_config_cb(const char *var, const char *value,
 				const struct config_context *ctx, void *cb)
 {
+	const char *msg_id;
+
 	if (strcmp(var, "fetch.fsck.skiplist") == 0) {
 		const char *path;
 
@@ -1873,12 +1875,14 @@ static int fetch_pack_config_cb(const char *var, const char *value,
 		return 0;
 	}
 
-	if (skip_prefix(var, "fetch.fsck.", &var)) {
-		if (is_valid_msg_type(var, value))
+	if (skip_prefix(var, "fetch.fsck.", &msg_id)) {
+		if (!value)
+			return config_error_nonbool(var);
+		if (is_valid_msg_type(msg_id, value))
 			strbuf_addf(&fsck_msg_types, "%c%s=%s",
-				fsck_msg_types.len ? ',' : '=', var, value);
+				fsck_msg_types.len ? ',' : '=', msg_id, value);
 		else
-			warning("Skipping unknown msg id '%s'", var);
+			warning("Skipping unknown msg id '%s'", msg_id);
 		return 0;
 	}
 
diff --git a/fsck.c b/fsck.c
index 6a0bbc5087..b624083a13 100644
--- a/fsck.c
+++ b/fsck.c
@@ -1403,6 +1403,8 @@ int git_fsck_config(const char *var, const char *value,
 		    const struct config_context *ctx, void *cb)
 {
 	struct fsck_options *options = cb;
+	const char *msg_id;
+
 	if (strcmp(var, "fsck.skiplist") == 0) {
 		const char *path;
 		struct strbuf sb = STRBUF_INIT;
@@ -1416,8 +1418,10 @@ int git_fsck_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (skip_prefix(var, "fsck.", &var)) {
-		fsck_set_msg_type(options, var, value);
+	if (skip_prefix(var, "fsck.", &msg_id)) {
+		if (!value)
+			return config_error_nonbool(var);
+		fsck_set_msg_type(options, msg_id, value);
 		return 0;
 	}
 
-- 
2.43.0.664.ga12c899002
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 13 of 17 in “fix segfaults with implicit-bool config”
  1. 0/7 fix segfaults with implicit-bool configJeff King, Dec 7, 2023
  2. 1/7 config: handle NULL value when parsing non-boolsJeff King, Dec 7, 2023
  3. Patrick SteinhardtDec 7, 2023
  4. Jeff KingDec 12, 2023
  5. 2/7 setup: handle NULL value when parsing extensionsJeff King, Dec 7, 2023
  6. 3/7 trace2: handle NULL values in tr2_sysenv config callbackJeff King, Dec 7, 2023
  7. 4/7 help: handle NULL value for alias.* configJeff King, Dec 7, 2023
  8. 5/7 submodule: handle NULL value when parsing submodule.*.branchJeff King, Dec 7, 2023
  9. Patrick SteinhardtDec 7, 2023
  10. Jeff KingDec 12, 2023
  11. 6/7 trailer: handle NULL value when parsing trailer-specific configJeff King, Dec 7, 2023
  12. Patrick SteinhardtDec 7, 2023
  13. 7/7 fsck: handle NULL value when parsing message configJeff King, Dec 7, 2023
  14. Patrick SteinhardtDec 7, 2023
  15. Patrick SteinhardtDec 7, 2023
  16. Jeff KingDec 12, 2023
  17. Patrick SteinhardtDec 12, 2023

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.