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

Re: [PATCH 6/7] trailer: handle NULL value when parsing trailer-specific config

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 7, 2023, 08:14 UTC
Message-ID
<ZXF-_bS6upwHIWMh@tanuki>
In-Reply-To
<20231207071132.GF1276005@coredump.intra.peff.net>
On Thu, Dec 07, 2023 at 02:11:32AM -0500, Jeff King wrote:
Show 47 quoted lines
> When parsing the "key", "command", and "cmd" trailer config, we just
> make a copy of the value string.  If we see an implicit bool like:
> 
>   [trailer "foo"]
>   key
> 
> we'll segfault trying to copy a NULL pointer. We can fix this with the
> usual config_error_nonbool() check.
> 
> I split this out from the other vanilla cases, because at first glance
> it looks like a better fix here would be to move the NULL check out of
> the switch statement. But it would change the behavior of other keys
> like trailer.*.ifExists, where an implicit bool is interpreted as
> EXISTS_DEFAULT.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  trailer.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/trailer.c b/trailer.c
> index b0e2ec224a..e4b08ed267 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -553,16 +553,22 @@ static int git_trailer_config(const char *conf_key, const char *value,
>  	case TRAILER_KEY:
>  		if (conf->key)
>  			warning(_("more than one %s"), conf_key);
> +		if (!value)
> +			return config_error_nonbool(conf_key);
>  		conf->key = xstrdup(value);
>  		break;
>  	case TRAILER_COMMAND:
>  		if (conf->command)
>  			warning(_("more than one %s"), conf_key);
> +		if (!value)
> +			return config_error_nonbool(conf_key);
>  		conf->command = xstrdup(value);
>  		break;
>  	case TRAILER_CMD:
>  		if (conf->cmd)
>  			warning(_("more than one %s"), conf_key);
> +		if (!value)
> +			return config_error_nonbool(conf_key);
>  		conf->cmd = xstrdup(value);
>  		break;
>  	case TRAILER_WHERE:

For the other cases we only generate warnings for unknown values, but return successfully. Should we do the same here?

Patrick
Previous: Jeff KingNext: Jeff King
Message 12 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.