mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-21 05:41:26 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: applet/new: fix sedesc freeing logic

Browse Source 
Since 465a6c8 ("BUG/MEDIUM: applet: only set appctx->sedesc on
successful allocation"), sedesc is attached to the appctx after the
task is successfully allocated.

If the task fails to allocate: current sedesc cleanup is performed
on appctx->sedesc which still points to NULL so sedesc won't be
freed.
This is fine when sedesc is provided as argument (!=NULL), but leads
to memory leaks if sedesc is allocated locally.

It was shown in GH #2086 that if sedesc != NULL when passed as
argument, it shouldn't be freed on error paths. This is what 465a6c8
was trying to address.

In an attempt to fix both issues at once, we do as Christopher
suggested: that is moving sedesc allocation attempt at the
end of the function, so that we don't have to free it in case
of error, thus removing the ambiguity.
(We won't risk freeing a sedesc that does not belong to us)

If we fail to allocate sedesc, then the task that was previously
created locally is simply destroyed.

This needs to be backported to 2.6 with 465a6c8 ("BUG/MEDIUM: applet:
only set appctx->sedesc on successful allocation")
[Copy pasting the original backport note from Willy:
In 2.6 the function is slightly
different and called appctx_new(), though the issue is exactly the
same.]
This commit is contained in:
Aurelien DARRAGON 2023-03-24 10:01:19 +01:00 committed by Christopher Faulet
parent 551b896772
commit 821581c990
1 changed files with 10 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/applet.c
View File

@ -48,19 +48,19 @@ struct appctx *appctx_new_on(struct applet *applet, struct sedesc *sedesc, int t
appctx->obj_type = OBJ_TYPE_APPCTX;
appctx->applet = applet;
appctx->sess = NULL;
appctx->sedesc = NULL;
if (!sedesc) {
sedesc = sedesc_new();
if (!sedesc)
goto fail_endp;
sedesc->se = appctx;
se_fl_set(sedesc, SE_FL_T_APPLET | SE_FL_ORPHAN);
}
appctx->t = task_new_on(thr);
if (unlikely(!appctx->t))
goto fail_task;
if (!sedesc) {
sedesc = sedesc_new();
if (unlikely(!sedesc))
goto fail_endp;
sedesc->se = appctx;
se_fl_set(sedesc, SE_FL_T_APPLET | SE_FL_ORPHAN);
}
appctx->sedesc = sedesc;
appctx->t->process = task_run_applet;
appctx->t->context = appctx;
@ -72,9 +72,9 @@ struct appctx *appctx_new_on(struct applet *applet, struct sedesc *sedesc, int t
_HA_ATOMIC_INC(&nb_applets);
return appctx;
fail_task:
sedesc_free(appctx->sedesc);
fail_endp:
task_destroy(appctx->t);
fail_task:
pool_free(pool_head_appctx, appctx);
fail_appctx:
return NULL;