still segfault

[bastien-roucaries]

Hi,

Under debian we still get segfault with lastest version

Can you get a glimpse at http://bugs.debian.org/1126177

rouca

[mirabilos]

If necessary, I can set up a chroot with the same software versions to get a more detailled backtrace. The segfaults and core dump I get is on a prod system, which cannot access the net or dbgsym packages by itself.

But I think rouca already has some information?

[icing]

I more detailed backtrace with debug symbols would be helpful. Thanks.

[bastien-roucaries]

@mirabilos debusine has debug symbols. You could download the core and on other chroot get more information

[bastien-roucaries]

@icing previous fix and new fix deserve a CVE BTW. it is a DoS at least

[icing]

CVE for the first fix is queued at the httpd project. I don't do CVEs here.

[mirabilos]

OK, I got sufficiently similar software versions, debug symbols, and source in a chroot. Some complaints, also because it cannot find the mmapped HTML files, but sufficient for looking at.

Corresponding source is in the artifact-3070764.tar.gz file from https://debusine.debian.net/debian/developers/work-request/356251/ and binaries are at https://debusine.debian.net/debian/developers/work-request/356253/#output

I’m attaching you a “thread apply all bt full”…

log.txt.gz

… but the interesting part is perhaps this, in the thread 177034 from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1126177#45 whose systemd-coredump backtrace read…

                Stack trace of thread 177034:
                #0  0x00007f0d1b7b28d8 n/a (libapr-1.so.0 + 0x248d8)
                #1  0x00007f0d1b7d6f21 n/a (libaprutil-1.so.0 + 0xbf21)
                #2  0x00007f0d1b3bfa44 n/a (mod_http2.so + 0xaa44)
                #3  0x00007f0d1b3bfbb4 n/a (mod_http2.so + 0xabb4)
                #4  0x00007f0d1b7b059e apr_pool_destroy (libapr-1.so.0 + 0x2259e)
                #5  0x00007f0d1b7b05bd apr_pool_destroy (libapr-1.so.0 + 0x225bd)
                #6  0x00007f0d1b3ce778 n/a (mod_http2.so + 0x19778)[…]

… and which is likely the cause for the segfault:

(gdb) thread 1
[Switching to thread 1 (Thread 0x7f0ce2ffd6c0 (LWP 177034))]
#0  0x00007f0d1b7b28d8 in ?? ()
(gdb) bt
#0  0x00007f0d1b7b28d8 in ?? ()
#1  0x00007f0d1b7d6f21 in ?? ()
#2  0x00007f0d000ff6c8 in ?? ()
#3  0x00007f0d1b3bfa44 in purge_consumed_buckets (beam=beam@entry=0x7f0d007d20a0)
    at ./modules/http2/h2_bucket_beam.c:165
#4  0x00007f0d1b3bfbb4 in beam_shutdown (how=APR_SHUTDOWN_READWRITE, beam=0x7f0d007d20a0)
    at ./modules/http2/h2_bucket_beam.c:276
#5  beam_shutdown (how=APR_SHUTDOWN_READWRITE, beam=0x7f0d007d20a0) at ./modules/http2/h2_bucket_beam.c:260
#6  beam_cleanup (data=0x7f0d007d20a0) at ./modules/http2/h2_bucket_beam.c:284
#7  0x00007f0d1b7b059e in ?? ()
#8  0x0000000000000000 in ?? ()
(gdb) frame 3
#3  0x00007f0d1b3bfa44 in purge_consumed_buckets (beam=beam@entry=0x7f0d007d20a0)
    at ./modules/http2/h2_bucket_beam.c:165
165               apr_bucket_delete(b);
(gdb) print b
$1 = (apr_bucket *) 0x7f0ca149b8a8
(gdb) print b
$1 = (apr_bucket *) 0x7f0ca149b8a8
(gdb) print *b
$2 = {link = {next = 0x0, prev = 0x7f0c769b1980}, type = 0x0, length = 0, start = 0, data = 0x0, free = 0x0,
  list = 0x0}
(gdb) list
160             if(AP_BUCKET_IS_EOR(b)) {
161               APR_BUCKET_REMOVE(b);
162               H2_BLIST_INSERT_TAIL(&beam->buckets_eor, b);
163             }
164             else {
165               apr_bucket_delete(b);
166             }
167         }
168     }
169

