On Tue, Oct 18, 2022 at 02:42:09PM -0600, Todd C. Miller wrote:
> On Tue, 18 Oct 2022 22:15:57 +0200, Stefan Sperling wrote:
> 
> > This patch avoids a per-tree malloc/free dance in got-read-tree
> > and got-read-pack while parsing trees.
> >
> > Instead of throwing the parsed-tree-entries array away after
> > parsing a tree, we can leave its memory allocation in place,
> > and keep extending it as needed with recallocarray().
> 
> Previously the newly-allocated memory was always zeroed out.  Now
> that you are re-using the old allocation do you also need to zero
> it before re-use?  I can't tell whether the callers really expect
> zeroed memory or if this was just to be on the safe side.
> 
>  - todd
> 

It is an array of the following struct:

struct got_parsed_tree_entry {
        const char *name; /* Points to name in parsed buffer */
        size_t namelen; /* strlen(name) */
        mode_t mode; /* Mode parsed from tree buffer. */
        uint8_t *id; /* Points to ID in parsed tree buffer. */
};

So this struct provides offsets into another buffer, where the
actual tree data is stored. This tree buffer itself is still
allocated and freed each time.

Each time we parse a new tree, all fields of above struct will be
rewritten, up to as many entries as the newly parsed tree contains.
If the allocation is larger, the remaining entries will contain
pointers into tree buffers which have been freed, and callers
know not to look at entries beyond the number of valid entries.
Could such pointers be abused? If so, I could simply NULL them out
before returning from the parser.