-
Notifications
You must be signed in to change notification settings - Fork 357
Commit ff7da85
committed
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=<optimized out>, 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=<optimized out>,
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=<optimized out>, size=<optimized out>, have_lock=<optimized out>,
have_lock@entry=0) at malloc.c:4667
#10 0x00007f6eeb4f6512 in _int_free (av=<optimized out>, p=<optimized out>,
have_lock=0) at malloc.c:4699
#11 __GI___libc_free (mem=<optimized out>) 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 <a.clayton@nginx.com>1 parent 440e755 commit ff7da85Copy full SHA for ff7da85
File tree
Expand file treeCollapse file tree
1 file changed
+0
-1
lines changedFilter options
- src
Expand file treeCollapse file tree
1 file changed
+0
-1
lines changedCollapse file: src/nxt_http_compression.c
-1Lines changed: 0 additions & 1 deletion
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
195 | 195 |
| |
196 | 196 |
| |
197 | 197 |
| |
198 |
| - | |
199 | 198 |
| |
200 | 199 |
| |
201 | 200 |
| |
|
0 commit comments