From what I can see, apr_bucket_delete(b), via some macro indirections, dereferences b->type, which is a nil pointer.

The entire *b looks like a “stop” type indicator, all values 0, and the last in its chain. Perhaps the code inserts this somewhere, even if it is not needed because the link member can already indicate end of the chain with a nil pointer in .next?

Does this help?

[icing]

The memory of the bucket looks wrong. next and type point are never supposed to be NULL.

Since this is triggered when the connection goes down, it is the input for a HTTP/2 stream that is still open when the connection is terminated.

How do you trigger this? Do you have a reproducer?

Otherwise it would probably most efficient to make a bisect of this problem.

[mirabilos]

It happens after about 1½ days of runtime on a prod system (though before the last upgrade a bit more frequent). We have no idea when (= with which version) it started.

I’m very much not sure we can do an entire bisecting like that.

[icing]

Ok, one of those bugs. What is the lastest version you ran without those problems?

[mirabilos]

The client just one day noticed segfaults in the log, so we begun to investigate. I have no “working” version for you, as I already said.

[mirabilos]

I can see if they have other machines with the http2 module enabled (I myself don’t).

[mirabilos]

So… yes, they do. Two other machines with a two-digit figure in them, which seem to be under heavy load, and two with less load.

Mixture of bullseye and bookworm.

They all run 2.4.66-1deb11u1 or 2.4.66-1deb12u1.

One of them has a corresponding test system that has 2.4.62-1~deb12u2 and no segfaults… but then, no load (requests) either, I suppose.

I cannot say for sure, but it’s possible that .66 introduced this.

[icing]

Ok, while I have great respect for our debian fellows, can you build https://github.com/icing/mod_h2/releases/tag/v2.0.37 and run that in your server? I just want to make sure that the issue happens in the latest release.

The thing is, the segfault observed is very much what the fix in v2.0.37 prevents.

[bastien-roucaries]

here is the diff, we used apache/httpd@542e0da as patch
minius is our version
plus is last mod http2 version

diff --git a/modules/http2/h2_c1.c b/modules/http2/h2_c1.c
index 626e665b..f4604a47 100644

--- a/modules/http2/h2_mplx.c
+++ b/modules/http2/h2_mplx.c
@@ -398,10 +398,11 @@ apr_status_t h2_mplx_c1_streams_do(h2_mplx *m, h2_mplx_stream_cb *cb, void *ctx)
 {
     stream_iter_ctx_t x;
     
-    H2_MPLX_ENTER(m);
-
     x.cb = cb;
     x.ctx = ctx;
+
+    H2_MPLX_ENTER(m);
+
     h2_ihash_iter(m->streams, m_stream_iter_wrap, &x);
         
     H2_MPLX_LEAVE(m);
@@ -1033,7 +1034,8 @@ static void s_c2_done(h2_mplx *m, conn_rec *c2, h2_conn_ctx_t *conn_ctx)
         int i;
 
         for (i = 0; i < m->spurge->nelts; ++i) {
-            if (stream == APR_ARRAY_IDX(m->spurge, i, h2_stream*)) {
+            stream = APR_ARRAY_IDX(m->spurge, i, h2_stream*);
+            if (stream && (stream->id == conn_ctx->stream_id)) {
                 ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c2,
                               H2_STRM_LOG(APLOGNO(03517), stream, "already in spurge"));
                 ap_assert("stream should not be in spurge" == NULL);

Here it look like a miss merge
https://github.com/apache/httpd/blob/2565ef17f5df54cebcecb19f45ec92f6520f0067/modules/http2/h2_mplx.c#L1036

They are few other occurence
see https://salsa.debian.org/rouca/apache2/-/tree/patch-queue/bookworm-security?ref_type=heads

[mirabilos]

bastien-roucaries dixit:

bastien-roucaries left a comment (icing/mod_h2#313) here is the diff, we used apache/httpd@542e0da as patch - is our version + is lat mod http2 version

Ah, so I can just apply the diff from your comment to the package, rebuild and test? I’d prefer that over trying to figure out an upstream build system.