From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 16 Dec 2024 10:04:53 +0000 (+0100)
Subject: BUG/MEDIUM: stconn: Only consider I/O timers to update stream's expiration date
X-Git-Tag: v3.2-dev2~66
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4f32d03360e42a3e7fe4ed71012ec90339b9731d;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: stconn: Only consider I/O timers to update stream's expiration date

In sc_notify(), it remained a case where it was possible to set an
expiration date on the stream in the past, leading to a crash because of a
BUG_ON(). This must never happen of course.

In sc_notify(), The stream's expiration may be updated in case no wakeup
conditions are encoutered. In that case, we must take care to never set an
expiration date in the past. However, it appeared there was still a
condition to do so. This code is based on an implicit postulate: the
stream's expiration date must always be set when we leave
process_stream(). It was true since the 2.9. But in 3.0, the buffer
allocation mechanism was improved and on an alloc failure in
process_stream(), the stream is inserted in a wait-list and its expiration
date is set to TICK_ETERNITY. With the good timing, and an analysis
expiration date set on a channel, it is possible to set the stream's
expiration date in past.

After analysis, it appeared that the proper way to fix the issue is to only
evaluate I/O timers (read and write timeout) and not stream's timers
(analase_exp or conn_exp) because only I/O timers may have changed since the
last process_stream() call.

This patch must be backported as far as 3.0 to fix the issue. But it is
probably a good idea to also backported it as far as 2.8.
---

diff --git a/src/stconn.c b/src/stconn.c
index a19fa1c8e1..1e69abc48f 100644
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -1199,17 +1199,15 @@ void sc_notify(struct stconn *sc)
 		task_wakeup(task, TASK_WOKEN_IO);
 	}
 	else {
-		/* Update expiration date for the task and requeue it if not already expired */
+		/* Update expiration date for the task and requeue it if not already expired.
+		 * Only I/O timeouts are evaluated. The stream is responsible of others.
+		 */
 		if (!tick_is_expired(task->expire, now_ms)) {
 			task->expire = tick_first(task->expire, sc_ep_rcv_ex(sc));
 			task->expire = tick_first(task->expire, sc_ep_snd_ex(sc));
 			task->expire = tick_first(task->expire, sc_ep_rcv_ex(sco));
 			task->expire = tick_first(task->expire, sc_ep_snd_ex(sco));
-			task->expire = tick_first(task->expire, ic->analyse_exp);
-			task->expire = tick_first(task->expire, oc->analyse_exp);
-			task->expire = tick_first(task->expire, __sc_strm(sc)->conn_exp);
 
-			/* WARNING: Don't forget to remove this BUG_ON before 2.9.0 */
 			BUG_ON(tick_is_expired(task->expire, now_ms));
 			task_queue(task);
 		}