Potential bug in ngx_event.c

[Rian Hunter]

On 2019-08-31 11:28, Maxim Dounin wrote:
> Hello!
> 
> On Fri, Aug 30, 2019 at 06:26:57PM -0700, Rian Hunter wrote:
> 
>> On 2019-08-30 07:19, Maxim Dounin wrote:
>> > 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.
> 
> No, fork() is explicitly documented to preserve semaphores
> (http://pubs.opengroup.org/onlinepubs/9699919799/functions/fork.html):
> 
> : Any semaphores that are open in the parent process shall also be
> : open in the child process.

That's only true for semaphores created with sem_open(). For semaphores 
created with sem_init(pshared=1) the caller is responsible for using 
supplying memory that resides in a shared memory segment (e.g. created 
from mm ap(flags=MAP_SHARED|...)).

See: 
https://blog.superpat.com/2010/07/14/semaphores-on-linux-sem_init-vs-sem_open/

Here is a program that illustrates my point (also here: 
https://gist.github.com/rianhunter/a0bd4c9e8ab550ecadbe2464995726a8):

     #include <stdio.h>
     #include <stdlib.h>
     #include <string.h>

     #include <errno.h>
     #include <semaphore.h>
     #include <time.h>
     #include <unistd.h>

     #include <sys/mman.h>
     #include <sys/types.h>
     #include <sys/wait.h>

     int test_semaphore(sem_t *sem) {
         pid_t pid;

         pid = fork();

         if (pid < 0) {
             fprintf(stderr, "failed to fork: %s\n", strerror(errno));
             return -1;
         }

         if (pid > 0) {
             int status;

             /* after posting, child should exit successfully */
             sem_post(sem);

             printf("Waiting for child to react to sem_post()...\n");
             waitpid(pid, &status, 0);

             if (!WIFEXITED(status) || WEXITSTATUS(status) > 1) {
                 return -1;
             }

             return !WEXITSTATUS(status);
         } else {
             int ret;
             struct timespec to;

             ret = clock_gettime(CLOCK_REALTIME, &to);
             if (ret < 0) {
                 fprintf(stderr, "failed to clock_gettime(): %s\n", 
strerror(errno));
                 return 255;
             }

             /* wait for sem_post() for 5 seconds */
             to.tv_sec += 5;
             ret = sem_timedwait(sem, &to);
             if (ret < 0 && errno != ETIMEDOUT) {
                 fprintf(stderr, "failed to timedwait: %s\n", 
strerror(errno));
                 return 255;
             }
             exit(!ret ? EXIT_SUCCESS : EXIT_FAILURE);
         }

         /* notreached */
     }

     int main() {
         int ret;
         sem_t stack_allocated, *mmap_shared;

         sem_init(&stack_allocated, 1, 0);

         ret = test_semaphore(&stack_allocated);
         if (ret < 0) {
             return EXIT_FAILURE;
         }

         if (!ret) {
             printf("Stack-allocated semaphore across fork() is 
broken\n");
         } else {
             printf("Stack-allocated semaphore across fork() works\n");
         }

         printf("\n");

         mmap_shared = mmap(NULL, getpagesize(), PROT_READ | PROT_WRITE, 
MAP_SHARED | MAP_ANONYMOUS, -1, 0);
         if (mmap_shared == MAP_FAILED) {
             return EXIT_FAILURE;
         }

         sem_init(mmap_shared, 1, 0);

         ret = test_semaphore(mmap_shared);
         if (ret < 0) {
             return EXIT_FAILURE;
         }

         if (!ret) {
             printf("Mapped semaphore across fork() is broken\n");
         } else {
             printf("Mapped semaphore across fork() works\n");
         }

         return EXIT_SUCCESS;
     }