git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Taylor Blau <me@ttaylorr.com>
Subject: [PATCH 3/3] revision: avoid leak when preparing bloom filter for "/"
Date: Tue, 4 Aug 2020 03:50:17 -0400	[thread overview]
Message-ID: <20200804075017.GC284046@coredump.intra.peff.net> (raw)
In-Reply-To: <20200804074146.GA190027@coredump.intra.peff.net>

If we're given an empty pathspec, we refuse to set up bloom filters, as
described in f3c2a36810 (revision: empty pathspecs should not use Bloom
filters, 2020-07-01).

But before the empty string check, we drop any trailing slash by
allocating a new string without it. So a pathspec consisting only of "/"
will allocate that string, but then still cause us to bail, leaking the
new string. Let's make sure to free it.

Signed-off-by: Jeff King <peff@peff.net>
---
Just noticed while reading the function to fix the previous patch.

I'm not even sure if it's possible to get here with a pathspec of "/",
since we'd probably give a "/ is outside repository" error before then.

So maybe this case doesn't even matter. If it doesn't, then it might
simplify the function a bit to do the empty-pathspec check before
handling trailing slashes. But handling it does help make it more clear
this function is doing the right thing no matter what input it is given,
so that's what I went with here.

 revision.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/revision.c b/revision.c
index 5ed86e4524..b80868556b 100644
--- a/revision.c
+++ b/revision.c
@@ -702,6 +702,7 @@ static void prepare_to_use_bloom_filter(struct rev_info *revs)
 	len = strlen(path);
 	if (!len) {
 		revs->bloom_filter_settings = NULL;
+		free(path_alloc);
 		return;
 	}
 
-- 
2.28.0.536.ga4d8134877

  parent reply	other threads:[~2020-08-04  7:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-04  7:41 [PATCH 0/3] some compiler/asan/ubsan fixes Jeff King
2020-08-04  7:43 ` [PATCH 1/3] config: work around gcc-10 -Wstringop-overflow warning Jeff King
2020-08-04 16:30   ` Junio C Hamano
2020-08-05 15:15     ` Taylor Blau
2020-08-04  7:46 ` [PATCH 2/3] revision: avoid out-of-bounds read/write on empty pathspec Jeff King
2020-08-04 13:08   ` Derrick Stolee
2020-08-05 15:17   ` Taylor Blau
2020-08-04  7:50 ` Jeff King [this message]
2020-08-04 13:09   ` [PATCH 3/3] revision: avoid leak when preparing bloom filter for "/" Derrick Stolee
2020-08-05 15:19     ` Taylor Blau

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20200804075017.GC284046@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --subject='Re: [PATCH 3/3] revision: avoid leak when preparing bloom filter for "/"' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).