Thread (14 messages) 14 messages, 3 authors, 2024-05-15

Re: [PATCH 5/6] pack-bitmap-write.c: avoid uninitialized 'write_as' field

From: Patrick Steinhardt <hidden>
Date: 2024-05-15 09:05:18

On Tue, May 14, 2024 at 03:57:03PM -0400, Taylor Blau wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
 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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help