Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#1474 closed defect (invalid)

ngx_http_variable_unknown_header crash

Reported by: crasyangel.lhy@… Owned by:
Priority: critical Milestone:
Component: nginx-core Version: 1.12.x
Keywords: coredump segment fault Cc: gcc, 4.8.3
uname -a: 2.6.32_1-16-0-0_virtio
nginx -V: nginx version: nginx/1.12.2
built by gcc 4.8.2 (GCC)
built with OpenSSL 1.0.2h 3 May 2016
TLS SNI support enabled
configure arguments: --with-debug --with-ipv6 --with-http_v2_module --with-http_spdy_module --add-module=addon/jstart --add-module=addon/upstream_jhash --add-module=addon/upstream_jconhash --add-module=addon/upstream_jprobe --add-module=addon/upstream_jdomain --add-module=addon/jflow_control --add-module=addon/jflv --add-module=addon/ps_imgpack --add-module=addon/jpartition --add-module=addon/jdisposition --add-module=addon/jstore --add-module=addon/jchecksum --add-module=addon/jbilling --add-module=addon/headers_more --add-module=addon/blowfish --add-module=addon/jcheck --add-module=addon/bmat --add-module=addon/bmat2 --add-module=addon/bmat_lua --add-module=addon/bmat2_lua --add-module=addon/md5check --add-module=addon/jcrc --add-module=addon/jurl_decrypt_blowfish --add-module=addon/jfirst_byte --add-module=addon/hntv_log --add-module=addon/hntv_pno_billing --add-module=addon/qq_sdtfrom_billing --add-module=addon/server_internal_rechange --add-module=addon/server_302_rechange --add-module=addon/jvip --add-module=addon/urlban --with-http_stub_status_module --with-pcre=addon/pcre-8.01/ --with-ld-opt='-lstdc++ -std=c99' --with-cc-opt=-DSSL_ASYNCH --with-openssl=/home/scmbuild/workspaces_cluster/baidu.jomo.jnx__GCC_4.8.2/baidu/jomo/jnx/third-lib/openssl-1.0.2h --with-openssl-opt=-lpthread --with-http_gunzip_module --with-http_realip_module --with-http_ssl_module --add-module=addon/lua-nginx-module-0.10.11 --add-module=addon/lua-get-conf --add-module=addon/pcs_rate_limit --add-module=addon/global_rate_limit --add-module=addon/ngx_devel_kit-0.2.19 --add-module=addon/lua-dynamic-upstream --add-module=addon/log-split --add-module=addon/decrypt_uri --add-module=addon/silent_limit_req --add-module=addon/ngx_http_iplib_module --prefix=output

Description

#0 ngx_http_variable_unknown_header (v=0x7f2cfec480b0, var=0x1aead9b0, part=0x7f2cfeec0108, prefix=prefix@entry=5) at src/http/ngx_http_variables.c:989
#1 0x00000000004a5183 in ngx_http_variable_unknown_header_in (r=<optimized out>, v=<optimized out>, data=<optimized out>)

at src/http/ngx_http_variables.c:946

#2 0x00000000004a4da3 in ngx_http_get_indexed_variable (r=0x7f2d05294620, index=<optimized out>) at src/http/ngx_http_variables.c:631
#3 0x00000000004a0f46 in ngx_http_log_variable_getlen (r=<optimized out>, data=<optimized out>) at src/http/modules/ngx_http_log_module.c:940
#4 0x00000000004a0b2a in ngx_http_log_handler (r=0x7f2d05294620) at src/http/modules/ngx_http_log_module.c:296
#5 0x00000000004964e0 in ngx_http_log_request (r=r@entry=0x7f2d05294620) at src/http/ngx_http_request.c:3627
#6 0x00000000004981af in ngx_http_free_request (r=r@entry=0x7f2d05294620, rc=rc@entry=0) at src/http/ngx_http_request.c:3574
#7 0x0000000000498331 in ngx_http_close_request (r=r@entry=0x7f2d05294620, rc=rc@entry=0) at src/http/ngx_http_request.c:3511
#8 0x00000000004996cc in ngx_http_finalize_connection (r=r@entry=0x7f2d05294620) at src/http/ngx_http_request.c:2638
#9 0x000000000049975b in ngx_http_finalize_request (r=r@entry=0x7f2d05294620, rc=-4) at src/http/ngx_http_request.c:2339
#10 0x0000000000499974 in ngx_http_finalize_request (r=r@entry=0x7f2d05294620, rc=rc@entry=502) at src/http/ngx_http_request.c:2397
#11 0x00000000004aa07f in ngx_http_upstream_finalize_request (r=r@entry=0x7f2d05294620, u=u@entry=0x7f2d03b1a190, rc=rc@entry=502)

