Fix use after free bug in LinesBucketQueue::emplace_back_bucket

I found a use after free bug in LinesBucketQueue::emplace_back_bucket. This was found by enabling address sanitizer.
The LinesBucketQueue class has two related members:
std::vector<LinesBucket> line_buckets;
std::priority_queue<LinesBucket *, std::vector<LinesBucket *>, LinesBucketPtrComp> line_bucket_ptr_queue;
line_bucket_ptr_queue holds pointers into line_buckets. However, since items are inserted into line_buckets one at a time, existing pointers that were stored inside line_bucket_ptr_queue become invalid. Specifically:
void LinesBucketQueue::emplace_back_bucket(ExtrusionLayers &&els, const void *objPtr, Point offset)
{
    auto oldSize = line_buckets.capacity();
    line_buckets.emplace_back(std::move(els), objPtr, offset); <--- Causes a reallocation, making previous pointers invalid
    line_bucket_ptr_queue.push(&line_buckets.back()); <-- priority queue compares against old, now invalid pointers
    ...

The proposed fix is to calculate the required number of entries in ConflictChecker::find_inter_of_lines_in_diff_objs, and then calling line_buckets.reserve(count). This ensures that sufficient buffer is allocated up front and the pointers are stable as items are added.
This commit is contained in:
Momin Al-Ghosien 2024-04-28 18:03:34 -07:00 committed by Lane.Wei
parent 437e445dc8
commit 306b09b4f5
2 changed files with 14 additions and 0 deletions

View File

@ -102,6 +102,11 @@ void LinesBucketQueue::emplace_back_bucket(ExtrusionLayers &&els, const void *ob
}
}
void LinesBucketQueue::reserve(size_t count)
{
line_buckets.reserve(count);
}
// remove lowest and get the current bottom z
float LinesBucketQueue::getCurrBottomZ()
{
@ -218,6 +223,14 @@ ConflictResultOpt ConflictChecker::find_inter_of_lines_in_diff_objs(PrintObjectP
{
if (objs.size() <= 1 && !wtdptr) { return {}; }
LinesBucketQueue conflictQueue;
// Calculate the number of required entries in the conflict queue
// One slot is used for the FakeWipeTower and 2 slots for each object
size_t requiredCount = 2*objs.size() + (wtdptr.has_value() ? 1 : 0);
// Reserve the required count to guarantee that pointers inside LinesBucketQueue::line_buckets
// are stable while we append the entries
conflictQueue.reserve(requiredCount);
if (wtdptr.has_value()) { // wipe tower at 0 by default
auto wtpaths = wtdptr.value()->getFakeExtrusionPathsFromWipeTower();
ExtrusionLayers wtels;

View File

@ -114,6 +114,7 @@ public:
public:
void emplace_back_bucket(ExtrusionLayers &&els, const void *objPtr, Point offset);
void reserve(size_t count);
bool valid() const { return line_bucket_ptr_queue.empty() == false; }
float getCurrBottomZ();
LineWithIDs getCurLines() const;