From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27654 invoked by alias); 4 Feb 2019 19:58:48 -0000 Mailing-List: contact libc-stable-help@sourceware.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: List-Archive: Sender: libc-stable-owner@sourceware.org Received: (qmail 27621 invoked by uid 89); 4 Feb 2019 19:58:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.2 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,KAM_NUMSUBJECT,KAM_SHORT,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy= X-Spam-Status: No, score=-25.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,KAM_NUMSUBJECT,KAM_SHORT,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-HELO: mail-qt1-f175.google.com Received: from mail-qt1-f175.google.com (HELO mail-qt1-f175.google.com) (209.85.160.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Feb 2019 19:58:44 +0000 Received: by mail-qt1-f175.google.com with SMTP id e5so1298182qtr.12 for ; Mon, 04 Feb 2019 11:58:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp :organization:message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=+GoK6FJKvhCAdNl7AOgTiRXQ13ahtFRhdocXuK4llAY=; b=WaAUYhwchVr0tvAHjJuTdAYjOl6u67K7j71gxJHZ2k7GOHgnjU9vyn1Aa2m644mPEH JtAb1ByZ5DdYgD5XJUMT1p3Nb3HaL7s23orR0WIr6bbBOSrm+VXnYqS6CCuHNPESIvkn WkHj6i33tBwmITOjJs/I7gp0ihYCCrP7ydJ5Q47qWjXNLIwW6yzukdrlwycauf4IMRlR NrQN0RmFup3Yn68wTYNoJCrWM9Tg618y4qyl3g1uQDjgUvrHK/I0QRvpQC62vPONrn5O K/3petA992t8pzNfKDLLnCeibVhaSXNWtEf90vgi8udrCMP/k6NYQchjC8bT+ZtWg1dY SeZQ== X-Gm-Message-State: AHQUAua+MjHSaYn8hNnsb6XCE0/7JQS8QbJFHjL1yYviUsjvr++pbTm4 tMVoqmy9aEfunzy9dgrI6EKP/ZZFyCviJg== X-Google-Smtp-Source: AHgI3IaFW6zM2Nf4tmSIqaqP6TAxM4U8Del7QXUIvpd2oHUxiiy19C47TORi2IZQ82qoAlMbzQCKpw== X-Received: by 2002:a0c:c192:: with SMTP id n18mr828185qvh.99.1549310322057; Mon, 04 Feb 2019 11:58:42 -0800 (PST) Received: from [10.150.73.190] (125.sub-174-228-143.myvzw.com. [174.228.143.125]) by smtp.gmail.com with ESMTPSA id a3sm17223288qta.21.2019.02.04.11.58.40 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 Feb 2019 11:58:41 -0800 (PST) Subject: Re: Backporting CVE-2016-10739 To: Florian Weimer Cc: Aurelien Jarno , libc-stable@sourceware.org References: <20190204134254.GA13816@aurel32.net> <871s4nppu4.fsf@oldenburg2.str.redhat.com> <87r2cno9qq.fsf@oldenburg2.str.redhat.com> <0a9daa70-7ea9-1ebd-8690-04b6ff2acd88@redhat.com> <87munbo8wy.fsf@oldenburg2.str.redhat.com> <47ca567f-7120-19c5-7ed6-c67c9f6306ca@redhat.com> <87y36vmsr9.fsf@oldenburg2.str.redhat.com> <877eefmk3z.fsf@oldenburg2.str.redhat.com> From: Carlos O'Donell Openpgp: preference=signencrypt Organization: Red Hat Message-ID: <2e644830-b506-f5b6-e020-99fc9ee9b94f@redhat.com> Date: Tue, 01 Jan 2019 00:00:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <877eefmk3z.fsf@oldenburg2.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-SW-Source: 2019-02/txt/msg00017.txt.bz2 On 2/4/19 2:25 PM, Florian Weimer wrote: > * Carlos O'Donell: > >> On 2/4/19 11:18 AM, Florian Weimer wrote: >>> * Carlos O'Donell: >>> >>>> On 2/4/19 10:44 AM, Florian Weimer wrote: >>>>> * Carlos O'Donell: >>>>> >>>>>>> +#include >>>>>>> + >>>>>> >>>>>> Please add a comment explaining why this is here. >>>>> >>>>> You mean like this? >>>>> >>>>> /* Obtain the prototype for __inet_aton_exact. */ >>>> >>>> It should reference the bug or CVE to document the intent >>>> of the changes. >>>> >>>> Post v3 and I'll sign off? >>> >>> This approach does not actually work because copying a prototype this >>> way and adding a hidden visibility attribute does not actually make the >>> symbol hidden. The patch below however has the desired effect, mainly >>> because interposition no longer happens and the __inet_aton_exact_hidden >>> function is not added to the dynamic symbol table of the nscd >>> executable. >>> >>> I suspect I would have had to use __attribute__ ((visibility >>> ("hidden"))) directly because we define attribute_hidden thusly: >>> >>> #if defined SHARED || defined LIBC_NONSHARED \ >>> || (BUILD_PIE_DEFAULT && IS_IN (libc)) >>> # define attribute_hidden __attribute__ ((visibility ("hidden"))) >>> #else >>> # define attribute_hidden >>> #endif >>> >>> I can post yet another patch which uses real hidden visibility and >>> avoids the symbol redirect. >> >> This version is a little hack-ish, but I don't object to it. It does >> the job. >> >> It is certainly a minimal set of changes, and that makes it quite >> useful for the release branch. >> >> I don't think you need to go any further unless you think the version >> using real hidden visibility is all that much better? > > Well, at least it does not have the completely ineffective > attribute_hidden. 8-) I'm just providing high-level review here, I haven't tested these patches, I rely on you for that :-} > Patch below. What do you think? This looks good to me, you make direct use of "__attribute__ ((visibility ("hidden")))" in an exceptional case, and that's fine. If this becomes less rare for some reason we might want a libc-symbols.h macro to define something that expresses the intent of the hidden visibility e.g. attr_dup_sym_hidden. It's rare we have a fix that needs a new private interface, and that we can also just dup the code to fix the problem. Most likely I expect we *can't* dup the code and so need the new private interface to fix the bug, or we rework the fix entirely. > This one gets the symbol visibility correct. OK. > $ eu-readelf -s ../build/nscd/nscd-inet_addr.o | grep inet_aton_exact > 22: 0000000000000150 56 FUNC GLOBAL HIDDEN 1 __inet_aton_exact > $ eu-readelf -s ../build/nscd/gai.o | grep inet_aton_exact > 99: 0000000000000000 0 NOTYPE GLOBAL HIDDEN UNDEF __inet_aton_exact That's good. > > And there's no __inet_aton_exact symbol in nscd. I think this is fairly > standard usage of a hidden symbol, so renaming the symbol should not be > necessary. Perfect, and interposition is also not possible. > Thanks, > Florian > > nscd: Do not use __inet_aton_exact@GLIBC_PRIVATE [BZ #20018] > > This commit avoids referencing the __inet_aton_exact@GLIBC_PRIVATE > symbol from nscd. In master, the separately-compiled getaddrinfo > implementation in nscd needs it, however such an internal ABI change > is not desirable on a release branch if it can be avoided. > > 2019-02-04 Florian Weimer > > [BZ #20018] > nscd: Do not rely on new GLIBC_PRIVATE ABI after CVE-2016-10739 fix. > * nscd/nscd-inet_addr.c: New file. Build resolv/inet_addr.c for > nscd, without public symbols. > * nscd/Makefile (nscd-modules): Add it. > * nscd/gai.c: Include and change visibility of > __inet_aton_exact. OK for release/2.28/master. Reviewed-by: Carlos O'Donell > diff --git a/nscd/Makefile b/nscd/Makefile > index b713a84c49..eb23c01a39 100644 > --- a/nscd/Makefile > +++ b/nscd/Makefile > @@ -36,7 +36,7 @@ nscd-modules := nscd connections pwdcache getpwnam_r getpwuid_r grpcache \ > getsrvbynm_r getsrvbypt_r servicescache \ > dbg_log nscd_conf nscd_stat cache mem nscd_setup_thread \ > xmalloc xstrdup aicache initgrcache gai res_hconf \ > - netgroupcache > + netgroupcache nscd-inet_addr OK. > > ifeq ($(build-nscd)$(have-thread-library),yesyes) > > diff --git a/nscd/gai.c b/nscd/gai.c > index f57f396f57..68a4abd30e 100644 > --- a/nscd/gai.c > +++ b/nscd/gai.c > @@ -33,6 +33,12 @@ > #define __getifaddrs getifaddrs > #define __freeifaddrs freeifaddrs > > +/* We do not want to export __inet_aton_exact. Get the prototype and > + change its visibility to hidden. */ > +#include > +__typeof__ (__inet_aton_exact) __inet_aton_exact > + __attribute__ ((visibility ("hidden"))); OK. > + > /* We are nscd, so we don't want to be talking to ourselves. */ > #undef USE_NSCD > > diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c > new file mode 100644 > index 0000000000..f366b9567d > --- /dev/null > +++ b/nscd/nscd-inet_addr.c > @@ -0,0 +1,32 @@ > +/* Legacy IPv4 text-to-address functions. Version for nscd. > + Copyright (C) 2019 Free Software Foundation, Inc. OK. > + 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 > + > +/* We do not want to export __inet_aton_exact. Get the prototype and > + change the visibility to hidden. */ > +#include > +__typeof__ (__inet_aton_exact) __inet_aton_exact > + __attribute__ ((visibility ("hidden"))); OK. > + > +/* Do not provide definitions of the public symbols exported from > + libc. */ > +#undef weak_alias > +#define weak_alias(from, to) OK. > + > +#include > -- Cheers, Carlos.