Git development
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH 1/2] apply: plug leak on "patch too large" error
Date: Fri, 15 May 2026 22:16:22 -0400	[thread overview]
Message-ID: <20260516021622.GA744303@coredump.intra.peff.net> (raw)
In-Reply-To: <20260516021343.GA174647@coredump.intra.peff.net>

In apply_patch(), we return immediately if read_patch_file() returns an
error. Traditionally this was OK, since an error from strbuf_read()
would restore the strbuf to its unallocated state.

But since f1c0e3946e (apply: reject patches larger than ~1 GiB,
2022-10-25), we may also return an error if we successfully read the
patch but it is too large. In this case we leak the strbuf contents when
apply_patch() returns.

You can see it in action by running t4141 under LSan with the EXPENSIVE
prereq enabled.

We can fix this in one of two places:

  1. In read_patch_file(), we could release the buffer before returning
     the error, behaving more like a raw strbuf_read() call.

  2. In apply_patch(), we can release the strbuf ourselves before
     returning.

I picked the latter, since it future proofs us against read_patch_file()
getting new error modes. We also have a cleanup label in that function
already, so now our error handling at this spot matches the rest of the
function (and all of the variables are initialized such that the rest of
the cleanup is correctly a noop at this point).

Signed-off-by: Jeff King <peff@peff.net>
---
 apply.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/apply.c b/apply.c
index 4aa1694cfa..249248d4f2 100644
--- a/apply.c
+++ b/apply.c
@@ -4881,8 +4881,10 @@ static int apply_patch(struct apply_state *state,
 
 	state->patch_input_file = filename;
 	state->linenr = 1;
-	if (read_patch_file(&buf, fd) < 0)
-		return -128;
+	if (read_patch_file(&buf, fd) < 0) {
+		res = -128;
+		goto end;
+	}
 	offset = 0;
 	while (offset < buf.len) {
 		struct patch *patch;
-- 
2.54.0.490.gaeb18d0c26


  reply	other threads:[~2026-05-16  2:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15 23:43 UBSan failing on expensive test(s) Junio C Hamano
2026-05-16  2:13 ` Jeff King
2026-05-16  2:16   ` Jeff King [this message]
2026-05-16  2:23   ` [PATCH 2/2] commit: handle large commit messages in utf8 verification Jeff King
2026-05-16  2:55   ` UBSan failing on expensive test(s) Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260516021622.GA744303@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox