Skip to content
This repository was archived by the owner on Oct 8, 2025. It is now read-only.

A couple of HTTP compression fixes #1643

Merged
merged 2 commits into from
Aug 25, 2025
Merged

A couple of HTTP compression fixes #1643

merged 2 commits into from
Aug 25, 2025

Conversation

ac000
Copy link
Member

@ac000 ac000 commented Aug 22, 2025

  • http: compression: Don't set buf->parent

This fixes a segfault due to a heap-buffer-overflow when compressing
application responses.

  • http: compression: brotli: Don't leak memory on error

This avoids leaking memory in the brotli compressor upon errors.

The following changes since commit eec23cd1fdc96b96582673a993861f484cec4517:

  java: Update classgraph to the latest version (2025-08-20 18:42:01 +0100)

are available in the Git repository at:

  https://github.com/ac000/unit.git compr

for you to fetch changes up to c23db48ce307da507cce85158625b08d4386ed49:

  http: compression: brotli: Don't leak memory on error (2025-08-22 17:47:21 +0100)

----------------------------------------------------------------
Andrew Clayton (2):
      http: compression: Don't set buf->parent
      http: compression: brotli: Don't leak memory on error

 src/nxt_brotli.c           | 11 ++++++++---
 src/nxt_http_compression.c |  1 -
 2 files changed, 8 insertions(+), 4 deletions(-)

@ac000 ac000 marked this pull request as ready for review August 25, 2025 02:41
@ac000 ac000 requested a review from hongzhidao August 25, 2025 02:42
ac000 added 2 commits August 25, 2025 16:16
@ac000
http: compression: Don't set buf->parent
ff7da85 
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.

  nginx#5  0x00007f6eeb4f11f5 in malloc_printerr (
      str=str@entry=0x7f6eeb625178 "corrupted double-linked list")
      at malloc.c:5829
  nginx#6  0x00007f6eeb4f1d0c in unlink_chunk (p=<optimized out>, av=0x7f6edc000030)
      at malloc.c:1619
  nginx#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
  nginx#8  0x00007f6eeb4f352e in _int_free_merge_chunk (av=av@entry=0x7f6edc000030,
      p=0x7f6edc008ea0, size=4192) at malloc.c:4742
  nginx#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
  nginx#10 0x00007f6eeb4f6512 in _int_free (av=<optimized out>, p=<optimized out>,
      have_lock=0) at malloc.c:4699
  nginx#11 __GI___libc_free (mem=<optimized out>) at malloc.c:3476
  nginx#12 0x000000000040d66a in nxt_mp_destroy (mp=0x7f6edc003790)
      at src/nxt_mp.c:342
  nginx#13 0x000000000040d5a4 in nxt_mp_release (mp=0x7f6edc003790)
      at src/nxt_mp.c:303
  nginx#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
      nginx#4 0x00000042497b in nxt_thread_trampoline src/nxt_thread.c:126
      nginx#5 0x7f6404828ee5 in asan_thread_start(void*) (/lib64/libasan.so.8+0x28ee5) (BuildId: 10b8ccd49f75c21babf1d7abe51bb63589d8471f)
      nginx#6 0x7f640446f153 in start_thread (/lib64/libc.so.6+0x71153) (BuildId: 126a08bf502f4950b215dc773e52df8dcf50c393)
      nginx#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
      nginx#4 0x0000004162fa in nxt_port_mmap_handler src/nxt_port.c:366
      nginx#5 0x000000415000 in nxt_port_handler src/nxt_port.c:184
      nginx#6 0x00000040a761 in nxt_port_read_msg_process src/nxt_port_socket.c:1271
      nginx#7 0x00000040d596 in nxt_port_queue_read_handler src/nxt_port_socket.c:997
      nginx#8 0x000000428fa0 in nxt_event_engine_start src/nxt_event_engine.c:542
      nginx#9 0x0000004423de in nxt_router_thread_start src/nxt_router.c:3727
      nginx#10 0x00000042497b in nxt_thread_trampoline src/nxt_thread.c:126
      nginx#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
      nginx#4 0x00000044b799 in nxt_router_conf_apply src/nxt_router.c:1271
      nginx#5 0x000000428fa0 in nxt_event_engine_start src/nxt_event_engine.c:542
      nginx#6 0x00000040140d in main src/nxt_main.c:35
      nginx#7 0x7f6404401574 in __libc_start_call_main (/lib64/libc.so.6+0x3574) (BuildId: 126a08bf502f4950b215dc773e52df8dcf50c393)
      nginx#8 0x7f6404401627 in __libc_start_main_alias_1 (/lib64/libc.so.6+0x3627) (BuildId: 126a08bf502f4950b215dc773e52df8dcf50c393)
      nginx#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>
@ac000
http: compression: brotli: Don't leak memory on error
a76c8a7 
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
@ac000
Copy link
Member Author

ac000 commented Aug 25, 2025

Rebase with master.

@ac000 ac000 merged commit a76c8a7 into nginx:master Aug 25, 2025
23 of 24 checks passed
@ac000 ac000 deleted the compr branch August 25, 2025 15:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@hongzhidao hongzhidao hongzhidao approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ac000 @hongzhidao