From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62096 invoked by alias); 4 Feb 2019 15:31:32 -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 62075 invoked by uid 89); 4 Feb 2019 15:31:31 -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=-26.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_NUMSUBJECT,KAM_SHORT,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=hacking, hide, Hide X-Spam-Status: No, score=-26.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,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-f182.google.com Received: from mail-qt1-f182.google.com (HELO mail-qt1-f182.google.com) (209.85.160.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Feb 2019 15:31:28 +0000 Received: by mail-qt1-f182.google.com with SMTP id b8so258662qtr.9 for ; Mon, 04 Feb 2019 07:31:28 -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=bLixmAaZ9jT2gyNmR+0SgnLb1TYmi2OOxZbUje33RyA=; b=f2/cU8Sz0NyaXWKPMAhftB5qI5f10cE6hbBZglTVm9R7KAPEpkvSLwVz23i3c0OiUN 08UZuB0Ioo4HzkdSJspgj+jA3rX/JqIVMhduf0GdfT1lSc2mwytb7Q0OlPTnzf6ZYslc TUJQnHqf0LAoYlH/YhGj2yaKtgQ/Ou0U3H5HZgKCYT+yWwZVhPWD9i8j8NUaXlAtWvIE 6ZujFtuOEW4msRWAty8/CCHaeCY8YzbE3QJUfNGj4gnBUUCyVamFWLZX0nbfv2Rutxl4 TEdQLCrY4npt0g9vYee2byN6W1MAGY0rD5p4A28CEY65FcGi/MlO4z2qphQXhykJ+kam 6jbw== X-Gm-Message-State: AHQUAuYduBqNqPk6YIwA9iuspbBzZNGxIx6df0zUyfmUkP2xGaUVsLZ8 VU2q7L51zMxeJli1b8ju+d2gESYVe+aqyA== X-Google-Smtp-Source: AHgI3IZuhGiXwvgAGZdLoH25FYf5Hdx3fd5E93RHmLFksH+eLm3RrN43x7yBAaxbTthhUmhl64zibw== X-Received: by 2002:ad4:42d1:: with SMTP id f17mr11168223qvr.59.1549294286565; Mon, 04 Feb 2019 07:31:26 -0800 (PST) Received: from [10.150.73.190] (75.sub-174-228-15.myvzw.com. [174.228.15.75]) by smtp.gmail.com with ESMTPSA id b27sm6529619qkj.17.2019.02.04.07.31.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 Feb 2019 07:31:25 -0800 (PST) Subject: Re: Backporting CVE-2016-10739 To: Florian Weimer , Aurelien Jarno Cc: libc-stable@sourceware.org References: <20190204134254.GA13816@aurel32.net> <871s4nppu4.fsf@oldenburg2.str.redhat.com> From: Carlos O'Donell Openpgp: preference=signencrypt Organization: Red Hat Message-ID: <81fea955-b84f-049e-7e1b-a3b5c3d80b7a@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: <871s4nppu4.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/msg00009.txt.bz2 On 2/4/19 9:53 AM, Florian Weimer wrote: > Here is the patch I'm testing to restore ABI after the regular > backports. This looks fairly reasonable to me as far as such things > go. > > I've put all patches on the fw/bug20018-backport branch on sourceware as > well. > > Thanks, > Florian > > Restore GLIBC_PRIVATE ABI after CVE-2016-10739 fix [BZ #20018] Shouldn't this say "Remove GLIBC_PRIVATE ABI" ? I admit this confused me for a while since the patch clearly removes the ABI from resolv/Versions. > This commit avoids adding the __inet_aton_exact@GLIBC_PRIVATE > symbol. 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 easily. > > 2019-02-04 Florian Weimer > > [BZ #20018] > Restore GLIBC_PRIVATE ABI after CVE-2016-10739 fix. s/Restore/Remove/g > * include/arpa/inet.h (__inet_aton_exact): Declare as hidden. > * resolv/inet_addr.c (__inet_aton_exact): Remove libc_hidden_def. > * resolv/Versions (GLIBC_PRIVATE): Do not export > __inet_aton_exact. > * nscd/nscd-inet_addr.c: New file. Build resolv/inet_addr.c for > nscd, without public symbols. > * nscd/Makefile (nscd-modules): Add it. > > diff --git a/include/arpa/inet.h b/include/arpa/inet.h > index 19aec74275..dce60b4909 100644 > --- a/include/arpa/inet.h > +++ b/include/arpa/inet.h > @@ -2,8 +2,8 @@ > > #ifndef _ISOMAC > /* Variant of inet_aton which rejects trailing garbage. */ > -extern int __inet_aton_exact (const char *__cp, struct in_addr *__inp); > -libc_hidden_proto (__inet_aton_exact) > +extern int __inet_aton_exact (const char *__cp, struct in_addr *__inp) > + attribute_hidden; Why do you mark this attribute_hidden? You should remove this from the public header, and move it to a private header that we already have. Again, this might be a balance question, between too many changes in the release branch with respect to master. Is your goal here to minimize the changes if we need future backports? I'm OK with this, but please add a comment here saying this. We should use comments to help reviewers and mergers undrestand why this is different. e.g. /* CVE-2016-10739: Hide this interface on the stable release branch. */ > > libc_hidden_proto (inet_ntop) > libc_hidden_proto (inet_pton) > 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, add a new object file to the nscd build. > > ifeq ($(build-nscd)$(have-thread-library),yesyes) > > diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c > new file mode 100644 > index 0000000000..cfa4ac7462 > --- /dev/null > +++ b/nscd/nscd-inet_addr.c > @@ -0,0 +1,24 @@ > +/* 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 > + . */ > + > +/* Do not provide definitions of the public symbols exported from > + libc. */ > +#undef weak_alias > +#define weak_alias(from, to) Why do we have to play with the weak aliases? Because they create visible symbols at the new name? > + > +#include OK, recompile inet_addr* as nscd-inet_addr* > diff --git a/resolv/Versions b/resolv/Versions > index 9a82704af7..b05778d965 100644 > --- a/resolv/Versions > +++ b/resolv/Versions > @@ -27,7 +27,6 @@ libc { > __h_errno; __resp; > > __res_iclose; > - __inet_aton_exact; OK, remove from libc's GLIBC_PRIVATE ABI. > __inet_pton_length; > __resolv_context_get; > __resolv_context_get_preinit; > diff --git a/resolv/inet_addr.c b/resolv/inet_addr.c > index 41b6166a5b..1bc4a2c4d6 100644 > --- a/resolv/inet_addr.c > +++ b/resolv/inet_addr.c > @@ -192,7 +192,6 @@ __inet_aton_exact (const char *cp, struct in_addr *addr) > else > return 0; > } > -libc_hidden_def (__inet_aton_exact) Why remove this? Why not redefine it like you did for weak alias? If we are going to modify inet_addr.c, we can modify it such that we avoid hacking weak_alias *or* hack both? Like -DNSCD_INET_ADDR=0 from the Makefile for the normal build and then #define NSCD_INET_ADDR 1 in the nscd-inet_addr.c sources? Something cleaner that makes it obvious there is an interface here between nscd/libc at the source level and that we are employing source-level reuse. I know it makes the change a bit bigger and that we need to balance a minimal change on a release branch, but it seems like we could be a little bit more explicit with our changes. Likewise if we're going to remove things, I think there is value in comments explaining why this is missing. That way if you get a merge failure you have a pointer to the reason, rather than nothing. It documents intent. > > /* inet_aton ignores trailing garbage. */ > int > -- Cheers, Carlos.