Re: getaddrinfo() is not thread-safe in 6.8

[Otto Moerbeek]

On Thu, Dec 24, 2020 at 08:28:17AM +0100, Otto Moerbeek wrote:

> On Wed, Dec 23, 2020 at 11:58:30PM -0700, Theo de Raadt wrote:
> 
> > Brad Smith <b...@comstyle.com> wrote:
> > 
> > > On 12/23/2020 10:35 PM, Alexey Sokolov wrote:
> > > >> Synopsis:      getaddrinfo() is not thread-safe in 6.8
> > > >> Category:      system
> > > >> Environment:
> > > >         System      : OpenBSD 6.8
> > > >         Details     : OpenBSD 6.8 (GENERIC.MP) #1: Tue Nov  3 09:06:04 
> > > > MST 2020
> > > >                         
> > > > r...@syspatch-68-amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > > >
> > > >         Architecture: OpenBSD.amd64
> > > >         Machine     : amd64
> > > >> Description:
> > > > Hi, getaddrinfo() crashes when multiple threads run getaddrinfo()
> > > > concurrently. This didn't happen in 6.7.
> > > > It looks like asr_ctx which is supposed to be thread-local according to
> > > > _asr_use_resolver(), is actually static / shared between threads.
> > > >> How-To-Repeat:
> > > > Compile this code (gcc a.c -pthread), and run. It will segfault in
> > > > several seconds. Happens with both gcc (4.2.1) and egcc (8.4.0).
> > > 
> > > Before going any further. Use the system compiler. That is Clang (cc).
> > 
> > I disagree.   As the test appears fairly idiomatic threading code, I
> > find it unlikely a compiler change should matter.
> > 
> 
> I can reprorduce on arm64 (with clang). If I change do_things() to run
> forever and move the main thread call to do_things down to after the
> creation loop the crash does not seem to happen. So I suppose the bug
> is related to to the creation and destruction of threads and their
> local data.
> 
>       -Otto
> 

Two observations, starting again with the original test code.

1. Removing the do_things() call in the main thread makes the crash go away.  

2. Making sure the thread runtime env is initialized before the call
to do_things() by doing a pthread_attr_init() call (see modified test
program below) make the crash also go away.

So there seem something fishy going on if the main thread starts using
thread local data before the thread runtime is properly initialized.

Cc:ing guenther@ to see if he has something to say.

        -Otto


#include <pthread.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <stdio.h>
#include <string.h>

#define NUM_THREADS 5

static void do_lookup(const char *host)
{
        int s;
        struct addrinfo hints;
        struct addrinfo *result;

        memset(&hints, 0, sizeof(hints));
        hints.ai_family = AF_UNSPEC;
        hints.ai_socktype = SOCK_STREAM;
        hints.ai_flags = AI_ADDRCONFIG;
        hints.ai_protocol = IPPROTO_TCP;

        s = getaddrinfo(host, NULL, &hints, &result);
        if (s != 0) {
                fprintf(stderr, "Lookup error for %s: %s\n", host, 
gai_strerror(s));
        } else {
                freeaddrinfo(result);
        }
}

static void *
do_things(void *arg)
{
        (void) arg;
        do_lookup("ipv4.google.com");
        do_lookup("ipv6.google.com");
        do_lookup("google.com");
        do_lookup("heise.de");
        return NULL;
}

int main()
{
        pthread_attr_t a;
        pthread_t threads[NUM_THREADS];
        int i;
        int s;

        pthread_attr_init(&a); // remove this and the crash reappears
        for (;;) {
                do_things(NULL);
                for (i = 0; i < NUM_THREADS; i++) {
                        s = pthread_create(&threads[i], NULL, do_things, NULL);
                        if (s != 0)
                                fprintf(stderr, "Error creating thread");
                }
                for (i = 0; i < NUM_THREADS; i++) {
                        s = pthread_join(threads[i], NULL);
                        if (s != 0)
                                fprintf(stderr, "Error joining thread");
                }
        }
        return 0;
}