Re: [PATCH v3] diff-delta: use safe integer arithmetic
flat view
From: Junio C Hamano <hidden>
Date: 2026-10-24 16:15:52
"Srujith A Poojary via GitGitGadget" [off-list ref]
writes:
From: Srujith A Poojary <redacted> Replace manual multiplication and addition in create_delta_index() with safe integer helpers (st_mult, st_add, st_add3) to prevent potential overflows when computing memory allocation sizes. On 32-bit platforms, struct alignment padding could cause the total allocation size calculation to exceed 2^32, making safe integer arithmetic necessary.
I wonder if "X could cause Y" above skips a bit too many steps. I was expecting more along the lines of ... We will need to allocate N instances of A and M instances of B, N and M are capped to X and Y respectively by this code ... code snippet ... and size of an instance of A is 8 and size of B could be either W or Z. When size of B is W, then the total allocation would be ... And that analysis applies only on the first allocation. I didn't even look at the packed side of the allocation size, expecting that you would know how to analyze after seeing my example on the other allocation.
Note that hsize + 1 inside st_add3() is safe from overflow because hsize is capped at 2^26 early in the function.
It is misleading to say hsize+1 is inside st_add3(), though.
quoted hunk ↗ jump to hunk
diff --git a/diff-delta.c b/diff-delta.c index 7cbedeb507..efebce3aca 100644 --- a/diff-delta.c +++ b/diff-delta.c@@ -162,8 +162,8 @@ struct delta_index * create_delta_index(const void *buf, size_t bufsize) hmask = hsize - 1; /* allocate lookup index */ - memsize = sizeof(*hash) * hsize + - sizeof(*entry) * entries; + memsize = st_add(st_mult(sizeof(*hash), hsize), + st_mult(sizeof(*entry), entries)); mem = malloc(memsize); if (!mem) return NULL;@@ -258,9 +258,9 @@ struct delta_index * create_delta_index(const void *buf, size_t bufsize) * Now create the packed index in array form * rather than linked lists. */ - memsize = sizeof(*index) - + sizeof(*packed_hash) * (hsize+1) - + sizeof(*packed_entry) * entries; + memsize = st_add3(sizeof(*index), + st_mult(sizeof(*packed_hash), hsize + 1), + st_mult(sizeof(*packed_entry), entries)); mem = malloc(memsize); if (!mem) { free(hash);base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926