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

Re: [PATCH] refspec: initalize `refspec_item` in `valid_fetch_refspec()`

From
BWBrandon Williams <bmwill@google.com>
Date
Jun 4, 2018, 17:36 UTC
Message-ID
<20180604173620.GB91449@google.com>
In-Reply-To
<20180604144305.29909-1-martin.agren@gmail.com>
On 06/04, Martin Ågren wrote:
Show 18 quoted lines
> We allocate a `struct refspec_item` on the stack without initializing
> it. In particular, its `dst` and `src` members will contain some random
> data from the stack. When we later call `refspec_item_clear()`, it will
> call `free()` on those pointers. So if the call to `parse_refspec()` did
> not assign to them, we will be freeing some random "pointers". This is
> undefined behavior.
> 
> To the best of my understanding, this cannot currently be triggered by
> user-provided data. And for what it's worth, the test-suite does not
> trigger this with SANITIZE=address. It can be provoked by calling
> `valid_fetch_refspec(":*")`.
> 
> Zero the struct, as is done in other users of `struct refspec_item`.
> 
> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> ---
> I found some time to look into this. It does not seem to be a
> user-visible bug, so not particularly critical.

Thanks for fixing this. I don't think I noticed this because at some point in developing this series I had a memset call in parse_refspec. I don't remember why I ended up removing it, but maybe it would have been better to leave it in there.

