[PATCH] Fixed segfault when switching off master process during upgrade

[Roman Arutyunyan]

Hi,

On Sun, Oct 30, 2022 at 05:42:33AM +0300, Maxim Dounin wrote:
> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1667097733 -10800
> #      Sun Oct 30 05:42:13 2022 +0300
> # Node ID ef9c94be7fe4685f0eeee41f76b964ea252f519f
> # Parent  b73d95226c84b93e51f23f7b35782d98d3b516b9
> Fixed segfault when switching off master process during upgrade.
> 
> Binary upgrades are not supported without master process, but it is,
> however, possible, that nginx running with master process is asked
> to upgrade binary, and the configuration file as available on disk
> at this time includes "master_process off;".
> 
> If this happens, listening sockets inherited from the previous binary
> will have ls[i].previous set.  But the old cycle on initial process
> startup, including startup after binary upgrade, is destroyed by
> ngx_init_cycle() once configuration parsing is complete.  As a result,
> an attempt to dereference ls[i].previous in ngx_event_process_init()
> accesses already freed memory.
> 
> Fix is to avoid looking into ls[i].previous if the old cycle is already
> freed.
> 
> diff --git a/src/event/ngx_event.c b/src/event/ngx_event.c
> --- a/src/event/ngx_event.c
> +++ b/src/event/ngx_event.c
> @@ -813,7 +813,9 @@ ngx_event_process_init(ngx_cycle_t *cycl
>          rev->deferred_accept = ls[i].deferred_accept;
>  #endif
>  
> -        if (!(ngx_event_flags & NGX_USE_IOCP_EVENT)) {
> +        if (!(ngx_event_flags & NGX_USE_IOCP_EVENT)
> +            && cycle->old_cycle)
> +        {
>              if (ls[i].previous) {
>  
>                  /*

Doesn't this also mean that we can throw away zeroing ls->previous?

diff --git a/src/os/unix/ngx_process_cycle.c b/src/os/unix/ngx_process_cycle.c
--- a/src/os/unix/ngx_process_cycle.c
+++ b/src/os/unix/ngx_process_cycle.c
@@ -888,15 +888,6 @@ ngx_worker_process_init(ngx_cycle_t *cyc
     tp = ngx_timeofday();
     srandom(((unsigned) ngx_pid << 16) ^ tp->sec ^ tp->msec);
 
-    /*
-     * disable deleting previous events for the listening sockets because
-     * in the worker processes there are no events at all at this point
-     */
-    ls = cycle->listening.elts;
-    for (i = 0; i < cycle->listening.nelts; i++) {
-        ls[i].previous = NULL;
-    }
-
     for (i = 0; cycle->modules[i]; i++) {
         if (cycle->modules[i]->init_process) {
             if (cycle->modules[i]->init_process(cycle) == NGX_ERROR) {

--
Roman Arutyunyan