From ff7da85109221a5c1e49d2ee3279a9a046d26959 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Fri, 22 Aug 2025 17:12:56 +0100 Subject: [PATCH 1/2] http: compression: Don't set buf->parent When doing some testing I was noticing when using brotli & zstd compression on application responses we were regularly (but not always) getting segfaults with "corrupted double-linked list" being logged from malloc(3) when we were freeing memory via nxt_mp_destroy() when doing nxt_router_http_request_release(). E.g. #5 0x00007f6eeb4f11f5 in malloc_printerr ( str=str@entry=0x7f6eeb625178 "corrupted double-linked list") at malloc.c:5829 #6 0x00007f6eeb4f1d0c in unlink_chunk (p=, av=0x7f6edc000030) at malloc.c:1619 #7 0x00007f6eeb4f1f78 in _int_free_create_chunk (av=av@entry=0x7f6edc000030, p=p@entry=0x7f6edc008ea0, size=size@entry=4192, nextchunk=, nextsize=75520) at malloc.c:4763 #8 0x00007f6eeb4f352e in _int_free_merge_chunk (av=av@entry=0x7f6edc000030, p=0x7f6edc008ea0, size=4192) at malloc.c:4742 #9 0x00007f6eeb4f36e4 in _int_free_chunk (av=0x7f6edc000030, p=, size=, have_lock=, have_lock@entry=0) at malloc.c:4667 #10 0x00007f6eeb4f6512 in _int_free (av=, p=, have_lock=0) at malloc.c:4699 #11 __GI___libc_free (mem=) at malloc.c:3476 #12 0x000000000040d66a in nxt_mp_destroy (mp=0x7f6edc003790) at src/nxt_mp.c:342 #13 0x000000000040d5a4 in nxt_mp_release (mp=0x7f6edc003790) at src/nxt_mp.c:303 #14 0x000000000042f9de in nxt_router_http_request_release (task=0x24cb8c10, obj=0x7f6edc003990, data=0x0) at src/nxt_router.c:5799 Interestingly gzip compression never seemed to trigger this... Also when doing brotli compression for example, I could prevent this from happening by simply commenting out BrotliEncoderDestroyInstance(brotli); in src/nxt_brotli.c::nxt_brotli_compress() Running under libasan showed the following ==281177==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7b94031e90f0 at pc 0x000000422b37 bp 0x7b640027c820 sp 0x7b640027c818 READ of size 4 at 0x7b94031e90f0 thread T2 #0 0x000000422b36 in nxt_buf_parent_completion src/nxt_buf.c:229 #1 0x000000422d5e in nxt_buf_ts_completion src/nxt_buf.c:294 #2 0x000000428fa0 in nxt_event_engine_start src/nxt_event_engine.c:542 #3 0x0000004423de in nxt_router_thread_start src/nxt_router.c:3727 #4 0x00000042497b in nxt_thread_trampoline src/nxt_thread.c:126 #5 0x7f6404828ee5 in asan_thread_start(void*) (/lib64/libasan.so.8+0x28ee5) (BuildId: 10b8ccd49f75c21babf1d7abe51bb63589d8471f) #6 0x7f640446f153 in start_thread (/lib64/libc.so.6+0x71153) (BuildId: 126a08bf502f4950b215dc773e52df8dcf50c393) #7 0x7f64044f1cab in __clone3 (/lib64/libc.so.6+0xf3cab) (BuildId: 126a08bf502f4950b215dc773e52df8dcf50c393) 0x7b94031e90f0 is located 8 bytes after 24-byte region [0x7b94031e90d0,0x7b94031e90e8) allocated by thread T2 here: #0 0x7f64048e6f2b in malloc (/lib64/libasan.so.8+0xe6f2b) (BuildId: 10b8ccd49f75c21babf1d7abe51bb63589d8471f) #1 0x000000401b10 in nxt_malloc src/nxt_malloc.c:35 #2 0x000000401bd8 in nxt_zalloc src/nxt_malloc.c:54 #3 0x000000410035 in nxt_port_incoming_port_mmap src/nxt_port_memory.c:247 #4 0x0000004162fa in nxt_port_mmap_handler src/nxt_port.c:366 #5 0x000000415000 in nxt_port_handler src/nxt_port.c:184 #6 0x00000040a761 in nxt_port_read_msg_process src/nxt_port_socket.c:1271 #7 0x00000040d596 in nxt_port_queue_read_handler src/nxt_port_socket.c:997 #8 0x000000428fa0 in nxt_event_engine_start src/nxt_event_engine.c:542 #9 0x0000004423de in nxt_router_thread_start src/nxt_router.c:3727 #10 0x00000042497b in nxt_thread_trampoline src/nxt_thread.c:126 #11 0x7f6404828ee5 in asan_thread_start(void*) (/lib64/libasan.so.8+0x28ee5) (BuildId: 10b8ccd49f75c21babf1d7abe51bb63589d8471f) Thread T2 created by T0 here: #0 0x7f64048de492 in pthread_create (/lib64/libasan.so.8+0xde492) (BuildId: 10b8ccd49f75c21babf1d7abe51bb63589d8471f) #1 0x00000042468b in nxt_thread_create src/nxt_thread.c:85 #2 0x00000044b799 in nxt_router_thread_create src/nxt_router.c:3575 #3 0x00000044b799 in nxt_router_threads_create src/nxt_router.c:3543 #4 0x00000044b799 in nxt_router_conf_apply src/nxt_router.c:1271 #5 0x000000428fa0 in nxt_event_engine_start src/nxt_event_engine.c:542 #6 0x00000040140d in main src/nxt_main.c:35 #7 0x7f6404401574 in __libc_start_call_main (/lib64/libc.so.6+0x3574) (BuildId: 126a08bf502f4950b215dc773e52df8dcf50c393) #8 0x7f6404401627 in __libc_start_main_alias_1 (/lib64/libc.so.6+0x3627) (BuildId: 126a08bf502f4950b215dc773e52df8dcf50c393) #9 0x000000401264 in _start (/opt/unit/sbin/unitd+0x401264) (BuildId: c05bd11884a7315b24ec2abf762c4f283def6fea) SUMMARY: AddressSanitizer: heap-buffer-overflow src/nxt_buf.c:229 in nxt_buf_parent_completion Shadow bytes around the buggy address: 0x7b94031e8e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7b94031e8e80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7b94031e8f00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7b94031e8f80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7b94031e9000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa 00 00 =>0x7b94031e9080: 00 fa fa fa 00 00 00 05 fa fa 00 00 00 fa[fa]fa 0x7b94031e9100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7b94031e9180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7b94031e9200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7b94031e9280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7b94031e9300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==281177==ABORTING "SUMMARY: AddressSanitizer: heap-buffer-overflow src/nxt_buf.c:229 in nxt_buf_parent_completion" Gave some clue. It seems that setting buf->parent on the last buffer triggers this. If we don't set it on the last buffer, everything works fine and no heap-overflow detected. Everything seems to also work fine if we simply don't set it all. So lets do that. Signed-off-by: Andrew Clayton --- src/nxt_http_compression.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/nxt_http_compression.c b/src/nxt_http_compression.c index f9a94d055..079eceef2 100644 --- a/src/nxt_http_compression.c +++ b/src/nxt_http_compression.c @@ -195,7 +195,6 @@ nxt_http_comp_compress_app_response(nxt_task_t *task, nxt_http_request_t *r, } buf->data = (*b)->data; - buf->parent = (*b)->parent; last = ctx->clen_sent + in_len == ctx->resp_clen; From a76c8a7b944e43f56ddbcac0b49832a0d7f91d8b Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Fri, 22 Aug 2025 17:47:21 +0100 Subject: [PATCH 2/2] http: compression: brotli: Don't leak memory on error Signed-off-by: Andrew Clayton --- src/nxt_brotli.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/nxt_brotli.c b/src/nxt_brotli.c index 773b8b6f2..c69d977e0 100644 --- a/src/nxt_brotli.c +++ b/src/nxt_brotli.c @@ -46,14 +46,14 @@ nxt_brotli_compress(nxt_http_comp_compressor_ctx_t *ctx, const uint8_t *in_buf, &in_len, &in_buf, &out_bytes, &out_buf, NULL); if (!ok) { - return -1; + goto out_err_free; } ok = BrotliEncoderCompressStream(brotli, BROTLI_OPERATION_FLUSH, &in_len, &in_buf, &out_bytes, &out_buf, NULL); if (!ok) { - return -1; + goto out_err_free; } if (last) { @@ -61,13 +61,18 @@ nxt_brotli_compress(nxt_http_comp_compressor_ctx_t *ctx, const uint8_t *in_buf, &in_len, &in_buf, &out_bytes, &out_buf, NULL); if (!ok) { - return -1; + goto out_err_free; } BrotliEncoderDestroyInstance(brotli); } return out_len - out_bytes; + +out_err_free: + BrotliEncoderDestroyInstance(brotli); + + return -1; }