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

[PATCH v3 04/21] strbuf: fix leak when `appendwholeline()` fails with EOF

From
Patrick Steinhardt <ps@pks.im>
Date
May 27, 2024, 11:46 UTC
Message-ID
<aa5cbd9d1404065e40474c2b55c0a8da9df308c8.1716810168.git.ps@pks.im>
In-Reply-To
<cover.1716810168.git.ps@pks.im>

In `strbuf_appendwholeline()` we call `strbuf_getwholeline()` with a temporary buffer. In case the call returns an error we indicate this by returning EOF, but never release the temporary buffer. This can cause a leak though because `strbuf_getwholeline()` calls getline(3). Quoting its documentation:

    If *lineptr was set to NULL before the call, then the buffer
    should be freed by the user program even on failure.

Consequently, the temporary buffer may hold allocated memory even when the call to `strbuf_getwholeline()` fails.

Fix this by releasing the temporary buffer on error.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 strbuf.c              | 4 +++-
 t/t1400-update-ref.sh | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/strbuf.c b/strbuf.c
index 0d929e4e19..e1076c9891 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -691,8 +691,10 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
 int strbuf_appendwholeline(struct strbuf *sb, FILE *fp, int term)
 {
 	struct strbuf line = STRBUF_INIT;
-	if (strbuf_getwholeline(&line, fp, term))
+	if (strbuf_getwholeline(&line, fp, term)) {
+		strbuf_release(&line);
 		return EOF;
+	}
 	strbuf_addbuf(sb, &line);
 	strbuf_release(&line);
 	return 0;
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index ec3443cc87..bbee2783ab 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -4,6 +4,8 @@
 #
 
 test_description='Test git update-ref and basic ref logging'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 Z=$ZERO_OID
-- 
2.45.1.246.gb9cfe4845c.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 81 of 115 in “Various memory leak fixes”
  1. 00/20 Various memory leak fixesPatrick Steinhardt, May 23, 2024
  2. 01/20 t: mark a bunch of tests as leak-freePatrick Steinhardt, May 23, 2024
  3. Junio C HamanoMay 23, 2024
  4. Patrick SteinhardtMay 24, 2024
  5. Junio C HamanoMay 24, 2024
  6. Junio C HamanoMay 24, 2024
  7. Karthik NayakMay 24, 2024
  8. 02/20 transport-helper: fix leaking helper namePatrick Steinhardt, May 23, 2024
  9. Junio C HamanoMay 23, 2024
  10. Karthik NayakMay 24, 2024
  11. 03/20 strbuf: fix leak when `appendwholeline()` fails with EOFPatrick Steinhardt, May 23, 2024
  12. 04/20 checkout: clarify memory ownership in `unique_tracking_name()`Patrick Steinhardt, May 23, 2024
  13. 05/20 http: refactor code to clarify memory ownershipPatrick Steinhardt, May 23, 2024
  14. 06/20 config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt, May 23, 2024
  15. 07/20 diff: refactor code to clarify memory ownership of prefixesPatrick Steinhardt, May 23, 2024
  16. Eric SunshineMay 23, 2024
  17. 08/20 convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt, May 23, 2024
  18. 09/20 builtin/log: stop using globals for log configPatrick Steinhardt, May 23, 2024
  19. 10/20 builtin/log: stop using globals for format configPatrick Steinhardt, May 23, 2024
  20. 11/20 config: clarify memory ownership in `git_config_string()`Patrick Steinhardt, May 23, 2024
  21. 12/20 config: plug various memory leaksPatrick Steinhardt, May 23, 2024
  22. Junio C HamanoMay 23, 2024
  23. Patrick SteinhardtMay 24, 2024
  24. Patrick SteinhardtMay 24, 2024
  25. Junio C HamanoMay 24, 2024
  26. Junio C HamanoMay 24, 2024
  27. 13/20 builtin/credential: clear credential before exitPatrick Steinhardt, May 23, 2024
  28. 14/20 commit-reach: fix memory leak in `ahead_behind()`Patrick Steinhardt, May 23, 2024
  29. 15/20 submodule: fix leaking memory for submodule entriesPatrick Steinhardt, May 23, 2024
  30. 16/20 strvec: add functions to replace and remove stringsPatrick Steinhardt, May 23, 2024
  31. Eric SunshineMay 23, 2024
  32. Patrick SteinhardtMay 24, 2024
  33. 17/20 builtin/mv: refactor `add_slash()` to always return allocated stringsPatrick Steinhardt, May 23, 2024
  34. 18/20 builtin/mv duplicate string list memoryPatrick Steinhardt, May 23, 2024
  35. 19/20 builtin/mv: refactor to use `struct strvec`Patrick Steinhardt, May 23, 2024
  36. 20/20 builtin/mv: fix leaks for submodule gitfile pathsPatrick Steinhardt, May 23, 2024
  37. Junio C HamanoMay 23, 2024
  38. Patrick SteinhardtMay 24, 2024
  39. 00/21 Various memory leak fixesPatrick Steinhardt, May 24, 2024
  40. 01/21 ci: add missing dependency for TTY prereqPatrick Steinhardt, May 24, 2024
  41. Junio C HamanoMay 24, 2024
  42. 02/21 t: mark a bunch of tests as leak-freePatrick Steinhardt, May 24, 2024
  43. 03/21 transport-helper: fix leaking helper namePatrick Steinhardt, May 24, 2024
  44. 04/21 strbuf: fix leak when `appendwholeline()` fails with EOFPatrick Steinhardt, May 24, 2024
  45. Jeff KingMay 25, 2024
  46. Patrick SteinhardtMay 27, 2024
  47. Jeff KingMay 29, 2024
  48. Patrick SteinhardtMay 29, 2024
  49. Jeff KingMay 30, 2024
  50. 05/21 checkout: clarify memory ownership in `unique_tracking_name()`Patrick Steinhardt, May 24, 2024
  51. 06/21 http: refactor code to clarify memory ownershipPatrick Steinhardt, May 24, 2024
  52. 07/21 config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt, May 24, 2024
  53. 08/21 diff: refactor code to clarify memory ownership of prefixesPatrick Steinhardt, May 24, 2024
  54. 09/21 convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt, May 24, 2024
  55. 10/21 builtin/log: stop using globals for log configPatrick Steinhardt, May 24, 2024
  56. 11/21 builtin/log: stop using globals for format configPatrick Steinhardt, May 24, 2024
  57. 12/21 config: clarify memory ownership in `git_config_string()`Patrick Steinhardt, May 24, 2024
  58. 13/21 config: plug various memory leaksPatrick Steinhardt, May 24, 2024
  59. Patrick SteinhardtMay 24, 2024
  60. Jeff KingMay 25, 2024
  61. Patrick SteinhardtMay 27, 2024
  62. Jeff KingMay 29, 2024
  63. 14/21 builtin/credential: clear credential before exitPatrick Steinhardt, May 24, 2024
  64. 15/21 commit-reach: fix memory leak in `ahead_behind()`Patrick Steinhardt, May 24, 2024
  65. 16/21 submodule: fix leaking memory for submodule entriesPatrick Steinhardt, May 24, 2024
  66. 17/21 strvec: add functions to replace and remove stringsPatrick Steinhardt, May 24, 2024
  67. 18/21 builtin/mv: refactor `add_slash()` to always return allocated stringsPatrick Steinhardt, May 24, 2024
  68. 19/21 builtin/mv duplicate string list memoryPatrick Steinhardt, May 24, 2024
  69. 20/21 builtin/mv: refactor to use `struct strvec`Patrick Steinhardt, May 24, 2024
  70. 21/21 builtin/mv: fix leaks for submodule gitfile pathsPatrick Steinhardt, May 24, 2024
  71. Junio C HamanoMay 25, 2024
  72. Patrick SteinhardtMay 27, 2024
  73. Junio C HamanoMay 27, 2024
  74. Junio C HamanoMay 27, 2024
  75. Patrick SteinhardtMay 28, 2024
  76. Karthik NayakMay 29, 2024
  77. 00/21 Various memory leak fixesPatrick Steinhardt, May 27, 2024
  78. 01/21 ci: add missing dependency for TTY prereqPatrick Steinhardt, May 27, 2024
  79. 02/21 t: mark a bunch of tests as leak-freePatrick Steinhardt, May 27, 2024
  80. 03/21 transport-helper: fix leaking helper namePatrick Steinhardt, May 27, 2024
  81. 04/21 strbuf: fix leak when `appendwholeline()` fails with EOFPatrick Steinhardt, May 27, 2024
  82. 05/21 checkout: clarify memory ownership in `unique_tracking_name()`Patrick Steinhardt, May 27, 2024
  83. 06/21 http: refactor code to clarify memory ownershipPatrick Steinhardt, May 27, 2024
  84. 07/21 config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt, May 27, 2024
  85. 08/21 diff: refactor code to clarify memory ownership of prefixesPatrick Steinhardt, May 27, 2024
  86. 09/21 convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt, May 27, 2024
  87. 10/21 builtin/log: stop using globals for log configPatrick Steinhardt, May 27, 2024
  88. 11/21 builtin/log: stop using globals for format configPatrick Steinhardt, May 27, 2024
  89. 12/21 config: clarify memory ownership in `git_config_string()`Patrick Steinhardt, May 27, 2024
  90. 13/21 config: plug various memory leaksPatrick Steinhardt, May 27, 2024
  91. 14/21 builtin/credential: clear credential before exitPatrick Steinhardt, May 27, 2024
  92. 15/21 commit-reach: fix memory leak in `ahead_behind()`Patrick Steinhardt, May 27, 2024
  93. 16/21 submodule: fix leaking memory for submodule entriesPatrick Steinhardt, May 27, 2024
  94. 17/21 strvec: add functions to replace and remove stringsPatrick Steinhardt, May 27, 2024
  95. 18/21 builtin/mv: refactor `add_slash()` to always return allocated stringsPatrick Steinhardt, May 27, 2024
  96. 19/21 builtin/mv duplicate string list memoryPatrick Steinhardt, May 27, 2024
  97. 20/21 builtin/mv: refactor to use `struct strvec`Patrick Steinhardt, May 27, 2024
  98. 21/21 builtin/mv: fix leaks for submodule gitfile pathsPatrick Steinhardt, May 27, 2024
  99. Junio C HamanoMay 27, 2024
  100. 0/5 add-ons for ps/leakfixesJeff King, May 30, 2024
  101. 1/5 t-strvec: use va_end() to match va_start()Jeff King, May 30, 2024
  102. 2/5 t-strvec: mark variable-arg helper with LAST_ARG_MUST_BE_NULLJeff King, May 30, 2024
  103. 3/5 mv: move src_dir cleanup to end of cmd_mv()Jeff King, May 30, 2024
  104. Patrick SteinhardtMay 30, 2024
  105. Jeff KingMay 30, 2024
  106. Patrick SteinhardtMay 30, 2024
  107. Jeff KingMay 30, 2024
  108. Patrick SteinhardtMay 30, 2024
  109. Jeff KingMay 30, 2024
  110. 4/5 mv: factor out empty src_dir removalJeff King, May 30, 2024
  111. 5/5 mv: replace src_dir with a strvecJeff King, May 30, 2024
  112. Junio C HamanoMay 30, 2024
  113. Jeff KingMay 31, 2024
  114. Junio C HamanoMay 31, 2024
  115. Patrick SteinhardtMay 30, 2024

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.