Show 23 quoted lines
> 
>  refspec.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/refspec.c b/refspec.c
> index ada7854f7a..7dd7e361e5 100644
> --- a/refspec.c
> +++ b/refspec.c
> @@ -189,7 +189,10 @@ void refspec_clear(struct refspec *rs)
>  int valid_fetch_refspec(const char *fetch_refspec_str)
>  {
>  	struct refspec_item refspec;
> -	int ret = parse_refspec(&refspec, fetch_refspec_str, REFSPEC_FETCH);
> +	int ret;
> +
> +	memset(&refspec, 0, sizeof(refspec));
> +	ret = parse_refspec(&refspec, fetch_refspec_str, REFSPEC_FETCH);
>  	refspec_item_clear(&refspec);
>  	return ret;
>  }
> -- 
> 2.18.0.rc0.43.gb85e7bcbff
> 
-- 
Brandon Williams
Previous: Martin ÅgrenNext: Ævar Arnfjörð Bjarmason
Message 61 of 112 in “refactoring refspecs”
  1. 00/35 refactoring refspecsBrandon Williams, May 14, 2018
  2. 01/35 refspec: move refspec parsing logic into its own fileBrandon Williams, May 14, 2018
  3. Junio C HamanoMay 15, 2018
  4. Brandon WilliamsMay 15, 2018
  5. Junio C HamanoMay 16, 2018
  6. 02/35 refspec: factor out parsing a single refspecBrandon Williams, May 14, 2018
  7. 03/35 refspec: rename struct refspec to struct refspec_itemBrandon Williams, May 14, 2018
  8. Junio C HamanoMay 15, 2018
  9. Brandon WilliamsMay 15, 2018
  10. 04/35 refspec: introduce struct refspecBrandon Williams, May 14, 2018
  11. Junio C HamanoMay 15, 2018
  12. Brandon WilliamsMay 15, 2018
  13. 06/35 submodule--helper: convert push_check to use struct refspecBrandon Williams, May 14, 2018
  14. 07/35 pull: convert get_tracking_branch to use refspec_item_initBrandon Williams, May 14, 2018
  15. 05/35 refspec: convert valid_fetch_refspec to use parse_refspecBrandon Williams, May 14, 2018
  16. Junio C HamanoMay 15, 2018
  17. 08/35 transport: convert transport_push to use struct refspecBrandon Williams, May 14, 2018
  18. 09/35 remote: convert check_push_refs to use struct refspecBrandon Williams, May 14, 2018
  19. 14/35 remote: convert fetch refspecs to struct refspecBrandon Williams, May 14, 2018
  20. Ævar Arnfjörð BjarmasonMay 15, 2018
  21. Brandon WilliamsMay 15, 2018
  22. 15/35 transport-helper: convert to use struct refspecBrandon Williams, May 14, 2018
  23. 17/35 fetch: convert refmap to use struct refspecBrandon Williams, May 14, 2018
  24. 18/35 refspec: remove the deprecated functionsBrandon Williams, May 14, 2018
  25. 19/35 fetch: convert do_fetch to take a struct refspecBrandon Williams, May 14, 2018
  26. 22/35 remote: convert get_stale_heads to take a struct refspecBrandon Williams, May 14, 2018
  27. 24/35 remote: convert query_refspecs to take a struct refspecBrandon Williams, May 14, 2018
  28. 25/35 remote: convert get_ref_match to take a struct refspecBrandon Williams, May 14, 2018
  29. 26/35 remote: convert match_explicit_refs to take a struct refspecBrandon Williams, May 14, 2018
  30. 27/35 push: check for errors earlierBrandon Williams, May 14, 2018
  31. 32/35 http-push: store refspecs in a struct refspecBrandon Williams, May 14, 2018
  32. 33/35 remote: convert match_push_refs to take a struct refspecBrandon Williams, May 14, 2018
  33. 34/35 remote: convert check_push_refs to take a struct refspecBrandon Williams, May 14, 2018
  34. 35/35 submodule: convert push_unpushed_submodules to take a struct refspecBrandon Williams, May 14, 2018
  35. Ævar Arnfjörð BjarmasonMay 15, 2018
  36. Stefan BellerMay 15, 2018
  37. Brandon WilliamsMay 15, 2018
  38. 31/35 transport: remove transport_verify_remote_namesBrandon Williams, May 14, 2018
  39. 28/35 push: convert to use struct refspecBrandon Williams, May 14, 2018
  40. 30/35 send-pack: store refspecs in a struct refspecBrandon Williams, May 14, 2018
  41. 29/35 transport: convert transport_push to take a struct refspecBrandon Williams, May 14, 2018
  42. 23/35 remote: convert apply_refspecs to take a struct refspecBrandon Williams, May 14, 2018
  43. 21/35 fetch: convert prune_refs to take a struct refspecBrandon Williams, May 14, 2018
  44. 20/35 fetch: convert get_ref_map to take a struct refspecBrandon Williams, May 14, 2018
  45. 16/35 fetch: convert fetch_one to use struct refspecBrandon Williams, May 14, 2018
  46. 13/35 remote: convert push refspecs to struct refspecBrandon Williams, May 14, 2018
  47. 12/35 fast-export: convert to use struct refspecBrandon Williams, May 14, 2018
  48. 10/35 remote: convert match_push_refs to use struct refspecBrandon Williams, May 14, 2018
  49. 11/35 clone: convert cmd_clone to use refspec_item_initBrandon Williams, May 14, 2018
  50. Stefan BellerMay 14, 2018
  51. Junio C HamanoMay 15, 2018
  52. Ævar Arnfjörð BjarmasonMay 15, 2018
  53. Brandon WilliamsMay 15, 2018
  54. 00/36 refactoring refspecsBrandon Williams, May 16, 2018
  55. 01/36 refspec: move refspec parsing logic into its own fileBrandon Williams, May 16, 2018
  56. 02/36 refspec: rename struct refspec to struct refspec_itemBrandon Williams, May 16, 2018
  57. 03/36 refspec: factor out parsing a single refspecBrandon Williams, May 16, 2018
  58. 05/36 refspec: convert valid_fetch_refspec to use parse_refspecBrandon Williams, May 16, 2018
  59. Martin ÅgrenJun 3, 2018
  60. refspec: initalize `refspec_item` in `valid_fetch_refspec()`Martin Ågren, Jun 4, 2018
  61. Brandon WilliamsJun 4, 2018
  62. Ævar Arnfjörð BjarmasonJun 4, 2018
  63. Martin ÅgrenJun 5, 2018
  64. Brandon WilliamsJun 5, 2018
  65. 0/3 refspec: refactor & fix free() behaviorÆvar Arnfjörð Bjarmason, Jun 5, 2018
  66. 2/3 refspec: add back a refspec_item_init() functionÆvar Arnfjörð Bjarmason, Jun 5, 2018
  67. 1/3 refspec: s/refspec_item_init/&_or_die/gÆvar Arnfjörð Bjarmason, Jun 5, 2018
  68. 3/3 refspec: initalize `refspec_item` in `valid_fetch_refspec()`Ævar Arnfjörð Bjarmason, Jun 5, 2018
  69. Brandon WilliamsJun 5, 2018
  70. Martin ÅgrenJun 5, 2018
  71. 04/36 refspec: introduce struct refspecBrandon Williams, May 16, 2018
  72. 06/36 submodule--helper: convert push_check to use struct refspecBrandon Williams, May 16, 2018
  73. 07/36 pull: convert get_tracking_branch to use refspec_item_initBrandon Williams, May 16, 2018
  74. 10/36 remote: convert match_push_refs to use struct refspecBrandon Williams, May 16, 2018
  75. 11/36 clone: convert cmd_clone to use refspec_item_initBrandon Williams, May 16, 2018
  76. 12/36 fast-export: convert to use struct refspecBrandon Williams, May 16, 2018
  77. 09/36 remote: convert check_push_refs to use struct refspecBrandon Williams, May 16, 2018
  78. 14/36 remote: convert fetch refspecs to struct refspecBrandon Williams, May 16, 2018
  79. 13/36 remote: convert push refspecs to struct refspecBrandon Williams, May 16, 2018
  80. 16/36 transport-helper: convert to use struct refspecBrandon Williams, May 16, 2018
  81. 18/36 fetch: convert refmap to use struct refspecBrandon Williams, May 16, 2018
  82. 19/36 refspec: remove the deprecated functionsBrandon Williams, May 16, 2018
  83. 20/36 fetch: convert do_fetch to take a struct refspecBrandon Williams, May 16, 2018
  84. 21/36 fetch: convert get_ref_map to take a struct refspecBrandon Williams, May 16, 2018
  85. 15/36 remote: remove add_prune_tags_to_fetch_refspecBrandon Williams, May 16, 2018
  86. 24/36 remote: convert apply_refspecs to take a struct refspecBrandon Williams, May 16, 2018
  87. 26/36 remote: convert get_ref_match to take a struct refspecBrandon Williams, May 16, 2018
  88. 27/36 remote: convert match_explicit_refs to take a struct refspecBrandon Williams, May 16, 2018
  89. 17/36 fetch: convert fetch_one to use struct refspecBrandon Williams, May 16, 2018
  90. 25/36 remote: convert query_refspecs to take a struct refspecBrandon Williams, May 16, 2018
  91. 28/36 push: check for errors earlierBrandon Williams, May 16, 2018
  92. 32/36 transport: remove transport_verify_remote_namesBrandon Williams, May 16, 2018
  93. 34/36 remote: convert match_push_refs to take a struct refspecBrandon Williams, May 16, 2018
  94. 35/36 remote: convert check_push_refs to take a struct refspecBrandon Williams, May 16, 2018
  95. 33/36 http-push: store refspecs in a struct refspecBrandon Williams, May 16, 2018
  96. 36/36 submodule: convert push_unpushed_submodules to take a struct refspecBrandon Williams, May 16, 2018
  97. 30/36 transport: convert transport_push to take a struct refspecBrandon Williams, May 16, 2018
  98. 31/36 send-pack: store refspecs in a struct refspecBrandon Williams, May 16, 2018
  99. 29/36 push: convert to use struct refspecBrandon Williams, May 16, 2018
  100. 22/36 fetch: convert prune_refs to take a struct refspecBrandon Williams, May 16, 2018
  101. 23/36 remote: convert get_stale_heads to take a struct refspecBrandon Williams, May 16, 2018
  102. 08/36 transport: convert transport_push to use struct refspecBrandon Williams, May 16, 2018
  103. 0/2 generating ref-prefixes for configured refspecsBrandon Williams, May 16, 2018
  104. 1/2 refspec: consolidate ref-prefix generation logicBrandon Williams, May 16, 2018
  105. Jonathan NiederMay 31, 2018
  106. Jonathan NiederMay 31, 2018
  107. fetch: do not pass ref-prefixes for fetch by exact SHA1Jonathan Nieder, May 31, 2018
  108. Brandon WilliamsMay 31, 2018
  109. Junio C HamanoJun 1, 2018
  110. Jonathan NiederJun 1, 2018
  111. 2/2 fetch: generate ref-prefixes when using a configured refspecBrandon Williams, May 16, 2018
  112. Junio C HamanoMay 17, 2018

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.