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

[PATCH v3 0/3] negative-refspec: fix segfault on : refspec

From
Nipunn Koorapati via GitGitGadget <gitgitgadget@gmail.com>
Date
Dec 21, 2020, 02:05 UTC
Message-ID
<pull.820.v3.git.1608516320.gitgitgadget@gmail.com>
In-Reply-To
<pull.820.v2.git.1608415117.gitgitgadget@gmail.com>

If remote.origin.push was set to ":", git segfaults during a push operation, due to bad parsing logic in query_matches_negative_refspec. Per bisect, the bug was introduced in: c0192df630 (refspec: add support for negative refspecs, 2020-09-30)

We found this issue when rolling out git 2.29 at Dropbox - as several folks had "push = :" in their configuration. I based my diff off the master branch, but also confirmed that it patches cleanly onto maint - if the maintainers would like to also fix the segfault on 2.29

Update since Patch series V1:
 * Handled matching refspec explicitly
 * Added testing for "+:" case
 * Added comment explaining how the two loops work together
Update since Patch series V2
 * style suggestion in remote.c
 * Use test_config
 * Add test for a case with a matching refspec + negative refspec
 * Fix test_config to work with --add
 * Updated commit message to describe what git is told to do instead of
   segfaulting
Nipunn Koorapati (3):
  test-lib-functions: handle --add in test_config
  negative-refspec: fix segfault on : refspec
  negative-refspec: improve comment on query_matches_negative_refspec
 remote.c                          | 16 +++++++++++++---
 t/t5582-fetch-negative-refspec.sh | 22 ++++++++++++++++++++++
 t/test-lib-functions.sh           |  9 ++++++++-
 3 files changed, 43 insertions(+), 4 deletions(-)
