Skip to content

bugfix: prevent SIGSEGV in receiveuntil __gc on aborted multipart upload#2503

Closed
climagabriel wants to merge 1 commit into
openresty:masterfrom
climagabriel:fix-segv-receiveuntil-gc-aborted-multipart
Closed

bugfix: prevent SIGSEGV in receiveuntil __gc on aborted multipart upload#2503
climagabriel wants to merge 1 commit into
openresty:masterfrom
climagabriel:fix-segv-receiveuntil-gc-aborted-multipart

Conversation

@climagabriel
Copy link
Copy Markdown
Contributor

@climagabriel climagabriel commented May 25, 2026

Summary

A client that POSTs a multipart body and aborts the connection mid-pattern against ngx.req.socket():receiveuntil(boundary) can SIGSEGV the worker from the LuaJIT GC's __gc finalizer on the compiled-pattern userdata.

ngx_http_lua_socket_read_error_retval_handler calls ngx_http_lua_socket_tcp_finalize_read_part directly when the iterator's recv returns an error (e.g. client RST). That clears u->buf_in but leaves cp->upstream pointing at u with cp->state > 0. When LuaJIT GC later sweeps cp, ngx_http_lua_socket_cleanup_compiled_pattern calls ngx_http_lua_socket_tcp_read_prepare, the cp->state > 0 path derefs u->buf_in — NULL deref.

Fix

In ngx_http_lua_socket_tcp_finalize_read_part, clear cp->upstream — mirroring the same detach that ngx_http_lua_socket_tcp_finalize already performs. The __gc handler then short-circuits at its existing if (u != NULL) guard.

Reproducer

POST a multipart body that ends mid-boundary, then close the socket with SO_LINGER {1, 0} so the server reads RST while the DFA is mid-match.

content_by_lua_block {
    local BOUNDARY = "----------GFioQpMK0vv2"
    local function read_aborted()
        local sock = ngx.req.socket()
        if not sock then return end
        sock:settimeout(5000)
        local iter = sock:receiveuntil("--" .. BOUNDARY)
        if not iter then return end
        while true do
            local d = iter(1)
            if not d then return end
        end
    end
    read_aborted()
    collectgarbage("collect")
    ngx.print("done")
}
python3 -c "
import socket, struct
s = socket.socket()
s.connect(('127.0.0.1', 18290))
s.sendall(
    b'POST /upload HTTP/1.1\r\n'
    b'Host: 127.0.0.1\r\n'
    b'Content-Type: multipart/form-data; boundary=----------GFioQpMK0vv2\r\n'
    b'Content-Length: 1048576\r\n'
    b'Connection: close\r\n\r\n'
    b'------'
)
s.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack('ii', 1, 0))
s.close()
"

Six leading dashes against the 12-dash boundary leader land cp->state at 6 with no DFA fallback; the synchronous collectgarbage runs cp's __gc inside the request. 100% crash rate.

Crash signature

#0  ngx_http_lua_socket_tcp_read_prepare (data=0x0)
#1  ngx_http_lua_socket_cleanup_compiled_pattern
#2  gc_call_finalizer                  lj_gc.c:521
#3  gc_finalize                        lj_gc.c:573
#4  lj_gc_fullgc                       lj_gc.c:804
#5  lj_cf_collectgarbage               lib_base.c:470
#6  ngx_http_lua_run_thread            ngx_http_lua_util.c:1196
#7  ngx_http_lua_socket_tcp_resume_helper

Observed in the wild across multiple PoPs against SWFUpload-style boundary scanners hitting WordPress upload endpoints behind a Lua front layer.

Test plan

  • Builds clean against upstream/master (41ed26b6).
  • Reproducer above crashes 100% on pre-fix master; no crash with this patch (pytest harness, --count=100, 8 aborts per iteration).
  • No regression in t/ test suite.

@climagabriel climagabriel force-pushed the fix-segv-receiveuntil-gc-aborted-multipart branch from 6335e76 to 1245fa0 Compare May 25, 2026 08:12
read_error_retval_handler calls finalize_read_part directly when
the receiveuntil iterator's recv errors. That clears u->buf_in
but leaves cp->upstream live with cp->state > 0. Later GC fires
cleanup_compiled_pattern -> read_prepare, which derefs the
now-NULL u->buf_in.

Mirror tcp_finalize's cp->upstream = NULL detach so __gc's
existing `if (u != NULL)` guard short-circuits.
@climagabriel climagabriel force-pushed the fix-segv-receiveuntil-gc-aborted-multipart branch from 1245fa0 to ebf9719 Compare May 25, 2026 09:36
@climagabriel
Copy link
Copy Markdown
Contributor Author

will re-open after coverage and mutation testing of the reproducer pytest script

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant