On Sat, Apr 25, 2026 at 10:53:04AM +0200, [email protected] wrote: > On Sat, Apr 25, 2026 at 10:39:52AM +0200, Willy Tarreau wrote: > > Subject: Re: [PATCH] BUG/MINOR: ssl_ckch: fix memory leak on realloc failure > > 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 > > > > 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 > > > > Ilya provided some fixes for some of them some weeks ago,
ah, sorry for missing them. > I thought we took them but the ckchs_dup() and > ssl_sock_resize_passphrase_cache() are addressed there. I'm going to merge > them. OK thanks! willy
