Skip to content

replace open-coded allocation size arithmetic with overflow-safe helpers#269

Open
rootvector2 wants to merge 3 commits intodovecot:mainfrom
rootvector2:hardening/alloc-size-arithmetic
Open

replace open-coded allocation size arithmetic with overflow-safe helpers#269
rootvector2 wants to merge 3 commits intodovecot:mainfrom
rootvector2:hardening/alloc-size-arithmetic

Conversation

@rootvector2
Copy link
Contributor

Replace several instances of multi-term allocation size arithmetic (e.g. sizeof(T) + variable, a + b + c) with small helper macros built on existing MALLOC_ADD and MALLOC_MULTIPLY.

This keeps overflow handling centralized, improves consistency across the codebase, and makes size computations easier to audit.

No behavioral changes intended.

Replace several instances of multi-term allocation size arithmetic
(e.g. sizeof(T) + variable, a + b + c) with small helper macros
built on existing MALLOC_ADD and MALLOC_MULTIPLY.

This keeps overflow handling centralized, improves consistency across
the codebase, and makes size computations easier to audit.

No behavioral changes intended.
MALLOC_MULTIPLY(sizeof(type), (count))

#define MALLOC_SIZEOF_PLUS(type, extra) \
MALLOC_ADD(sizeof(type), (extra))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me these MALLOC_SIZEOF_* macros look like they're less readable, and also the resulting code isn't much smaller (1 byte if I count right).

st.st_size + 1);
size_t buf_size = MALLOC_ADD3(prefix_len, value_path_len, 1);
buf_size = MALLOC_ADD(buf_size, (size_t)st.st_size);
buf_size = MALLOC_ADD(buf_size, 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buf_size = MALLOC_ADD3(buf_size, (size_t)st.st_size, 1);

i_malloc(sizeof(struct stats_dist) +
sizeof(uint64_t) * sample_count);
i_malloc(MALLOC_SIZEOF_PLUS(struct stats_dist,
MALLOC_SIZEOF_MULTIPLY(uint64_t, sample_count)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised there are so few allocations that are using arithmetic. But if we do these changes, we should change all of them to be consistent. You're missing some of the +1 ones, like dict-cdb.c, http-header.c, imap-parser.c, strfuncs.c at least. Also grep for t_malloc, that has some others too.

@rootvector2
Copy link
Contributor Author

Thanks for the feedback.

I’ve removed the MALLOC_SIZEOF_* helpers and kept only MALLOC_ADD3.
I also applied the same allocation-style cleanup consistently across
remaining multi-term and sizeof(T) * count allocation sites (including
t_malloc/p_realloc cases), while keeping the changes mechanical and
avoiding allocator internals.

Please let me know if this looks better.

@rootvector2 rootvector2 requested a review from sirainen February 20, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants