сб, 25 апр. 2026 г. в 12:48, [email protected] <[email protected] >:
> 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. > some of realloc usages are ok, some are not. if that script purpose is to find realloc, ok. > > > -- > William Lallemand >
