This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix the race between atexit() and exit()



Quoting Carlos O'Donell <carlos@systemhalted.org>:


1. Describe in detail the conditions of the race and your proposed fix.

current exit() code
==================
while (__exit_funcs != NULL) <==== not atomic, also what if a new handler is registered right after this
{
struct exit_function_list *old;


while (__exit_funcs->idx > 0) <==== not atomic, what if another thread increments the index while registering a new handle
{
const struct exit_function *const f =
&__exit_funcs->fns[--__exit_funcs->idx]; <==== not atomic
switch (f->flavor)
{
void (*atfct) (void);
void (*onfct) (int status, void *arg);
void (*cxafct) (void *arg, int status);


case ef_free:
case ef_us: <=== if the current handler registration is in process, we miss it
break;
...
...


  old = __exit_funcs; <==== not atomic
  __exit_funcs = __exit_funcs->next; <==== not atomic
  if (__exit_funcs != NULL)
/* Don't free the last element in the chain, this is the statically
   allocate element.  */
free (old); <====== not atomic
}


The proposed patch
===================
1. serialises any modification of __exit_funcs list by locking
2. protects assignments of local variables to list property values like index and next in locks
3. protects change of status (flavor) by atomic operations
3. check if any new handler has been registered in a block before freeing it
4. fails new registrations once exit() finish processing the list



2. Provide the test case.

Test case ========= #include <stdio.h> #include <stdlib.h> #include <pthread.h> #include <unistd.h> #include <sys/types.h> #include <sys/mman.h> #include <sys/stat.h> #include <fcntl.h>

char *data;

#define ATEXITFUNC1(x) void atexit_func1##x()         \
    {       \
        *(data - 10 + 96 + x) = x-10;       \
    }

#define ATEXITFUNC(x) void atexit_func##x()             \
        {       \
                *(data - 10 + 64 + x) = x-10;           \
        if(atexit(atexit_func1##x))          \
            write(2,"F\n",2); \
        else \
            *(data - 10  + 32 + x) = x -10; \
        }

#define REGISTER_ATEXITFUNC(x) void *register_atexit_func##x(void *t)   \
        {                                               \
                while(thread_start == 0);               \
                if(atexit(atexit_func##x))                      \
            write(2,"F\n",2); \
        else \
                    *(data - 10  + x) = x -10; \
        }

int thread_start = 0;


ATEXITFUNC1(10) ATEXITFUNC1(11) ATEXITFUNC1(12) ATEXITFUNC1(13) ATEXITFUNC1(14) ATEXITFUNC1(15) ATEXITFUNC1(16) ATEXITFUNC1(17) ATEXITFUNC1(18) ATEXITFUNC1(19) ATEXITFUNC1(20) ATEXITFUNC1(21) ATEXITFUNC1(22) ATEXITFUNC1(23) ATEXITFUNC1(24) ATEXITFUNC1(25) ATEXITFUNC1(26) ATEXITFUNC1(27) ATEXITFUNC1(28) ATEXITFUNC1(29) ATEXITFUNC1(30) ATEXITFUNC1(31) ATEXITFUNC1(32) ATEXITFUNC1(33) ATEXITFUNC1(34) ATEXITFUNC1(35) ATEXITFUNC1(36) ATEXITFUNC1(37) ATEXITFUNC1(38) ATEXITFUNC1(39)

ATEXITFUNC(10)
ATEXITFUNC(11)
ATEXITFUNC(12)
ATEXITFUNC(13)
ATEXITFUNC(14)
ATEXITFUNC(15)
ATEXITFUNC(16)
ATEXITFUNC(17)
ATEXITFUNC(18)
ATEXITFUNC(19)
ATEXITFUNC(20)
ATEXITFUNC(21)
ATEXITFUNC(22)
ATEXITFUNC(23)
ATEXITFUNC(24)
ATEXITFUNC(25)
ATEXITFUNC(26)
ATEXITFUNC(27)
ATEXITFUNC(28)
ATEXITFUNC(29)
ATEXITFUNC(30)
ATEXITFUNC(31)
ATEXITFUNC(32)
ATEXITFUNC(33)
ATEXITFUNC(34)
ATEXITFUNC(35)
ATEXITFUNC(36)
ATEXITFUNC(37)
ATEXITFUNC(38)
ATEXITFUNC(39)

REGISTER_ATEXITFUNC(10)
REGISTER_ATEXITFUNC(11)
REGISTER_ATEXITFUNC(12)
REGISTER_ATEXITFUNC(13)
REGISTER_ATEXITFUNC(14)
REGISTER_ATEXITFUNC(15)
REGISTER_ATEXITFUNC(16)
REGISTER_ATEXITFUNC(17)
REGISTER_ATEXITFUNC(18)
REGISTER_ATEXITFUNC(19)
REGISTER_ATEXITFUNC(20)
REGISTER_ATEXITFUNC(21)
REGISTER_ATEXITFUNC(22)
REGISTER_ATEXITFUNC(23)
REGISTER_ATEXITFUNC(24)
REGISTER_ATEXITFUNC(25)
REGISTER_ATEXITFUNC(26)
REGISTER_ATEXITFUNC(27)
REGISTER_ATEXITFUNC(28)
REGISTER_ATEXITFUNC(29)
REGISTER_ATEXITFUNC(30)
REGISTER_ATEXITFUNC(31)
REGISTER_ATEXITFUNC(32)
REGISTER_ATEXITFUNC(33)
REGISTER_ATEXITFUNC(34)
REGISTER_ATEXITFUNC(35)
REGISTER_ATEXITFUNC(36)
REGISTER_ATEXITFUNC(37)
REGISTER_ATEXITFUNC(38)
REGISTER_ATEXITFUNC(39)

int main()
{
    pthread_t tid[30];
    int ret_code[30];
    int pagesize;
    int fd,len;

    fd = open("foo", O_RDWR);
    if(fd<1)
    {
        perror("open");
        exit(1);
    }

    pagesize = getpagesize();
    len=lseek(fd,pagesize,SEEK_CUR);
    if(write(fd, " ", 1 ) != 1)
    {
        perror("write error. ");
        exit(1);
    }

    data = mmap((caddr_t)0, pagesize, PROT_WRITE, MAP_SHARED, fd, 0);
    if(!data)
    {
        perror("mmap");
        exit(1);
    }

ret_code[0] = pthread_create(&tid[0], 0, register_atexit_func10, &tid[0]);
ret_code[1] = pthread_create(&tid[1], 0, register_atexit_func11, &tid[1]);
ret_code[2] = pthread_create(&tid[2], 0, register_atexit_func12, &tid[2]);
ret_code[3] = pthread_create(&tid[3], 0, register_atexit_func13, &tid[3]);
ret_code[4] = pthread_create(&tid[4], 0, register_atexit_func14, &tid[4]);
ret_code[5] = pthread_create(&tid[5], 0, register_atexit_func15, &tid[5]);
ret_code[6] = pthread_create(&tid[6], 0, register_atexit_func16, &tid[6]);
ret_code[7] = pthread_create(&tid[7], 0, register_atexit_func17, &tid[7]);
ret_code[8] = pthread_create(&tid[8], 0, register_atexit_func18, &tid[8]);
ret_code[9] = pthread_create(&tid[9], 0, register_atexit_func19, &tid[9]);
ret_code[10] = pthread_create(&tid[10], 0, register_atexit_func20, &tid[10]);
ret_code[11] = pthread_create(&tid[11], 0, register_atexit_func21, &tid[11]);
ret_code[12] = pthread_create(&tid[12], 0, register_atexit_func22, &tid[12]);
ret_code[13] = pthread_create(&tid[13], 0, register_atexit_func23, &tid[13]);
ret_code[14] = pthread_create(&tid[14], 0, register_atexit_func24, &tid[14]);
ret_code[15] = pthread_create(&tid[15], 0, register_atexit_func25, &tid[15]);
ret_code[16] = pthread_create(&tid[16], 0, register_atexit_func26, &tid[16]);
ret_code[17] = pthread_create(&tid[17], 0, register_atexit_func27, &tid[17]);
ret_code[18] = pthread_create(&tid[18], 0, register_atexit_func28, &tid[18]);
ret_code[19] = pthread_create(&tid[19], 0, register_atexit_func29, &tid[19]);
ret_code[20] = pthread_create(&tid[20], 0, register_atexit_func30, &tid[20]);
ret_code[21] = pthread_create(&tid[21], 0, register_atexit_func31, &tid[21]);
ret_code[22] = pthread_create(&tid[22], 0, register_atexit_func32, &tid[22]);
ret_code[23] = pthread_create(&tid[23], 0, register_atexit_func33, &tid[23]);
ret_code[24] = pthread_create(&tid[24], 0, register_atexit_func34, &tid[24]);
ret_code[25] = pthread_create(&tid[25], 0, register_atexit_func35, &tid[25]);
ret_code[26] = pthread_create(&tid[26], 0, register_atexit_func36, &tid[26]);
ret_code[27] = pthread_create(&tid[27], 0, register_atexit_func37, &tid[27]);
ret_code[28] = pthread_create(&tid[28], 0, register_atexit_func38, &tid[28]);
ret_code[29] = pthread_create(&tid[29], 0, register_atexit_func39, &tid[29]);


        thread_start = 1;
        exit(0);
}

After running the test, check the result by "hexdump -C foo"
The first 4 lines indicates the registration status and next 4 indicates whether the handler was invoked
If they match (excluding the first column) then the test passed.
Program prints "F" to stderror to indicate failed registrations


Running without the patch gives
00000000  00 01 02 03 04 05 06 07  08 09 0a 0b 0c 0d 0e 0f  |................|
00000010  10 11 12 13 14 15 16 17  18 19 1a 1b 1c 1d 00 00  |................|
00000020  00 01 02 03 04 05 06 07  08 09 0a 0b 00 0d 0e 0f  |................|
00000030  10 11 12 13 14 15 16 17  18 19 1a 1b 1c 1d 00 00  |................|
00000040  00 01 02 03 04 05 06 07  08 09 0a 0b 00 0d 0e 0f  |................|
00000050  10 11 12 13 14 15 16 17  18 19 1a 1b 1c 1d 00 00  |................|
00000060  00 01 02 03 04 05 06 07  08 09 0a 0b 00 0d 0e 0f  |................|
00000070  10 11 12 13 14 15 16 17  18 19 1a 1b 1c 1d 00 00  |................|

We can see that handler "0c" was registered (first line) but it was not invoked (see line 5)

3. Run the testsuite for your target to show there were no
regressions. Mention the target.
The test was run for about 10 hrs in loop in an x86_64 SMP target without failures.

4. When quoting POSIX please provide references.

POSIX Ref: => man 3p atexit


DESCRIPTION
The atexit() function shall register the function pointed to by func, to be called without arguments at
normal program termination. At normal program termination, all functions registered by the atexit() func-
tion shall be called, in the reverse order of their registration, except that a function is called after
any previously registered functions that had already been called at the time it was registered. Normal
termination occurs either by a call to exit() or a return from main().





Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]