Message ID | 20240604101327.GH1304593@coredump.intra.peff.net (mailing list archive) |
---|---|
State | Accepted |
Commit | d765fa03311631982017bdb3c755aff7ed3757d0 |
Headers | show |
Series | leak fixes for sparse-checkout code | expand |
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 75c07d5bb4..8f8f5c359f 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -581,6 +581,7 @@ static void add_patterns_from_input(struct pattern_list *pl, strbuf_to_cone_pattern(&line, pl); } } + strbuf_release(&line); } else { if (file) { struct strbuf line = STRBUF_INIT;
In add_patterns_from_input(), we may read lines from a file with a loop like this: while (!strbuf_getline(&line, file)) { ... strbuf_to_cone_pattern(&line, pl); } /* we don't strbuf_release(&line) here! */ This generally is OK because strbuf_to_cone_pattern() consumes the buffer via strbuf_detach(). But we can leak in a few cases: 1. We don't always consume the buffer! If the line ends up empty after trimming, we leave strbuf_to_cone_pattern() without detaching. In most cases this is OK, because a subsequent getline() call will use the same buffer. But if you had an empty line at the end of file, for example, it would leak. 2. Even if strbuf_to_cone_pattern() always consumed the buffer, there's a subtle issue with strbuf_getline(). As we saw in 94e2aa555e (strbuf: fix leak when `appendwholeline()` fails with EOF, 2024-05-27), it's possible for it to return EOF with an allocated buffer (e.g., if the underlying getdelim() call saw an error). So we should always strbuf_release() after finishing a read loop like this. Note that even the code to read patterns from argv has the same problem. Because that also uses strbuf_to_cone_pattern(), we stuff each argv entry into a strbuf. It uses the same "line" strbuf as the getline code, but we should position the strbuf_release() to cover both code paths. This fixes at least 9 leaks found in t1091. Signed-off-by: Jeff King <peff@peff.net> --- builtin/sparse-checkout.c | 1 + 1 file changed, 1 insertion(+)