From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 10 Nov 2020 17:45:34 +0000 (+0100)
Subject: BUG/MAJOR: spoe: Be sure to remove all references on a released spoe applet
X-Git-Tag: v2.1.11~56
X-Git-Url: http://git.haproxy.org/?a=commitdiff_plain;h=8fed2b58cb2f225f3670fa15145647fd11090d54;p=haproxy-2.1.git

BUG/MAJOR: spoe: Be sure to remove all references on a released spoe applet

When a SPOE applet is used to send a frame, a reference on this applet is saved
in the spoe context of the offladed stream. But, if the applet is released
before receving the corresponding ack, we must be sure to remove this
reference. This was performed for fragmented frames only. But it must also be
performed for a spoe contexts in the applet waiting_queue and in the thread
waiting_queue (used in async mode).

This bug leads to a memory corruption when an offloaded stream try to update the
state of a released applet because it still have a reference on it. There are
many ways to trigger this bug. The easiest is probably during reloads. On the
old process, all applets are woken up to be released ASAP.

Many thanks to Maciej Zdeb to report the bug and to work on it for 2
months. Without his help, it would have been much more difficult to fix the
bug. It is always a huge pleasure to see how some users are enthousiast and
helpful. Thanks again Maciej !

This patch must be backported to all versions where the spoe is supported (>=
1.7).

(cherry picked from commit cf181c76e341f2d49f6cae0ca8200158058073f1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit abe894d2c91da5f57ae7704eba59c41b409fc1a0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit a2cf638010c841aa28c9d5a3ef57155fdc74cff5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
---

diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index 39585ee..3b3bc16 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -1286,6 +1286,7 @@ spoe_release_appctx(struct appctx *appctx)
 		LIST_INIT(&ctx->list);
 		_HA_ATOMIC_SUB(&agent->counters.nb_waiting, 1);
 		spoe_update_stat_time(&ctx->stats.tv_wait, &ctx->stats.t_waiting);
+		ctx->spoe_appctx = NULL;
 		ctx->state = SPOE_CTX_ST_ERROR;
 		ctx->status_code = (spoe_appctx->status_code + 0x100);
 		task_wakeup(ctx->strm->task, TASK_WOKEN_MSG);
@@ -1301,8 +1302,13 @@ spoe_release_appctx(struct appctx *appctx)
 		task_wakeup(ctx->strm->task, TASK_WOKEN_MSG);
 	}
 
-	if (!LIST_ISEMPTY(&agent->rt[tid].applets))
+	if (!LIST_ISEMPTY(&agent->rt[tid].applets)) {
+		list_for_each_entry_safe(ctx, back, &agent->rt[tid].waiting_queue, list) {
+			if (ctx->spoe_appctx == spoe_appctx)
+				ctx->spoe_appctx = NULL;
+		}
 		goto end;
+	}
 
 	/* If this was the last running applet, notify all waiting streams */
 	list_for_each_entry_safe(ctx, back, &agent->rt[tid].sending_queue, list) {
@@ -1310,6 +1316,7 @@ spoe_release_appctx(struct appctx *appctx)
 		LIST_INIT(&ctx->list);
 		_HA_ATOMIC_SUB(&agent->counters.nb_sending, 1);
 		spoe_update_stat_time(&ctx->stats.tv_queue, &ctx->stats.t_queue);
+		ctx->spoe_appctx = NULL;
 		ctx->state = SPOE_CTX_ST_ERROR;
 		ctx->status_code = (spoe_appctx->status_code + 0x100);
 		task_wakeup(ctx->strm->task, TASK_WOKEN_MSG);
@@ -1319,6 +1326,7 @@ spoe_release_appctx(struct appctx *appctx)
 		LIST_INIT(&ctx->list);
 		_HA_ATOMIC_SUB(&agent->counters.nb_waiting, 1);
 		spoe_update_stat_time(&ctx->stats.tv_wait, &ctx->stats.t_waiting);
+		ctx->spoe_appctx = NULL;
 		ctx->state = SPOE_CTX_ST_ERROR;
 		ctx->status_code = (spoe_appctx->status_code + 0x100);
 		task_wakeup(ctx->strm->task, TASK_WOKEN_MSG);