crash at ngx_http_upstream_check_handler.c:1656

[maage]

Got crash:when reloading:

#0  ngx_http_check_add_timers (cycle=0x13832310) at ngx_http_upstream_check/ngx_http_upstream_check_handler.c:1656
1656            cf = ucscf->check_type_conf;
(gdb) where
#0  ngx_http_check_add_timers (cycle=0x13832310) at ngx_http_upstream_check/ngx_http_upstream_check_handler.c:1656
#1  0x000000000042e29e in ngx_worker_process_init (cycle=0x13832310, priority=<value optimized out>)
    at src/os/unix/ngx_process_cycle.c:962
#2  0x000000000042ea0c in ngx_worker_process_cycle (cycle=0x341bf524c0, data=<value optimized out>)
    at src/os/unix/ngx_process_cycle.c:724
#3  0x000000000042d210 in ngx_spawn_process (cycle=0x13832310, proc=0x42e9f0 <ngx_worker_process_cycle>, data=0x0, 
    name=0x4bba79 "worker process", respawn=-4) at src/os/unix/ngx_process.c:196
#4  0x000000000042e09c in ngx_start_worker_processes (cycle=0x13832310, n=2, type=-4)
    at src/os/unix/ngx_process_cycle.c:360
#5  0x000000000042f429 in ngx_master_process_cycle (cycle=0x13832310) at src/os/unix/ngx_process_cycle.c:249
#6  0x00000000004151ff in main (argc=5, argv=<value optimized out>) at src/core/nginx.c:405
(gdb) print peers
$1 = (ngx_http_check_peers_t *) 0x138b2648
(gdb) print *peers
$2 = {check_shm_name = {len = 327886240, data = 0x1391d7a0 "\020|\213\023"}, peers = {elts = 0x137f3970, 
    nelts = 329496184, size = 328325728, nalloc = 327104912, pool = 0x138b2690}, peers_shm = 0x1391dbf0}
(gdb) print (ngx_http_check_peer_t*)peers->peers->elts
$4 = (ngx_http_check_peer_t *) 0x137f3970
(gdb) print *(ngx_http_check_peer_t*)peers->peers->elts
$5 = {state = 327104896, pool = 0xff00000000, index = 327104848, max_busy = 1095216660480, peer_addr = 0x137f39a0, 
  check_ev = {data = 0x137f3970, write = 0, accept = 0, instance = 0, active = 0, disabled = 1, ready = 1, 
    oneshot = 1, complete = 0, eof = 1, error = 0, timedout = 0, timer_set = 0, delayed = 1, read_discarded = 1, 
    unexpected_eof = 0, deferred_accept = 0, pending_eof = 1, posted_ready = 1, available = 1, 
    handler = 0x48b740 <ngx_http_check_begin_handler>, index = 327104960, log = 0x13832328, timer = {key = 327104912, 
      left = 0xff00000000, right = 0x137f39e0, parent = 0xff00000000, color = 176 '°', data = 57 '9'}, closed = 0, 
    channel = 0, resolver = 0, next = 0x137f3a00, prev = 0xff00000000}, check_timeout_ev = {data = 0x137f3970, 
    write = 0, accept = 0, instance = 0, active = 0, disabled = 0, ready = 0, oneshot = 0, complete = 0, eof = 0, 
    error = 0, timedout = 0, timer_set = 0, delayed = 0, read_discarded = 0, unexpected_eof = 0, deferred_accept = 0, 
    pending_eof = 0, posted_ready = 0, available = 0, handler = 0x48ae50 <ngx_http_check_timeout_handler>, 
    index = 1095216660480, log = 0x137f39f0, timer = {key = 1095216660480, left = 0x137f3a40, right = 0xff00000000, 
      parent = 0x137f3a10, color = 0 '\000', data = 0 '\000'}, closed = 0, channel = 0, resolver = 0, 
    next = 0xff00000000, prev = 0x137f3a30}, pc = {connection = 0xff00000000, sockaddr = 0x137f3a80, socklen = 0, 
    name = 0x137f3a50, tries = 1095216660480, check_index = 327105136, get = 0xff00000000, free = 0x20bd00005, 
    data = 0x2441, set_session = 0x13923260, save_session = 0x13a10870, local = 0x0, rcvbuf = 0, log = 0x2450bcd0024, 
    cached = 0, log_error = 0}, check_data = 0x137f2990, send_handler = 0x71, recv_handler = 0x137f2880, 
  init = 0x13960ad0, parse = 0x341e88b780 <Perl_pp_nextstate>, reinit = 0, shm = 0x1391dc08, conf = 0x0}

