From 17d73b0dc19fb82852b518bc1c9678479aa02668 Mon Sep 17 00:00:00 2001 From: Christian Lachner Date: Fri, 30 Mar 2018 11:10:46 +0200 Subject: [PATCH] haproxy: Update MEDIUM+ patches for HAProxy v1.8.5 - Add new MEDIUM+ patches (see https://www.haproxy.org/bugs/bugs-1.8.5.html) - Raise patch-level to 02 Signed-off-by: Christian Lachner --- net/haproxy/Makefile | 2 +- ...eading-h2c_stream_close-to-h2s_close.patch | 167 ++++++++++++++++++ ...vide-and-use-h2s_detach-and-h2s_free.patch | 109 ++++++++++++ ...ms-from-the-send-list-before-closing.patch | 71 ++++++++ ...the-task-outside-of-the-task-handler.patch | 91 ++++++++++ ...-on-detach-if-connection-is-in-error.patch | 39 ++++ 6 files changed, 478 insertions(+), 1 deletion(-) create mode 100755 net/haproxy/patches/0001-CLEANUP-h2-rename-misleading-h2c_stream_close-to-h2s_close.patch create mode 100755 net/haproxy/patches/0002-MINOR-h2-provide-and-use-h2s_detach-and-h2s_free.patch create mode 100755 net/haproxy/patches/0003-BUG-MAJOR-h2-remove-orphaned-streams-from-the-send-list-before-closing.patch create mode 100755 net/haproxy/patches/0004-BUG-MEDIUM-h2-threads-never-release-the-task-outside-of-the-task-handler.patch create mode 100755 net/haproxy/patches/0005-BUG-MEDIUM-h2-dont-consider-pending-data-on-detach-if-connection-is-in-error.patch diff --git a/net/haproxy/Makefile b/net/haproxy/Makefile index b975fe708..5ee0f9eb6 100644 --- a/net/haproxy/Makefile +++ b/net/haproxy/Makefile @@ -10,7 +10,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=haproxy PKG_VERSION:=1.8.5 -PKG_RELEASE:=01 +PKG_RELEASE:=02 PKG_SOURCE:=haproxy-$(PKG_VERSION).tar.gz PKG_SOURCE_URL:=https://www.haproxy.org/download/1.8/src/ diff --git a/net/haproxy/patches/0001-CLEANUP-h2-rename-misleading-h2c_stream_close-to-h2s_close.patch b/net/haproxy/patches/0001-CLEANUP-h2-rename-misleading-h2c_stream_close-to-h2s_close.patch new file mode 100755 index 000000000..87e3e49c4 --- /dev/null +++ b/net/haproxy/patches/0001-CLEANUP-h2-rename-misleading-h2c_stream_close-to-h2s_close.patch @@ -0,0 +1,167 @@ +From 27b2c5ead5cf85626d4169ab46b3246d65033b58 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Thu, 1 Mar 2018 16:31:34 +0100 +Subject: [PATCH] CLEANUP: h2: rename misleading h2c_stream_close() to + h2s_close() + +This function takes an h2c and an h2s but it never uses the h2c, which +is a bit confusing at some places in the code. Let's make it clear that +it only operates on the h2s instead by renaming it and removing the +unused h2c argument. + +(cherry picked from commit 00dd07895a6ee856c811c6d60a8e3d4c7d973c63) +Signed-off-by: Willy Tarreau +--- + src/mux_h2.c | 37 ++++++++++++++++++------------------- + 1 file changed, 18 insertions(+), 19 deletions(-) + +diff --git a/src/mux_h2.c b/src/mux_h2.c +index bb0a3e3..0bb79a4 100644 +--- a/src/mux_h2.c ++++ b/src/mux_h2.c +@@ -630,12 +630,11 @@ static inline __maybe_unused int h2_get_frame_hdr(struct buffer *b, struct h2_fh + return ret; + } + +-/* marks stream as CLOSED for connection and decrement the number +- * of active streams for this connection if the stream was not yet closed. +- * Please use this exclusively before closing a stream to ensure stream count +- * is well maintained. ++/* marks stream as CLOSED and decrement the number of active streams for ++ * its connection if the stream was not yet closed. Please use this exclusively ++ * before closing a stream to ensure stream count is well maintained. + */ +-static inline void h2c_stream_close(struct h2c *h2c, struct h2s *h2s) ++static inline void h2s_close(struct h2s *h2s) + { + if (h2s->st != H2_SS_CLOSED) + h2s->h2c->nb_streams--; +@@ -924,7 +923,7 @@ static int h2s_send_rst_stream(struct h2c *h2c, struct h2s *h2s) + + ignore: + h2s->flags |= H2_SF_RST_SENT; +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + return ret; + } + +@@ -988,7 +987,7 @@ static int h2c_send_rst_stream(struct h2c *h2c, struct h2s *h2s) + ignore: + if (h2s->st > H2_SS_IDLE && h2s->st < H2_SS_CLOSED) { + h2s->flags |= H2_SF_RST_SENT; +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + } + + return ret; +@@ -1066,7 +1065,7 @@ static void h2_wake_some_streams(struct h2c *h2c, int last, uint32_t flags) + + if (!h2s->cs) { + /* this stream was already orphaned */ +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + eb32_delete(&h2s->by_id); + pool_free(pool_head_h2s, h2s); + continue; +@@ -1084,7 +1083,7 @@ static void h2_wake_some_streams(struct h2c *h2c, int last, uint32_t flags) + else if (flags & CS_FL_EOS && h2s->st == H2_SS_OPEN) + h2s->st = H2_SS_HREM; + else if (flags & CS_FL_EOS && h2s->st == H2_SS_HLOC) +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + } + } + +@@ -1551,7 +1550,7 @@ static int h2c_handle_rst_stream(struct h2c *h2c, struct h2s *h2s) + return 1; + + h2s->errcode = h2_get_n32(h2c->dbuf, 0); +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + + if (h2s->cs) { + h2s->cs->flags |= CS_FL_EOS | CS_FL_ERROR; +@@ -2099,7 +2098,7 @@ static int h2_process_mux(struct h2c *h2c) + h2s->cs->flags &= ~CS_FL_DATA_WR_ENA; + else { + /* just sent the last frame for this orphaned stream */ +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + eb32_delete(&h2s->by_id); + pool_free(pool_head_h2s, h2s); + } +@@ -2142,7 +2141,7 @@ static int h2_process_mux(struct h2c *h2c) + h2s->cs->flags &= ~CS_FL_DATA_WR_ENA; + else { + /* just sent the last frame for this orphaned stream */ +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + eb32_delete(&h2s->by_id); + pool_free(pool_head_h2s, h2s); + } +@@ -2501,7 +2500,7 @@ static void h2_detach(struct conn_stream *cs) + + if (h2s->by_id.node.leaf_p) { + /* h2s still attached to the h2c */ +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + eb32_delete(&h2s->by_id); + + /* We don't want to close right now unless we're removing the +@@ -2557,7 +2556,7 @@ static void h2_shutr(struct conn_stream *cs, enum cs_shr_mode mode) + if (h2s->h2c->mbuf->o && !(cs->conn->flags & CO_FL_XPRT_WR_ENA)) + conn_xprt_want_send(cs->conn); + +- h2c_stream_close(h2s->h2c, h2s); ++ h2s_close(h2s); + } + + static void h2_shutw(struct conn_stream *cs, enum cs_shw_mode mode) +@@ -2575,7 +2574,7 @@ static void h2_shutw(struct conn_stream *cs, enum cs_shw_mode mode) + return; + + if (h2s->st == H2_SS_HREM) +- h2c_stream_close(h2s->h2c, h2s); ++ h2s_close(h2s); + else + h2s->st = H2_SS_HLOC; + } else { +@@ -2593,7 +2592,7 @@ static void h2_shutw(struct conn_stream *cs, enum cs_shw_mode mode) + h2c_send_goaway_error(h2s->h2c, h2s) <= 0) + return; + +- h2c_stream_close(h2s->h2c, h2s); ++ h2s_close(h2s); + } + + if (h2s->h2c->mbuf->o && !(cs->conn->flags & CO_FL_XPRT_WR_ENA)) +@@ -3049,7 +3048,7 @@ static int h2s_frt_make_resp_headers(struct h2s *h2s, struct buffer *buf) + if (h2s->st == H2_SS_OPEN) + h2s->st = H2_SS_HLOC; + else +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + } + else if (h1m->status >= 100 && h1m->status < 200) { + /* we'll let the caller check if it has more headers to send */ +@@ -3291,7 +3290,7 @@ static int h2s_frt_make_resp_data(struct h2s *h2s, struct buffer *buf) + if (h2s->st == H2_SS_OPEN) + h2s->st = H2_SS_HLOC; + else +- h2c_stream_close(h2c, h2s); ++ h2s_close(h2s); + + if (!(h1m->flags & H1_MF_CHNK)) { + // trim any possibly pending data (eg: inconsistent content-length) +@@ -3364,7 +3363,7 @@ static int h2_snd_buf(struct conn_stream *cs, struct buffer *buf, int flags) + if (h2s->st == H2_SS_ERROR || h2s->flags & H2_SF_RST_RCVD) { + cs->flags |= CS_FL_ERROR; + if (h2s_send_rst_stream(h2s->h2c, h2s) > 0) +- h2c_stream_close(h2s->h2c, h2s); ++ h2s_close(h2s); + } + + if (h2s->flags & H2_SF_BLK_SFCTL) { +-- +1.7.10.4 + diff --git a/net/haproxy/patches/0002-MINOR-h2-provide-and-use-h2s_detach-and-h2s_free.patch b/net/haproxy/patches/0002-MINOR-h2-provide-and-use-h2s_detach-and-h2s_free.patch new file mode 100755 index 000000000..17d085983 --- /dev/null +++ b/net/haproxy/patches/0002-MINOR-h2-provide-and-use-h2s_detach-and-h2s_free.patch @@ -0,0 +1,109 @@ +From 518db3f8602fae9caa816ec373855cf0f8c6c45d Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Thu, 1 Mar 2018 16:27:53 +0100 +Subject: [PATCH] MINOR: h2: provide and use h2s_detach() and h2s_free() + +These ones save us from open-coding the cleanup functions on each and +every error path. The code was updated to use them with no functional +change. + +(cherry picked from commit 0a10de606685ed4e65d4cc84237c6a09dd6fe27c) +Signed-off-by: Willy Tarreau +--- + src/mux_h2.c | 38 +++++++++++++++++++++++--------------- + 1 file changed, 23 insertions(+), 15 deletions(-) + +diff --git a/src/mux_h2.c b/src/mux_h2.c +index 0bb79a4..ff1de8c 100644 +--- a/src/mux_h2.c ++++ b/src/mux_h2.c +@@ -641,6 +641,19 @@ static inline void h2s_close(struct h2s *h2s) + h2s->st = H2_SS_CLOSED; + } + ++/* detaches an H2 stream from its H2C. */ ++static void h2s_detach(struct h2s *h2s) ++{ ++ h2s_close(h2s); ++ eb32_delete(&h2s->by_id); ++} ++ ++/* releases an H2 stream back to the pool, and detaches it from the h2c. */ ++static void h2s_free(struct h2s *h2s) ++{ ++ pool_free(pool_head_h2s, h2s); ++} ++ + /* creates a new stream on the h2c connection and returns it, or NULL in + * case of memory allocation error. + */ +@@ -685,9 +698,8 @@ static struct h2s *h2c_stream_new(struct h2c *h2c, int id) + out_free_cs: + cs_free(cs); + out_close: +- h2c->nb_streams--; +- eb32_delete(&h2s->by_id); +- pool_free(pool_head_h2s, h2s); ++ h2s_detach(h2s); ++ h2s_free(h2s); + h2s = NULL; + out: + return h2s; +@@ -1065,9 +1077,8 @@ static void h2_wake_some_streams(struct h2c *h2c, int last, uint32_t flags) + + if (!h2s->cs) { + /* this stream was already orphaned */ +- h2s_close(h2s); +- eb32_delete(&h2s->by_id); +- pool_free(pool_head_h2s, h2s); ++ h2s_detach(h2s); ++ h2s_free(h2s); + continue; + } + +@@ -2098,9 +2109,8 @@ static int h2_process_mux(struct h2c *h2c) + h2s->cs->flags &= ~CS_FL_DATA_WR_ENA; + else { + /* just sent the last frame for this orphaned stream */ +- h2s_close(h2s); +- eb32_delete(&h2s->by_id); +- pool_free(pool_head_h2s, h2s); ++ h2s_detach(h2s); ++ h2s_free(h2s); + } + } + } +@@ -2141,9 +2151,8 @@ static int h2_process_mux(struct h2c *h2c) + h2s->cs->flags &= ~CS_FL_DATA_WR_ENA; + else { + /* just sent the last frame for this orphaned stream */ +- h2s_close(h2s); +- eb32_delete(&h2s->by_id); +- pool_free(pool_head_h2s, h2s); ++ h2s_detach(h2s); ++ h2s_free(h2s); + } + } + } +@@ -2500,8 +2509,7 @@ static void h2_detach(struct conn_stream *cs) + + if (h2s->by_id.node.leaf_p) { + /* h2s still attached to the h2c */ +- h2s_close(h2s); +- eb32_delete(&h2s->by_id); ++ h2s_detach(h2s); + + /* We don't want to close right now unless we're removing the + * last stream, and either the connection is in error, or it +@@ -2526,7 +2534,7 @@ static void h2_detach(struct conn_stream *cs) + h2c->task->expire = TICK_ETERNITY; + } + } +- pool_free(pool_head_h2s, h2s); ++ h2s_free(h2s); + } + + static void h2_shutr(struct conn_stream *cs, enum cs_shr_mode mode) +-- +1.7.10.4 + diff --git a/net/haproxy/patches/0003-BUG-MAJOR-h2-remove-orphaned-streams-from-the-send-list-before-closing.patch b/net/haproxy/patches/0003-BUG-MAJOR-h2-remove-orphaned-streams-from-the-send-list-before-closing.patch new file mode 100755 index 000000000..6940e6aa5 --- /dev/null +++ b/net/haproxy/patches/0003-BUG-MAJOR-h2-remove-orphaned-streams-from-the-send-list-before-closing.patch @@ -0,0 +1,71 @@ +From cf2ab4d22d977b172cf155e14060cf0f785f8404 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Wed, 28 Mar 2018 11:29:04 +0200 +Subject: [PATCH] BUG/MAJOR: h2: remove orphaned streams from the send list + before closing + +Several people reported very strange occasional crashes when using H2. +Every time it appeared that either an h2s or a task was corrupted. The +outcome is that a missing LIST_DEL() when removing an orphaned stream +from the list in h2_wake_some_streams() can cause this stream to +remain present in the send list after it was freed. This may happen +when receiving a GOAWAY frame for example. In the mean time the send +list may be processed due to pending streams, and the just released +stream is still found. If due to a buffer full condition we left the +h2_process_demux() loop before being able to process the pending +stream, the pool entry may be reassigned somewhere else. Either another +h2 connection will get it, or a task, since they are the same size and +are shared. Then upon next pass in h2_process_mux(), the stream is +processed again. Either it crashes here due to modifications, or the +contents are harmless to it and its last changes affect the other object +reasigned to this area (typically a struct task). In the case of a +collision with struct task, the LIST_DEL operation performed on h2s +corrupts the task's wait queue's leaf_p pointer, thus all the wait +queue's structure. + +The fix consists in always performing the LIST_DEL in h2s_detach(). +It will also make h2s_stream_new() more robust against a possible +future situation where stream_create_from_cs() could have sent data +before failing. + +Many thanks to all the reporters who provided extremely valuable +information, traces and/or cores, namely Thierry Fournier, Yves Lafon, +Holger Amann, Peter Lindegaard Hansen, and discourse user "slawekc". + +This fix must be backported to 1.8. It is probably better to also +backport the following code cleanups with it as well to limit the +divergence between master and 1.8-stable : + + 00dd078 CLEANUP: h2: rename misleading h2c_stream_close() to h2s_close() + 0a10de6 MINOR: h2: provide and use h2s_detach() and h2s_free() + +(cherry picked from commit 4a333d3d53af786fe09df2f83b4e5db38cfef004) +Signed-off-by: Willy Tarreau +--- + src/mux_h2.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/src/mux_h2.c b/src/mux_h2.c +index ff1de8c..ac5e34f 100644 +--- a/src/mux_h2.c ++++ b/src/mux_h2.c +@@ -645,6 +645,8 @@ static inline void h2s_close(struct h2s *h2s) + static void h2s_detach(struct h2s *h2s) + { + h2s_close(h2s); ++ LIST_DEL(&h2s->list); ++ LIST_INIT(&h2s->list); + eb32_delete(&h2s->by_id); + } + +@@ -2495,6 +2497,7 @@ static void h2_detach(struct conn_stream *cs) + + /* the stream could be in the send list */ + LIST_DEL(&h2s->list); ++ LIST_INIT(&h2s->list); + + if ((h2c->flags & H2_CF_DEM_BLOCK_ANY && h2s->id == h2c->dsi) || + (h2c->flags & H2_CF_MUX_BLOCK_ANY && h2s->id == h2c->msi)) { +-- +1.7.10.4 + diff --git a/net/haproxy/patches/0004-BUG-MEDIUM-h2-threads-never-release-the-task-outside-of-the-task-handler.patch b/net/haproxy/patches/0004-BUG-MEDIUM-h2-threads-never-release-the-task-outside-of-the-task-handler.patch new file mode 100755 index 000000000..abd3e7c0d --- /dev/null +++ b/net/haproxy/patches/0004-BUG-MEDIUM-h2-threads-never-release-the-task-outside-of-the-task-handler.patch @@ -0,0 +1,91 @@ +From 4f2bd42ed3870dbaf143701f0cfbd64966d44252 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Thu, 29 Mar 2018 15:22:59 +0200 +Subject: [PATCH] BUG/MEDIUM: h2/threads: never release the task outside of + the task handler + +Currently, h2_release() will release all resources assigned to the h2 +connection, including the timeout task if any. But since the multi-threaded +scheduler, the timeout task could very well be queued in the thread-local +list of running tasks without any way to remove it, so task_delete() will +have no effect and task_free() will cause this undefined object to be +dereferenced. + +In order to prevent this from happening, we never release the task in +h2_release(), instead we wake it up after marking its context NULL so that +the task handler can release the task. + +Future improvements could consist in modifying the scheduler so that a +task_wakeup() has to be done on any task having to be killed, letting +the scheduler take care of it. + +This fix must be backported to 1.8. This bug was apparently not reported +so far. + +(cherry picked from commit 0975f11d554baf30602ce4be3faf0b9741711a80) +Signed-off-by: Willy Tarreau +--- + src/mux_h2.c | 30 +++++++++++++++++------------- + 1 file changed, 17 insertions(+), 13 deletions(-) + +diff --git a/src/mux_h2.c b/src/mux_h2.c +index 3c076d2..92fae06 100644 +--- a/src/mux_h2.c ++++ b/src/mux_h2.c +@@ -484,8 +484,8 @@ static void h2_release(struct connection *conn) + HA_SPIN_UNLOCK(BUF_WQ_LOCK, &buffer_wq_lock); + + if (h2c->task) { +- task_delete(h2c->task); +- task_free(h2c->task); ++ h2c->task->context = NULL; ++ task_wakeup(h2c->task, TASK_WOKEN_OTHER); + h2c->task = NULL; + } + +@@ -2369,9 +2369,18 @@ static struct task *h2_timeout_task(struct task *t) + struct h2c *h2c = t->context; + int expired = tick_is_expired(t->expire, now_ms); + +- if (!expired) ++ if (!expired && h2c) + return t; + ++ task_delete(t); ++ task_free(t); ++ ++ if (!h2c) { ++ /* resources were already deleted */ ++ return NULL; ++ } ++ ++ h2c->task = NULL; + h2c_error(h2c, H2_ERR_NO_ERROR); + h2_wake_some_streams(h2c, 0, 0); + +@@ -2388,17 +2397,12 @@ static struct task *h2_timeout_task(struct task *t) + if (h2c->mbuf->o && !(h2c->flags & H2_CF_GOAWAY_FAILED) && conn_xprt_ready(h2c->conn)) + h2c->conn->xprt->snd_buf(h2c->conn, h2c->mbuf, 0); + +- if (!eb_is_empty(&h2c->streams_by_id)) +- goto wait; +- +- h2_release(h2c->conn); +- return NULL; ++ /* either we can release everything now or it will be done later once ++ * the last stream closes. ++ */ ++ if (eb_is_empty(&h2c->streams_by_id)) ++ h2_release(h2c->conn); + +- wait: +- /* the streams have been notified, we must let them finish and close */ +- h2c->task = NULL; +- task_delete(t); +- task_free(t); + return NULL; + } + +-- +1.7.10.4 + diff --git a/net/haproxy/patches/0005-BUG-MEDIUM-h2-dont-consider-pending-data-on-detach-if-connection-is-in-error.patch b/net/haproxy/patches/0005-BUG-MEDIUM-h2-dont-consider-pending-data-on-detach-if-connection-is-in-error.patch new file mode 100755 index 000000000..24b508b03 --- /dev/null +++ b/net/haproxy/patches/0005-BUG-MEDIUM-h2-dont-consider-pending-data-on-detach-if-connection-is-in-error.patch @@ -0,0 +1,39 @@ +From 58cef63f20cc40248cd1cd113571cae588943d06 Mon Sep 17 00:00:00 2001 +From: Willy Tarreau +Date: Thu, 29 Mar 2018 15:41:32 +0200 +Subject: [PATCH] BUG/MEDIUM: h2: don't consider pending data on detach if + connection is in error + +Interrupting an h2load test shows that some connections remain active till +the client timeout. This is due to the fact that h2_detach() immediately +returns if the h2s flags indicate that the h2s is still waiting for some +buffer room in the output mux (possibly to emit a response or to send some +window updates). If the connection is broken, these data will never leave +and must not prevent the stream from being terminated nor the connection +from being released. + +This fix must be backported to 1.8. + +(cherry picked from commit 3041fcc2fde3f3f33418c9f579b657d993b0006d) +Signed-off-by: Willy Tarreau +--- + src/mux_h2.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/mux_h2.c b/src/mux_h2.c +index 92fae06..4d30f91 100644 +--- a/src/mux_h2.c ++++ b/src/mux_h2.c +@@ -2487,7 +2487,8 @@ static void h2_detach(struct conn_stream *cs) + /* this stream may be blocked waiting for some data to leave (possibly + * an ES or RST frame), so orphan it in this case. + */ +- if (h2s->flags & (H2_SF_BLK_MBUSY | H2_SF_BLK_MROOM | H2_SF_BLK_MFCTL)) ++ if (!(cs->conn->flags & CO_FL_ERROR) && ++ (h2s->flags & (H2_SF_BLK_MBUSY | H2_SF_BLK_MROOM | H2_SF_BLK_MFCTL))) + return; + + if ((h2c->flags & H2_CF_DEM_BLOCK_ANY && h2s->id == h2c->dsi) || +-- +1.7.10.4 +