6940e6aa5e18edc1a638f5098f2d4cc36082e848
[openwrt-packages.git] /
1 From cf2ab4d22d977b172cf155e14060cf0f785f8404 Mon Sep 17 00:00:00 2001
2 From: Willy Tarreau <w@1wt.eu>
3 Date: Wed, 28 Mar 2018 11:29:04 +0200
4 Subject: [PATCH] BUG/MAJOR: h2: remove orphaned streams from the send list
5  before closing
6
7 Several people reported very strange occasional crashes when using H2.
8 Every time it appeared that either an h2s or a task was corrupted. The
9 outcome is that a missing LIST_DEL() when removing an orphaned stream
10 from the list in h2_wake_some_streams() can cause this stream to
11 remain present in the send list after it was freed. This may happen
12 when receiving a GOAWAY frame for example. In the mean time the send
13 list may be processed due to pending streams, and the just released
14 stream is still found. If due to a buffer full condition we left the
15 h2_process_demux() loop before being able to process the pending
16 stream, the pool entry may be reassigned somewhere else. Either another
17 h2 connection will get it, or a task, since they are the same size and
18 are shared. Then upon next pass in h2_process_mux(), the stream is
19 processed again. Either it crashes here due to modifications, or the
20 contents are harmless to it and its last changes affect the other object
21 reasigned to this area (typically a struct task). In the case of a
22 collision with struct task, the LIST_DEL operation performed on h2s
23 corrupts the task's wait queue's leaf_p pointer, thus all the wait
24 queue's structure.
25
26 The fix consists in always performing the LIST_DEL in h2s_detach().
27 It will also make h2s_stream_new() more robust against a possible
28 future situation where stream_create_from_cs() could have sent data
29 before failing.
30
31 Many thanks to all the reporters who provided extremely valuable
32 information, traces and/or cores, namely Thierry Fournier, Yves Lafon,
33 Holger Amann, Peter Lindegaard Hansen, and discourse user "slawekc".
34
35 This fix must be backported to 1.8. It is probably better to also
36 backport the following code cleanups with it as well to limit the
37 divergence between master and 1.8-stable :
38
39   00dd078 CLEANUP: h2: rename misleading h2c_stream_close() to h2s_close()
40   0a10de6 MINOR: h2: provide and use h2s_detach() and h2s_free()
41
42 (cherry picked from commit 4a333d3d53af786fe09df2f83b4e5db38cfef004)
43 Signed-off-by: Willy Tarreau <w@1wt.eu>
44 ---
45  src/mux_h2.c |    3 +++
46  1 file changed, 3 insertions(+)
47
48 diff --git a/src/mux_h2.c b/src/mux_h2.c
49 index ff1de8c..ac5e34f 100644
50 --- a/src/mux_h2.c
51 +++ b/src/mux_h2.c
52 @@ -645,6 +645,8 @@ static inline void h2s_close(struct h2s *h2s)
53  static void h2s_detach(struct h2s *h2s)
54  {
55         h2s_close(h2s);
56 +       LIST_DEL(&h2s->list);
57 +       LIST_INIT(&h2s->list);
58         eb32_delete(&h2s->by_id);
59  }
60  
61 @@ -2495,6 +2497,7 @@ static void h2_detach(struct conn_stream *cs)
62  
63         /* the stream could be in the send list */
64         LIST_DEL(&h2s->list);
65 +       LIST_INIT(&h2s->list);
66  
67         if ((h2c->flags & H2_CF_DEM_BLOCK_ANY && h2s->id == h2c->dsi) ||
68             (h2c->flags & H2_CF_MUX_BLOCK_ANY && h2s->id == h2c->msi)) {
69 -- 
70 1.7.10.4
71
git clone https://git.99rst.org/PROJECT