From mboxrd@z Thu Jan  1 00:00:00 1970
From: john at keeping.me.uk (John Keeping)
Date: Sun, 16 Oct 2016 12:54:23 +0100
Subject: [PATCH 1/1] ui-log: fix data corruption
In-Reply-To: <CAHmME9ogcpFmL0pSjg=g2bUVG-1Zrw=DZHp6_LoVSc8mGNLi_w@mail.gmail.com>
References: <20161007133526.2931-1-list@eworm.de>
 <CAHmME9oBNqZqgapgxiF=fX5Ufj9bOG5-yOnSs9m1Nb3nhZR+tQ@mail.gmail.com>
 <20161007232123.5d7e8789@leda.localdomain>
 <20161009112602.GN1997@john.keeping.me.uk>
 <20161010160022.77e9feac@leda.localdomain>
 <CAHmME9ogcpFmL0pSjg=g2bUVG-1Zrw=DZHp6_LoVSc8mGNLi_w@mail.gmail.com>
Message-ID: <20161016115423.GF22407@john.keeping.me.uk>

On Wed, Oct 12, 2016 at 01:23:45PM +0200, Jason A. Donenfeld wrote:
> So what's the verdict here? Do we have a double free or not?

The current code is fine, providing we don't re-order it.

The issue is that we free commit blobs while walking the log, so any
attempt to use them after that without reloading the data goes through a
null pointer.

We should possibly change cgit_parse_commit() to use get_commit_buffer()
rather than get_cached_commit_buffer(), which improves the usability of
that function but hides potential inefficiency if we free the buffer and
end up reloading it.

The use of get_cached_commit_buffer() dates from an update in git.git
which moved the buffer pointer out of struct commit, previously we just
accessed commit->buffer here and relied on the buffer having been loaded
previously.