From patchwork Tue Jul 4 09:31:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 21398 Received: (qmail 30837 invoked by alias); 4 Jul 2017 09:31:50 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 30493 invoked by uid 89); 4 Jul 2017 09:31:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3D1C0C058EB2 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=fweimer@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 3D1C0C058EB2 Date: Tue, 04 Jul 2017 11:31:40 +0200 To: libc-alpha@sourceware.org Subject: [PATCH COMMITTED] resolv: Fix improper assert in __resolv_conf_attach User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20170704093140.15BA4439942F0@oldenburg.str.redhat.com> From: fweimer@redhat.com (Florian Weimer) 2017-07-04 Florian Weimer * resolv/resolv_conf.c (struct resolv_conf_global): Clarify free list management and the role of free_list_start. (decrement_at_index): Put zero at the end of the free list. (__resolv_conf_attach): Fix bogus assert. * resolv/Makefile (tests): Add tst-resolv-res_init-multi. (tst-resolv-res_init-multi): Link with -lresolv, -lpthread. diff --git a/resolv/Makefile b/resolv/Makefile index 5cb2e53..6942e85 100644 --- a/resolv/Makefile +++ b/resolv/Makefile @@ -51,6 +51,7 @@ tests += \ tst-resolv-basic \ tst-resolv-edns \ tst-resolv-network \ + tst-resolv-res_init-multi \ tst-resolv-search \ # These tests need libdl. @@ -160,6 +161,8 @@ $(objpfx)tst-resolv-basic: $(objpfx)libresolv.so $(shared-thread-library) $(objpfx)tst-resolv-edns: $(objpfx)libresolv.so $(shared-thread-library) $(objpfx)tst-resolv-network: $(objpfx)libresolv.so $(shared-thread-library) $(objpfx)tst-resolv-res_init: $(libdl) $(objpfx)libresolv.so +$(objpfx)tst-resolv-res_init-multi: $(objpfx)libresolv.so \ + $(shared-thread-library) $(objpfx)tst-resolv-res_init-thread: $(libdl) $(objpfx)libresolv.so \ $(shared-thread-library) $(objpfx)tst-resolv-qtypes: $(objpfx)libresolv.so $(shared-thread-library) diff --git a/resolv/resolv_conf.c b/resolv/resolv_conf.c index b98cf92..0ed36cd 100644 --- a/resolv/resolv_conf.c +++ b/resolv/resolv_conf.c @@ -58,8 +58,10 @@ struct resolv_conf_global the array element is overwritten with NULL. */ struct resolv_conf_array array; - /* Start of the free list in the array. The MSB is set if this - field has been initialized. */ + /* Start of the free list in the array. Zero if the free list is + empty. Otherwise, free_list_start >> 1 is the first element of + the free list (and the free list entries all have their LSB set + and are shifted one to the left). */ uintptr_t free_list_start; /* Cached current configuration object for /etc/resolv.conf. */ @@ -567,11 +569,7 @@ decrement_at_index (struct resolv_conf_global *global_copy, size_t index) struct resolv_conf *conf = (struct resolv_conf *) *slot; conf_decrement (conf); /* Put the slot onto the free list. */ - if (global_copy->free_list_start == 0) - /* Not yet initialized. */ - *slot = 1; - else - *slot = global_copy->free_list_start; + *slot = global_copy->free_list_start; global_copy->free_list_start = (index << 1) | 1; } } @@ -598,7 +596,8 @@ __resolv_conf_attach (struct __res_state *resp, struct resolv_conf *conf) index = global_copy->free_list_start >> 1; uintptr_t *slot = resolv_conf_array_at (&global_copy->array, index); global_copy->free_list_start = *slot; - assert (global_copy->free_list_start & 1); + assert (global_copy->free_list_start == 0 + || global_copy->free_list_start & 1); /* Install the configuration pointer. */ *slot = (uintptr_t) conf; } diff --git a/resolv/tst-resolv-res_init-multi.c b/resolv/tst-resolv-res_init-multi.c new file mode 100644 index 0000000..bdc68a5 --- /dev/null +++ b/resolv/tst-resolv-res_init-multi.c @@ -0,0 +1,89 @@ +/* Multi-threaded test for resolver initialization. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include + +/* Whether name lookups succeed does not really matter. We use this + to trigger initialization of the resolver. */ +static const char *test_hostname = "www.gnu.org"; + +/* The different initialization methods. */ +enum test_type { init, byname, gai }; +enum { type_count = 3 }; + +/* Thread function. Perform a few resolver options. */ +static void * +thread_func (void *closure) +{ + enum test_type *ptype = closure; + /* Perform a few calls to the requested operation. */ + TEST_VERIFY (*ptype >= 0); + TEST_VERIFY (*ptype < (int) type_count); + for (int i = 0; i < 3; ++i) + switch (*ptype) + { + case init: + res_init (); + break; + case byname: + gethostbyname (test_hostname); + break; + case gai: + { + struct addrinfo hints = { 0, }; + struct addrinfo *ai = NULL; + if (getaddrinfo (test_hostname, "80", &hints, &ai) == 0) + freeaddrinfo (ai); + } + break; + } + free (ptype); + return NULL; +} + +static int +do_test (void) +{ + /* Start a small number of threads which perform resolver + operations. */ + enum { thread_count = 30 }; + + pthread_t threads[thread_count]; + for (int i = 0; i < thread_count; ++i) + { + enum test_type *ptype = xmalloc (sizeof (*ptype)); + *ptype = i % type_count; + threads[i] = xpthread_create (NULL, thread_func, ptype); + } + for (int i = 0; i < type_count; ++i) + { + enum test_type *ptype = xmalloc (sizeof (*ptype)); + *ptype = i; + thread_func (ptype); + } + for (int i = 0; i < thread_count; ++i) + xpthread_join (threads[i]); + return 0; +} + +#include