Potential bug in ngx_event.c

[Rian Hunter]

On 2019-08-30 07:19, Maxim Dounin wrote:
> Hello!
> 
> On Fri, Aug 23, 2019 at 11:00:41AM -0700, Rian Hunter wrote:
> 
>> While browsing the source I noticed something that seemed wrong, even
>> though I haven't observed any buggy behavior or have reproduced this 
>> bug
>> myself. In ngx_event.c there is a line:
>> 
>>      if (ngx_shmtx_create(&ngx_accept_mutex, (ngx_shmtx_sh_t *) 
>> shared,
>>                           cycle->lock_file.data)
>>          != NGX_OK)
>>      {
>>          return NGX_ERROR;
>>      }
>> 
>> ngx_shmtx_create() is passed &ngx_accept_mutex, but this must be a
>> pointer to a shared memory region otherwise the sem_wait()/sem_post()
>> calls in ngx_shmtx.c will not function correctly. &ngx_accept_mutex is 
>> a
>> pointer to BSS, ngx_accept_mutex_ptr is a pointer to shared memory. Is
>> this intentional?
> 
> Yes, this is intentional.
> 
> The first agument of the ngx_shmtx_create() function is a pointer
> to the ngx_shmtx_t structure, which is not expected to be shared
> between processes.  Copy of the structure as created by fork() is
> enough.

The POSIX sem_t member (called "sem") needs to reside in shared memory
if sem_wait()/sem_post() are to have an effect across processes. Memory
copied across fork is not sufficient.

> Only ngx_shmtx_sh_t - which is used to store atomic variables with
> lock and wait counter - needs to be shared.  And it is shared, as
> "(ngx_shmtx_sh_t *) shared" is used as the second argument.

Yes, the atomic modifications to the variable stored in that memory is
correct and will be shared across processes but the 
sem_wait()/sem_post() calls
within ngx_shmtx.c have no effect since the "sem" member of ngx_shmtx_t 
is not shared.

The end result will be that ngx_shmtx_wakeup() (amongst others) has no 
effect.