сб, 25 апр. 2026 г. в 10:40, Willy Tarreau <[email protected]>:
> On Sat, Apr 25, 2026 at 07:14:47AM +0000, Egor Shestakov wrote:
> > Hi Bianca!
> >
> > On Fri, Apr 24, 2026 at 11:04:03PM +0000, Bianca Dogareci wrote:
> > > Please find attached a patch fixing a memory leak when realloc() fails
> > > in ssl_ckch.c.
> >
> > > Fix three instances of the classic realloc() bug in ckchs_dup() and
> > > ckch_conf_parse() where overwriting the original pointer with NULL
> > > on allocation failure loses reference to the original memory block.
> > >
> > > Use temporary pointers to check realloc() result before updating
> > > the original pointer. This prevents memory leaks when realloc()
> > > fails.
> >
> > You're right about this, thanks!
> >
> > HAProxy has a my_realloc2() function that does the same thing but also
> > frees the original pointer on failure, to make the handling of failures
> of
> > realloc() simpler.
> >
> > Could please adjust your patch for my_realloc2?
>
> Indeed. There's a Coccinelle test to find them:
>
> dev/coccinelle/realloc_leak.cocci
>
coccinelle looks a bit broken
$ spatch --sp-file dev/coccinelle/realloc_leak.cocci src/hlua.c
init_defs_builtins: /usr/lib64/coccinelle/standard.h
HANDLING: src/hlua.c
SPECIAL NAMES: adding DECLARE_STATIC_TYPED_POOL as a declarer
SPECIAL NAMES: adding list_for_each_entry_from as a iterator
SPECIAL NAMES: adding list_for_each_entry as a iterator
SPECIAL NAMES: adding list_for_each_entry_safe as a iterator
diff =
--- src/hlua.c
+++ /tmp/cocci-output-1646809-b4de51-hlua.c
@@ -13509,7 +13509,6 @@ static int hlua_load_per_thread(char **a
for (len = 0; per_thread_load[len] != NULL; len++)
;
- per_thread_load = realloc(per_thread_load, (len + 2) *
sizeof(*per_thread_load));
if (per_thread_load == NULL) {
memprintf(err, "out of memory error");
return -1;
@@ -14155,7 +14154,6 @@ static void *hlua_alloc(void *ud, void *
if (!nsize)
ha_free(&ptr);
else
- ptr = realloc(ptr, nsize);
return ptr;
}
@@ -14174,7 +14172,6 @@ static void *hlua_alloc(void *ud, void *
if (!nsize)
ha_free(&ptr);
else
- ptr = realloc(ptr, nsize);
if (unlikely(!ptr && nsize)) // failed
_HA_ATOMIC_SUB(&zone->allocated, nsize - osize);
$
it recommends just to remove all "realloc" occurences :)
it looks like "realloc" is not welcome. maybe it is done in purpose, not
sure.
taking into account that strict behaviour, maybe we should get rid of
realloc completely.
I used "cppcheck" for detention
$ grep memleakOnRealloc typescript
src/quic_rx.c:819:3: error: Common realloc mistake: 'stok_ptr' nulled but
not freed upon failure [memleakOnRealloc]
$
I'll try to report "treat realloc on exit as safe" to cppcheck, but
honestly they have huge backlog and just few hands.
btw, Egor, Biancca, feel free to play with "dev/coccinelle" scripts. they
are nice
>
> There are a few other instances of this issue. Most of them end up with an
> exit() so they are technically false positives but a few are real leaks
> (e.g. in Lua):
>
> hlua_load_per_thread() (this one will exit):
> src/hlua.c:13512: per_thread_load = realloc(per_thread_load, (len +
> 2) * sizeof(*per_thread_load));
> src/hlua.c-13513- if (per_thread_load == NULL) {
> src/hlua.c-13514- memprintf(err, "out of memory error");
> src/hlua.c-13515- return -1;
> src/hlua.c-13516- }
>
> hlua_alloc() (these ones are real leaks):
> src/hlua.c:14158: ptr = realloc(ptr, nsize);
> src/hlua.c-14159- return ptr;
> src/hlua.c-14160- }
>
> src/hlua.c:14177: ptr = realloc(ptr, nsize);
> src/hlua.c-14178-
> src/hlua.c-14179- if (unlikely(!ptr && nsize)) // failed
> src/hlua.c-14180- _HA_ATOMIC_SUB(&zone->allocated, nsize -
> osize);
>
> qc_try_store_new_token() (real leak):
> src/quic_rx.c:819: stok_ptr = realloc(stok_ptr, len);
> src/quic_rx.c-820- if (stok_ptr)
> src/quic_rx.c-821-
> s->per_thr[tid].quic_retry_token.ptr = stok_ptr;
> src/quic_rx.c-822- else {
> src/quic_rx.c-823- memset(istptr(*stok), 0,
> istlen(*stok));
> src/quic_rx.c-824- istfree(stok);
>
> ckchs_dup() (real leaks):
> src/ssl_ckch.c:1101: r = realloc(r, sizeof(char *) * (n
> + 2));
> src/ssl_ckch.c-1102- if (!r)
> src/ssl_ckch.c-1103- goto error;
>
> src/ssl_ckch.c:1122: r = realloc(r, sizeof(char *) * (n
> + 2));
> src/ssl_ckch.c-1123- if (!r)
> src/ssl_ckch.c-1124- goto error;
> src/ssl_ckch.c-1125-
>
> ckch_conf_parse() (real leak):
> src/ssl_ckch.c:5200: r = realloc(r,
> sizeof(char *) * (n + 2));
> src/ssl_ckch.c-5201- if (!r) {
> src/ssl_ckch.c-5202-
> ha_alert("parsing [%s:%d]: out of memory.\n", file, linenum);
> src/ssl_ckch.c-5203- err_code
> |= ERR_ALERT | ERR_ABORT;
> src/ssl_ckch.c-5204- goto
> array_err;
> src/ssl_ckch.c-5205- }
>
> ssl_sock_resize_passphrase_cache() : real leaks
> src/ssl_sock.c:3746: passphrase_randoms = realloc(passphrase_randoms,
> sizeof(*passphrase_randoms) * (new_size));
> src/ssl_sock.c-3747- if (!passphrase_randoms) {
> src/ssl_sock.c-3748- ha_alert("ssl_sock_passwd_cb: passphrase
> randoms realloc failed");
> src/ssl_sock.c-3749- passphrase_idx = -1;
> src/ssl_sock.c-3750- passphrase_cache_size = 0;
> src/ssl_sock.c-3751- }
>
> src/ssl_sock.c:3762: passphrase_cache =
> realloc(passphrase_cache, sizeof(*passphrase_cache) *
> passphrase_cache_size);
> src/ssl_sock.c-3763- if (!passphrase_cache) {
> src/ssl_sock.c-3764- ha_alert("ssl_sock_passwd_cb:
> passphrase cache realloc failed");
> src/ssl_sock.c-3765- passphrase_idx = -1;
> src/ssl_sock.c-3766- passphrase_cache_size = 0;
> src/ssl_sock.c-3767- }
>
> ssl_sess_new_srv_cb(): real leak and even looks worse since it doesn't
> reset the allocated size after the NULL pointer:
> src/ssl_sock.c:4254: ptr = realloc(ptr, len);
> src/ssl_sock.c-4255- if (!ptr)
> src/ssl_sock.c-4256-
> free(s->ssl_ctx.reused_sess[tid].ptr);
> src/ssl_sock.c-4257- s->ssl_ctx.reused_sess[tid].ptr =
> ptr;
> src/ssl_sock.c-4258-
> s->ssl_ctx.reused_sess[tid].allocated_size = len;
>
> It's important to fix them in separate patches (at least one per function)
> because it's very likely that several of them will need to be backported.
>
> Willy
>
>
>
