-
Notifications
You must be signed in to change notification settings - Fork 74
Memory leak in par_msquares_function when PAR_MSQUARES_DUAL is enabled #61
Description
Summary
LeakSanitizer reports a small but repeatable memory leak when calling par_msquares_grayscale with the PAR_MSQUARES_DUAL flag enabled. The allocation originates from par_msquares_function allocating the mlist->meshes pointer array via PAR_CALLOC, but the allocation is not released on the PAR_MSQUARES_DUAL recursion / merge path.
Even though the leak is small per invocation (two allocations of 8 bytes on x86_64 in my run), it can accumulate in long-running processes that repeatedly invoke marching squares, potentially leading to memory growth / DoS over time.
Affected version
- Repository:
prideout/par - Commit:
b3571fdf83518d921af3b69d13ea0cb8749147aa - Component: marching squares (contour extraction / mesh generation)
- File:
par_msquares.h - Function:
par_msquares_function
Environment
- OS: Ubuntu 22.04.5 LTS
- Kernel: Linux 5.15.0-160-generic
- Tooling: Clang + libFuzzer + AddressSanitizer/LeakSanitizer
Steps to reproduce (fuzz harness)
- Checkout the affected commit.
- Build the following harness with libFuzzer + ASan/LSan.
- Run the fuzzer; LeakSanitizer reports leaks (in my case, it triggers quickly and even reports a leak found in the initial corpus).
Harness (fuzz_msquares.c)
#include <stdint.h>
#include <stddef.h>
#include <stdlib.h>
#include <math.h>
#define PAR_MSQUARES_IMPLEMENTATION
#include "par_msquares.h"
static uint32_t read_u32(const uint8_t *p) {
return ((uint32_t)p[0] << 24) |
((uint32_t)p[1] << 16) |
((uint32_t)p[2] << 8) |
((uint32_t)p[3]);
}
int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
if (size < 32) return 0;
const size_t header = 24;
uint32_t w_raw = read_u32(data + 0);
uint32_t h_raw = read_u32(data + 4);
uint32_t cell_raw = read_u32(data + 8);
uint32_t thr_raw = read_u32(data + 12);
int cells_x = (int)(w_raw % 64u) + 1; // 1..64
int cells_y = (int)(h_raw % 64u) + 1; // 1..64
int cellsize = (int)(cell_raw % 16u) + 1; // 1..16
int width = cells_x * cellsize;
int height = cells_y * cellsize;
float threshold = (float)thr_raw / (float)UINT32_MAX;
size_t num_pixels_from_data = (size - header) / 4;
if (num_pixels_from_data == 0) return 0;
size_t target_pixels = (size_t)width * (size_t)height;
if (num_pixels_from_data < target_pixels) {
size_t side = (size_t)sqrt((double)num_pixels_from_data);
if (side == 0) return 0;
width = (int)side;
height = (int)side;
target_pixels = side * side;
}
if (width <= 0 || (width % cellsize) != 0) return 0;
float *gray = (float *)malloc(target_pixels * sizeof(float));
if (!gray) return 0;
const uint8_t *p = data + header;
for (size_t i = 0; i < target_pixels; ++i) {
if ((size_t)(p - data + 4) > size) {
gray[i] = 0.0f;
continue;
}
uint32_t v = read_u32(p);
p += 4;
gray[i] = (float)v / (float)UINT32_MAX;
}
int flags = (int)data[header % size];
par_msquares_meshlist *mlist =
par_msquares_grayscale(gray, width, height, cellsize, threshold, flags);
if (mlist) {
int count = par_msquares_get_count(mlist);
for (int i = 0; i < count; ++i) {
const par_msquares_mesh *mesh = par_msquares_get_mesh(mlist, i);
if (!mesh) continue;
volatile int npoints = mesh->npoints;
volatile int ntriangles = mesh->ntriangles;
(void)npoints;
(void)ntriangles;
}
par_msquares_free(mlist);
}
free(gray);
return 0;
}Build & run example
# example (adjust compiler flags as you prefer)
clang -g -O1 -fsanitize=fuzzer,address,undefined -fno-omit-frame-pointer \
fuzz_msquares.c -o fuzz_msquares
ASAN_OPTIONS=detect_leaks=1 ./fuzz_msquares -runs=1000LeakSanitizer output
=================================================================
==14==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x55a8617a6919 in calloc ...
#1 0x55a8617fba38 in par_msquares_function.specialized.1 .../par_msquares.h:715:21
#2 0x55a8617fbc44 in par_msquares_function.specialized.1 .../par_msquares.h:698:16
#3 0x55a8617f9ed4 in par_msquares_grayscale .../par_msquares.h:484:12
#4 0x55a8617f9ed4 in LLVMFuzzerTestOneInput .../fuzz_msquares.c:83:9
...
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x55a8617a6919 in calloc ...
#1 0x55a8617fba38 in par_msquares_function.specialized.1 .../par_msquares.h:715:21
#2 0x55a8617fbb80 in par_msquares_function.specialized.1 .../par_msquares.h:692:16
#3 0x55a8617f9ed4 in par_msquares_grayscale .../par_msquares.h:484:12
#4 0x55a8617f9ed4 in LLVMFuzzerTestOneInput .../fuzz_msquares.c:83:9
...
SUMMARY: AddressSanitizer: 16 byte(s) leaked in 2 allocation(s).
=================================================================Suspected root cause
In par_msquares_function, when flags contains PAR_MSQUARES_DUAL, the function calls itself twice (two recursive invocations). Each invocation allocates a par_msquares_meshlist and a mlist->meshes pointer array via:
mlist->meshes = PAR_CALLOC(par_msquares__mesh*, 1);However, on the PAR_MSQUARES_DUAL path, the intermediate meshlists returned from the recursive calls appear to have their mlist->meshes arrays “lost” (not freed) even though the final result is freed via par_msquares_free(). This results in two small direct leaks per top-level call (on x86_64: 2 * 8 bytes).
Suggested fix
On the PAR_MSQUARES_DUAL merge path, after transferring ownership of the generated mesh pointers into the merged result, ensure that all intermediate allocations are released, including the intermediate mlist->meshes arrays, before freeing the intermediate meshlist objects.
Conceptually:
- if
merged->meshes[...]points to meshes produced bym0/m1, - then free
m0->meshes/m1->meshes(the pointer arrays), - then free
m0/m1(the meshlist structs), - and finally return
merged.
If you’d like, I can also propose a minimal patch once I can see the exact PAR_MSQUARES_DUAL branch code around the merge logic.