Clearly dangling pointer.

Ran program with valgrind:

==18586== Invalid read of size 4                                
==18586==    at 0x80CBD81: ngx_http_check_add_timers (ngx_http_upstream_check_handler.c:1628)
==18586==    by 0x80CA082: ngx_http_check_init_process (ngx_http_upstream_check_module.c:602)
==18586==    by 0x8068F86: ngx_worker_process_init (ngx_process_cycle.c:962)
==18586==    by 0x80694D4: ngx_worker_process_cycle (ngx_process_cycle.c:724)
==18586==    by 0x8067C60: ngx_spawn_process (ngx_process.c:196)
==18586==    by 0x8068A6E: ngx_start_worker_processes (ngx_process_cycle.c:360)
==18586==    by 0x806A52A: ngx_master_process_cycle (ngx_process_cycle.c:249)
==18586==    by 0x804D792: main (nginx.c:405)
==18586==  Address 0x447863c is 5,244 bytes inside a block of size 16,384 free'd
==18586==    at 0x400551D: free (vg_replace_malloc.c:325)
==18586==    by 0x804E245: ngx_destroy_pool (ngx_palloc.c:86)
==18586==    by 0x805A49B: ngx_init_cycle (ngx_cycle.c:734)
==18586==    by 0x806A4EC: ngx_master_process_cycle (ngx_process_cycle.c:240)
==18586==    by 0x804D792: main (nginx.c:405)
==18586== 
{
   <insert_a_suppression_name_here>
   Memcheck:Addr4
   fun:ngx_http_check_add_timers
   fun:ngx_http_check_init_process
   fun:ngx_worker_process_init
   fun:ngx_worker_process_cycle
   fun:ngx_spawn_process
   fun:ngx_start_worker_processes
   fun:ngx_master_process_cycle
   fun:main
}

Crash happens when reloading. You need also to disable check first at new config.

I dont know how to make proper test case, but I used following test as base:

use Test::Nginx::LWP;
plan tests => repeat_each() * 2 * blocks();
no_root_location;
Test::Nginx::Util::master_on;
run_tests();
__DATA__
=== TEST 1: sleep
--- http_config
    upstream test{
        server localhost:80;
        check interval=1000 rise=1 fall=2 timeout=200 type=http;
        check_http_send "HEAD / HTTP/1.0\r\n\r\n";
        check_http_expect_alive http_2xx http_3xx;
    }
--- config
    location / {
        echo_sleep 5;
        echo '<ok>';
    }
--- request
GET /
--- response_body_like: ^<(.*)>$

And then disabled check by commenting lines (vi t/servroot/conf/nginx.conf). Config block when I run it etcproxy and valgrind so I can reload consistenlty.

Test script used:

#!/bin/sh
killall nginx
export TEST_NGINX_CLIENT_PORT=1234
export TEST_NGINX_USE_VALGRIND=1
export PATH="$HOME"/nginx/sbin:"$PATH"
prove -r ${1:-t}

This patch seems to fix this crash.

--- nginx-1.0.0-orig/ngx_http_upstream_check/ngx_http_upstream_check_handler.c  2011-01-06 05:35:23.000000000 +0200
+++ nginx-1.0.0/ngx_http_upstream_check/ngx_http_upstream_check_handler.c       2011-04-29 15:59:32.000000000 +0300
@@ -1602,6 +1602,8 @@
         check_peers_ctx = ucmcf->peers;

         shm_zone->init = ngx_http_upstream_check_init_shm_zone;
+    } else {
+        check_peers_ctx = NULL;
     }

     return NGX_CONF_OK;

[yaoweibin]

Thanks for your bug report. Sorry to bother you for my careless .Your patch is very correct. The global variable is evil. Maybe I should consider not to use it.

[yaoweibin]

The nginx_tcp_proxy_module also has the same problem. If you use it, you should update your source. See the commit: yaoweibin/nginx_tcp_proxy_module@d10ea36

I developed this check module first in the tcp proxy module, then ported to general HTTP module. They shared many codes.