mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-12-04 17:21:12 +01:00
Code Issues Projects Releases Wiki Activity

BUG/MEDIUM: h1: prevent a crash on HTTP/2 upgrade

Browse Source 
Change h1_process() to return -2 when the mux is destroyed but the
connection is not, so that we can differentiate between "both mux and
connection were destroyed" and "only the mux was destroyed".
It can happen that only the mux gets destroyed, and the connection is
still alive, if we did upgrade it to HTTP/2.
In h1_wake(), if the connection is alive, then return 0, as the wake
methods should only return -1 if the connection is dead.
This fixes a bug where the ssl xprt would consider the connection
destroyed, and thus would consider its tasklet should die, and return
NULL, and its TASK_RUNNING flag would never be removed, leading to an
infinite loop later on. This would happen anytime an HTTP/2 upgrade was
successful.

This should be backported up to 2.8. While the bug by commit
00f43b7c8b136515653bcb2fc014b0832ec32d61, it was not triggered before
only by chance, and exists in previous releases too.
This commit is contained in:
Olivier Houchard 2025-11-14 12:42:38 +01:00 committed by Olivier Houchard
parent 2f8f09854f
commit 333deef485
1 changed files with 24 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/mux_h1.c
View File

@ -1210,7 +1210,7 @@ static int h1s_finish_detach(struct h1s *h1s)
* subscribe for reads waiting for new data * subscribe for reads waiting for new data
*/ */
if (unlikely(b_data(&h1c->ibuf))) { if (unlikely(b_data(&h1c->ibuf))) {
if (h1_process(h1c) == -1) { if (h1_process(h1c) < 0) {
/* h1c was released, don't reuse it anymore */ /* h1c was released, don't reuse it anymore */
goto released; goto released;
} }
@ -4078,12 +4078,14 @@ static int h1_send(struct h1c *h1c)
} }
/* callback called on any event by the connection handler. /* callback called on any event by the connection handler.
* It applies changes and returns zero, or < 0 if it wants immediate * It applies changes and returns zero, -1 if everything was destroyed,
* destruction of the connection. * and -2 if the mux was destroyed, but the connection is still alive because
* it was upgraded to H2.
*/ */
static int h1_process(struct h1c * h1c) static int h1_process(struct h1c * h1c)
{ {
struct connection *conn = h1c->conn; struct connection *conn = h1c->conn;
int ret = -1;
TRACE_ENTER(H1_EV_H1C_WAKE, conn); TRACE_ENTER(H1_EV_H1C_WAKE, conn);
@ -4296,11 +4298,20 @@ static int h1_process(struct h1c * h1c)
return 0; return 0;
} }
else { else {
h1_release(h1c); int relret;
relret = h1_release(h1c);
/*
* Okay if we just released the mux, but the connection itself
* is alive, because we upgraded to H2, we have to let the
* caller know.
*/
if (relret == -1)
ret = -2;
TRACE_DEVEL("leaving after releasing the connection", H1_EV_H1C_WAKE); TRACE_DEVEL("leaving after releasing the connection", H1_EV_H1C_WAKE);
} }
released: released:
return -1; return ret;
} }
struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state) struct task *h1_io_cb(struct task *t, void *ctx, unsigned int state)
@ -4406,7 +4417,13 @@ static int h1_wake(struct connection *conn)
if (h1c->state == H1_CS_UPGRADING || h1c->state == H1_CS_RUNNING) if (h1c->state == H1_CS_UPGRADING || h1c->state == H1_CS_RUNNING)
h1_alert(h1s); h1_alert(h1s);
} /*
* If we just destroyed the mux, but the connection is still alive,
* because the mux has been upgraded to H2, then from the caller's
* point of view, everything is okay, so return 0.
*/
} else if (ret == -2)
ret = 0;
return ret; return ret;
} }
@ -4585,7 +4602,7 @@ static void h1_detach(struct sedesc *sd)
* subscribe for reads waiting for new data * subscribe for reads waiting for new data
*/ */
if (unlikely(b_data(&h1c->ibuf))) { if (unlikely(b_data(&h1c->ibuf))) {
if (h1_process(h1c) == -1) if (h1_process(h1c) < 0)
goto end; goto end;
} }
else else