From patchwork Fri Aug 7 15:58:05 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: max X-Patchwork-Id: 505181 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id C403E14027C for ; Sat, 8 Aug 2015 01:58:19 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=AGRgfTZt; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type; q=dns; s=default; b= xIoIB/Qs7xq9FmPCuwh4vQnprEkFc+C0Rzbq7Dqney3uw5HpxINhuTUTd2v28FYa ghgkvDFhjpvMo16AnwWwOWH1VEyvl00Tv6RGstuLOlGQ+Xe83Z/WN5lr5eYBDuO4 DpevWtC581dp1ITIl2pLsyELzpVM15TbMoQr5Y0ceHE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type; s=default; bh=znkl cF60yxXGbF1A/ztUNoixnEc=; b=AGRgfTZtQxQhGd5xi5my+5/5etXL26L2k10Z rhTM1ddyQLdM26geDWvdkqw4BhoFrNM99jgnB0p1S/Ya20Td2xwWktdK5YuZXImO 6WqD7I8uHCdKWkjN8RAEBjI8CU4DsPI4dAy8jC7derzEU4b4Bcy48gedHX9pm5cS EnVVhPw= Received: (qmail 62532 invoked by alias); 7 Aug 2015 15:58:13 -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 62517 invoked by uid 89); 7 Aug 2015 15:58:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mailout1.w1.samsung.com Message-id: <55C4D58D.8010307@partner.samsung.com> Date: Fri, 07 Aug 2015 18:58:05 +0300 From: Maxim Ostapenko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-version: 1.0 To: Andreas Schwab , Pavel Kopyl Cc: "H.J. Lu" , Yury Gribov , Roland McGrath , GNU C Library , Carlos O'Donell , Viacheslav Garbuzov Subject: Re: [PATCHv5][PING^3][BZ #17833] _dl_close_worker() does not release inconsistent objects. References: <54BD4F65.2090108@samsung.com> <5553382F.3020906@samsung.com> <5565B5E5.7060101@samsung.com> <5565C2A8.60306@samsung.com> <5565C862.1040003@samsung.com> <5566395A.3090605@samsung.com> <5567892C.4070004@samsung.com> <5568A408.2080903@samsung.com> <5592AB91.2050709@samsung.com> <5595C0F8.3060300@samsung.com> <559B829C.8080700@samsung.com> <559BFDDC.4010604@samsung.com> In-reply-to: Content-type: multipart/mixed; boundary=------------090706020809040304040406 Hi! On 06/08/15 18:30, Andreas Schwab wrote: > Pavel Kopyl writes: > >> diff --git a/elf/dl-close.c b/elf/dl-close.c >> index 412f71d..0595675 100644 >> --- a/elf/dl-close.c >> +++ b/elf/dl-close.c >> @@ -108,7 +108,7 @@ remove_slotinfo (size_t idx, struct dtv_slotinfo_list *listp, size_t disp, >> >> >> void >> -_dl_close_worker (struct link_map *map) >> +_dl_close_worker (struct link_map *map, bool force) >> { >> /* One less direct use. */ >> --map->l_direct_opencount; >> @@ -152,6 +152,10 @@ _dl_close_worker (struct link_map *map) >> l->l_idx = idx; >> maps[idx] = l; >> ++idx; >> + >> + /* clear DF_1_NODELETE to force object deletion. */ >> + if (force) >> + l->l_flags_1 &= ~DF_1_NODELETE; > This will remove the NODELETE flag from *all* loaded objects. That > doesn't make sense. > > Andreas. > Indeed, we shouldn't remove NODELETE from all loaded objects, only for buggy library. Here a draft patch that should fix the issue. Andreas, does this look reasonable for you? If yes, I'll reformat it (e.g. add proper ChangeLog entry etc) and send for review as BZ#18778 fix. -Maxim diff --git a/elf/dl-close.c b/elf/dl-close.c index 9105277..5caf530 100644 --- a/elf/dl-close.c +++ b/elf/dl-close.c @@ -144,6 +144,14 @@ _dl_close_worker (struct link_map *map, bool force) char done[nloaded]; struct link_map *maps[nloaded]; + /* Clear DF_1_NODELETE to force object deletion. We don't need to touch + l_tls_dtor_count because forced object deletion only happens when an + error occurs during object load. Destructor registration for TLS + non-POD objects should not have happened till then for this + object. */ + if (force) + map->l_flags_1 &= ~DF_1_NODELETE; + /* Run over the list and assign indexes to the link maps and enter them into the MAPS array. */ int idx = 0; @@ -153,13 +161,6 @@ _dl_close_worker (struct link_map *map, bool force) maps[idx] = l; ++idx; - /* Clear DF_1_NODELETE to force object deletion. We don't need to touch - l_tls_dtor_count because forced object deletion only happens when an - error occurs during object load. Destructor registration for TLS - non-POD objects should not have happened till then for this - object. */ - if (force) - l->l_flags_1 &= ~DF_1_NODELETE; } assert (idx == nloaded); diff --git a/elf/tst-nodelete.cc b/elf/tst-nodelete.cc index 176cb68..4152bc3 100644 --- a/elf/tst-nodelete.cc +++ b/elf/tst-nodelete.cc @@ -1,12 +1,15 @@ #include "../dlfcn/dlfcn.h" #include #include +#include static int do_test (void) { int result = 0; + void *pthread = dlopen (LIBPTHREAD_SO, RTLD_LAZY); + /* This is a test for correct handling of dlopen failures for library that is loaded with RTLD_NODELETE flag. The first dlopen should fail because of undefined symbols in shared library. The second dlopen then verifies @@ -30,6 +33,9 @@ do_test (void) result = 1; } + if (pthread) + dlclose (pthread); + /* This is a test for correct handling of dlopen failures for library with unique symbols. The first dlopen should fail because of undefined symbols in shared library. The second dlopen then verifies that library diff --git a/elf/tst-znodelete-zlib.cc b/elf/tst-znodelete-zlib.cc deleted file mode 100644 index 1e8f368..0000000 --- a/elf/tst-znodelete-zlib.cc +++ /dev/null @@ -1,6 +0,0 @@ -extern int not_exist (void); - -int foo (void) -{ - return not_exist (); -}