On Sat, Apr 25, 2026 at 12:37:42PM +0200, Илья Шипицин wrote: > Subject: Re: [PATCH] BUG/MINOR: ssl_ckch: fix memory leak on realloc failure > сб, 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. >
It's not broken, the cocci file was made to find locations where the return value is assigned to the same variable as the one in parameter. This one was not made to patch the issue by itself. -- William Lallemand
