From: Patrick Steinhardt Date: Wed, 15 May 2024 09:05:10 GMT Subject: Re: [PATCH 5/6] pack-bitmap-write.c: avoid uninitialized 'write_as' field Message-ID: In-Reply-To: On Tue, May 14, 2024 at 03:57:03PM -0400, Taylor Blau wrote: > Prepare to free() memory associated with bitmapped_commit structs by > zero'ing the 'write_as' field. > > In ideal cases, it is fine to do something like: > > for (i = 0; i < writer->selected_nr; i++) { > struct bitmapped_commit *bc = &writer->selected[i]; > if (bc->write_as != bc->bitmap) > ewah_free(bc->write_as); > ewah_free(bc->bitmap); > } > > but if not all of the 'write_as' fields were populated (e.g., because > the packing_data given does not form a reachability closure), then we > may attempt to free uninitialized memory. > > Guard against this by preemptively zero'ing this field just in case. > > Signed-off-by: Taylor Blau > --- > pack-bitmap-write.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c > index c0087dab12..420f17c2e0 100644 > --- a/pack-bitmap-write.c > +++ b/pack-bitmap-write.c > @@ -112,6 +112,7 @@ static inline void push_bitmapped_commit(struct bitmap_writer *writer, > > writer->selected[writer->selected_nr].commit = commit; > writer->selected[writer->selected_nr].bitmap = NULL; > + writer->selected[writer->selected_nr].write_as = NULL; > writer->selected[writer->selected_nr].flags = 0; Instead of having to ensure that all fields are initialized we could also set the whole structure to zero via `memset()`, which might be a bit more sustainable in the future. That alone doesn't really warrant a reroll though. Patrick