base-commit: 6d3ef5b467eccd2769f1aa1c555d317d3c8dc707
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-820%2Fnipunn1313%2Fnk%2Fpush-refspec-segfault-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-820/nipunn1313/nk/push-refspec-segfault-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/820
Range-diff vs v2:
 -:  ----------- > 1:  733c674bd19 test-lib-functions: handle --add in test_config
 1:  e42200b644a ! 2:  20cff2f5c59 negative-refspec: fix segfault on : refspec
     @@ Commit message
          remote.origin.push is set to ":", then refspec->src is NULL,
          causing a segfault within strcmp
      
     -    Added testing for this case in fetch-negative-refspec
     +    Tell git to handle matching refspec by adding the needle to the
     +    set of positively matched refspecs, since matching ":" refspecs
     +    match anything as src.
     +
     +    Added testing for matching refspec pushes fetch-negative-refspec
     +    both individually and in combination with a negative refspec
      
          Signed-off-by: Nipunn Koorapati <nipunn@dropbox.com>
      
     @@ remote.c: static int query_matches_negative_refspec(struct refspec *rs, struct r
      +		} else if (refspec->matching) {
      +			/* For the special matching refspec, any query should match */
      +			string_list_append(&reversed, needle);
     -+		} else if (refspec->src == NULL) {
     ++		} else if (!refspec->src) {
      +			BUG("refspec->src should not be null here");
      +		} else if (!strcmp(needle, refspec->src)) {
      +			string_list_append(&reversed, refspec->src);
     @@ t/t5582-fetch-negative-refspec.sh: test_expect_success "fetch --prune with negat
       '
       
      +test_expect_success "push with matching ':' refspec" '
     -+	(
     -+		cd two &&
     -+		git config remote.one.push : &&
     -+		# Fails w/ tip behind counterpart - but should not segfault
     -+		test_must_fail git push one master &&
     ++	test_config -C two remote.one.push : &&
     ++	# Fails w/ tip behind counterpart - but should not segfault
     ++	test_must_fail git -C two push one
     ++'
     ++
     ++test_expect_success "push with matching '+:' refspec" '
     ++	test_config -C two remote.one.push +: &&
     ++	# Fails w/ tip behind counterpart - but should not segfault
     ++	test_must_fail git -C two push one
     ++'
      +
     -+		git config remote.one.push +: &&
     -+		# Fails w/ tip behind counterpart - but should not segfault
     -+		test_must_fail git push one master &&
     ++test_expect_success "push with matching and negative refspec" '
     ++	test_config -C two --add remote.one.push : &&
     ++	# Fails to push master w/ tip behind counterpart
     ++	test_must_fail git -C two push one &&
      +
     -+		git config --unset remote.one.push
     -+	)
     ++	# If master is in negative refspec, then the command will succeed
     ++	test_config -C two --add remote.one.push ^refs/heads/master &&
     ++	git -C two push one
      +'
      +
       test_done
 2:  8da8d9cd1c5 = 3:  0fd4e9f7459 negative-refspec: improve comment on query_matches_negative_refspec
-- 
gitgitgadget
Previous: Nipunn Koorapati via GitGitGadgetNext: Nipunn Koorapati via GitGitGadget
Message 8 of 33 in “negative-refspec: fix segfault on : refspec”
  1. negative-refspec: fix segfault on : refspecNipunn Koorapati via GitGitGadget, Dec 19, 2020
  2. Junio C HamanoDec 19, 2020
  3. Jacob KellerFeb 19, 2021
  4. 0/2 negative-refspec: fix segfault on : refspecNipunn Koorapati via GitGitGadget, Dec 19, 2020
  5. 1/2 negative-refspec: fix segfault on : refspecNipunn Koorapati via GitGitGadget, Dec 19, 2020
  6. Eric SunshineDec 20, 2020
  7. 2/2 negative-refspec: improve comment on query_matches_negative_refspecNipunn Koorapati via GitGitGadget, Dec 19, 2020
  8. 0/3 negative-refspec: fix segfault on : refspecNipunn Koorapati via GitGitGadget, Dec 21, 2020
  9. 3/3 negative-refspec: improve comment on query_matches_negative_refspecNipunn Koorapati via GitGitGadget, Dec 21, 2020
  10. 2/3 negative-refspec: fix segfault on : refspecNipunn Koorapati via GitGitGadget, Dec 21, 2020
  11. Eric SunshineDec 21, 2020
  12. 1/3 test-lib-functions: handle --add in test_configNipunn Koorapati via GitGitGadget, Dec 21, 2020
  13. Eric SunshineDec 21, 2020
  14. Junio C HamanoDec 21, 2020
  15. Eric SunshineDec 21, 2020
  16. Nipunn KoorapatiDec 22, 2020
  17. Eric SunshineDec 22, 2020
  18. Nipunn KoorapatiDec 22, 2020
  19. Eric SunshineDec 22, 2020
  20. 0/2 negative-refspec: fix segfault on : refspecNipunn Koorapati via GitGitGadget, Dec 22, 2020
  21. 2/2 negative-refspec: improve comment on query_matches_negative_refspecNipunn Koorapati via GitGitGadget, Dec 22, 2020
  22. 1/2 negative-refspec: fix segfault on : refspecNipunn Koorapati via GitGitGadget, Dec 22, 2020
  23. Junio C HamanoDec 22, 2020
  24. Junio C HamanoDec 22, 2020
  25. 0/2 negative-refspec: fix segfault on : refspecNipunn Koorapati via GitGitGadget, Dec 22, 2020
  26. 2/2 negative-refspec: improve comment on query_matches_negative_refspecNipunn Koorapati via GitGitGadget, Dec 22, 2020
  27. 1/2 negative-refspec: fix segfault on : refspecNipunn Koorapati via GitGitGadget, Dec 22, 2020
  28. Jacob KellerFeb 19, 2021
  29. Junio C HamanoDec 22, 2020
  30. Nipunn KoorapatiDec 23, 2020
  31. Junio C HamanoDec 24, 2020
  32. Nipunn KoorapatiJan 11, 2021
  33. Junio C HamanoJan 12, 2021

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.