at src/http/ngx_http_upstream.c:4450

#12 0x00000000004acb41 in ngx_http_upstream_intercept_errors (u=0x7f2d03b1a190, r=0x7f2d05294620) at src/http/ngx_http_upstream.c:2617
#13 ngx_http_upstream_process_header (r=0x7f2d05294620, u=0x7f2d03b1a190) at src/http/ngx_http_upstream.c:2401
#14 0x00000000004aa1cf in ngx_http_upstream_handler (ev=0x407c7d0) at src/http/ngx_http_upstream.c:1275
#15 0x0000000000483c7a in ngx_epoll_process_events (cycle=0x16f3bc0, timer=<optimized out>, flags=<optimized out>)

at src/event/modules/ngx_epoll_module.c:902

#16 0x0000000000479bff in ngx_process_events_and_timers (cycle=cycle@entry=0x16f3bc0) at src/event/ngx_event.c:251
#17 0x0000000000481a0c in ngx_worker_process_cycle (cycle=0x16f3bc0, data=<optimized out>) at src/os/unix/ngx_process_cycle.c:749
#18 0x00000000004800d2 in ngx_spawn_process (cycle=cycle@entry=0x16f3bc0, proc=proc@entry=0x481979 <ngx_worker_process_cycle>, data=data@entry=0x1,

name=name@entry=0x75e95e "worker process", respawn=respawn@entry=-4) at src/os/unix/ngx_process.c:198

#19 0x0000000000480cbe in ngx_start_worker_processes (cycle=cycle@entry=0x16f3bc0, n=6, type=type@entry=-4) at src/os/unix/ngx_process_cycle.c:358
#20 0x0000000000482ae3 in ngx_master_process_cycle (cycle=0x16f3bc0, cycle@entry=0x16efbb0) at src/os/unix/ngx_process_cycle.c:243
#21 0x000000000045a54e in main (argc=<optimized out>, argv=<optimized out>) at src/core/nginx.c:375

    header = part->elts;

    for (i = 0; /* void */ ; i++) {

        if (i >= part->nelts) {
            if (part->next == NULL) {
                break;
            }

            part = part->next;
            header = part->elts;
            i = 0;
        }

        if (header[i].hash == 0) {
            continue;
        }

It move to next when this part goto end, as part = part->next; header = part->elts; i = 0;
And use header[i] (part->next->elts[0]) directly without checking whether the part is null or not
Better use continue when move to next (after i=0;), to avoid that the first elt does not initialize after ngx_list_push

So many same case in different place

Change History (5)

comment:1 by Ruslan Ermilov, 6 years ago

Wrong. The

if (part->next == NULL) {
    break;
}

fragment takes care of detecting the end of parts.

in reply to:  1 ; comment:2 by crasyangel.lhy@…, 6 years ago

No, i mean the next part is empty, but not the null pointer, it crashes when the first elt does not initialize after ngx_list_push
Because i >= part->nelts and part->next == NULL would not exec after move to next part

Replying to ru:

Wrong. The

if (part->next == NULL) {
    break;
}

fragment takes care of detecting the end of parts.

Last edited 6 years ago by crasyangel.lhy@… (previous) (diff)

in reply to:  2 comment:3 by crasyangel.lhy@…, 6 years ago

Not exactly,ngx_list_push would increase nelts, but the core shows the part->next->nelts is zero and part->next->next is null, I do not found the case yet

Replying to crasyangel.lhy@…:

No, i mean the next part is empty, but not the null pointer, it crashes when the first elt does not initialize after ngx_list_push
Because i >= part->nelts and part->next == NULL would not exec after move to next part

Replying to ru:

Wrong. The

if (part->next == NULL) {
    break;
}

fragment takes care of detecting the end of parts.

comment:4 by crasyangel.lhy@…, 6 years ago

It should be the module headers_more bug

thx

Last edited 6 years ago by crasyangel.lhy@… (previous) (diff)

comment:5 by Ruslan Ermilov, 6 years ago

Resolution: invalid
Status: newclosed

Originator reports that the bug is likely caused by 3rd party headers_more module.

Last edited 6 years ago by Ruslan Ermilov (previous) (diff)
Note: See TracTickets for help on using tickets.