From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS17314 8.43.84.0/22 X-Spam-Status: No, score=-3.7 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI,NICE_REPLY_A, PDS_RDNS_DYNAMIC_FP,RCVD_IN_DNSWL_HI,RDNS_DYNAMIC,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id F13621F8C6 for ; Thu, 15 Jul 2021 05:24:17 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1F4BE3AA7C9E for ; Thu, 15 Jul 2021 05:24:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1F4BE3AA7C9E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1626326657; bh=IFIndYj6d3bPFhoZel2g0+r+/iMQxIhOlLs9X9bfcOI=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=cLL276ZAXSBVOh6i17iPkhfyRy9u6jygoUjPLwzo9oVtOeIKtuQrQXFUloNmH2rUd 9afQPlXC3hFjopG3X5NbIe5Nk+Vsvk0beHP/MHlgBq3s1Iweq9ivFpgv6ptjiUlApk 8hJAfgtRSFexA6zT4L/I8TDIHMv2ahoAIipeAtIM= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 062C9386103A for ; Thu, 15 Jul 2021 05:02:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 062C9386103A Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-398-IBc0RsiDPfmlpt_rRGqDJA-1; Thu, 15 Jul 2021 01:02:48 -0400 X-MC-Unique: IBc0RsiDPfmlpt_rRGqDJA-1 Received: by mail-qv1-f71.google.com with SMTP id cw12-20020ad44dcc0000b02902f1474ce8b7so3233696qvb.20 for ; Wed, 14 Jul 2021 22:02:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=IFIndYj6d3bPFhoZel2g0+r+/iMQxIhOlLs9X9bfcOI=; b=O2RUfb16/Z0f93+0biAuGVObkOtKnBu1Ak8lB/cAqWiXEE98xF7qL8aHhuI1IRqe6v cSun2tbphcNlzjJl2CjoWtrZknWSirtXenqa5eQ0GO6sHSiQcftZPJ0tCTPesZRkombZ fBGX+xa/U6Gyk14fMAI0TveleDt2guctqbJiSZCoUmxWFk3Wfy6Ke1mSZ326rEBGi7U6 kBVrTrVTdvXaYMuLLislbFm1jPB6sui5/LDXZRe/rgAuzcTU7zHoU4dP3GHktHGC7PIO 59IladN0xkBXLJt43rT0uEyfufHUZLB5KbOSsuDLApf0RzRmnvE8pu5/2lmRAvvGeP3S wRQQ== X-Gm-Message-State: AOAM533WKWtlGgUcHDeG6PNqpBOPy+cfEUEIAxG+jEPSzPne6fq+kW2Q mtnKszbR0+E0o8G95syYDHnbnXHM/DTB0FDDhLBz3P+WUJYm4f5XRkQIAxmDyorSYZKm8VzcRBN Qw/mx19daniZuG952qamtTfMlPNOb5f5jHgVAxFagtBH9068Xncj9y4cBItObaft8P8MVJQ== X-Received: by 2002:a05:622a:3:: with SMTP id x3mr2136999qtw.81.1626325367854; Wed, 14 Jul 2021 22:02:47 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyplwzb0TsMcBdgeKeREx6p/7Pa8j6wWmY1VQYiRz/6FXsgxTh6ZmHRYcTwxdigeJ2yEdDQqg== X-Received: by 2002:a05:622a:3:: with SMTP id x3mr2136973qtw.81.1626325367621; Wed, 14 Jul 2021 22:02:47 -0700 (PDT) Received: from [192.168.1.16] (198-84-214-74.cpe.teksavvy.com. [198.84.214.74]) by smtp.gmail.com with ESMTPSA id 71sm1730517qtc.97.2021.07.14.22.02.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 14 Jul 2021 22:02:47 -0700 (PDT) Subject: Re: [PATCH 30/30] nss: Directly load nss_dns, without going through dlsym/dlopen To: Florian Weimer , libc-alpha@sourceware.org References: <173b8b3328d263fd01e89562685a5dc14fd37b25.1625755446.git.fweimer@redhat.com> Organization: Red Hat Message-ID: <95d3d5ec-702e-cf87-970a-a20e59fa1bfb@redhat.com> Date: Thu, 15 Jul 2021 01:02:46 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <173b8b3328d263fd01e89562685a5dc14fd37b25.1625755446.git.fweimer@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Carlos O'Donell via Libc-alpha Reply-To: Carlos O'Donell Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On 7/8/21 11:07 AM, Florian Weimer via Libc-alpha wrote: > This partially fixes static-only NSS support (bug 27959): The dns > module no longer needs dlopen. Support for the files module remains > to be added, and also support for disabling dlopen altogher. Noted that files is already included and commit message is adjusted. The design here looks good, we split out files and dns and handle them as builtins with only a handful of functions to initialize them without too much abstraction. OK for glibc 2.34. Tested without regression on x86_64 and i686. Reviewed-by: Carlos O'Donell Tested-by: Carlos O'Donell > This commit introduces module_load_builtin into nss/nss_module.c, which > handles the common parts of loading the built-in nss_files and nss_dns > modules. > --- > include/nss_dns.h | 13 +++++---- > nss/nss_files_functions.c | 6 ----- > nss/nss_module.c | 55 +++++++++++++++++++++++++++++--------- > nss/nss_module.h | 10 +++++-- > resolv/Makefile | 1 + > resolv/nss_dns_functions.c | 40 +++++++++++++++++++++++++++ > 6 files changed, 99 insertions(+), 26 deletions(-) > create mode 100644 resolv/nss_dns_functions.c > > diff --git a/include/nss_dns.h b/include/nss_dns.h > index 63b5853870..53205b27a6 100644 > --- a/include/nss_dns.h > +++ b/include/nss_dns.h > @@ -24,13 +24,16 @@ > NSS_DECLARE_MODULE_FUNCTIONS (dns) > > libc_hidden_proto (_nss_dns_getcanonname_r) > -libc_hidden_proto (_nss_dns_gethostbyname3_r) > -libc_hidden_proto (_nss_dns_gethostbyname2_r) > -libc_hidden_proto (_nss_dns_gethostbyname_r) > -libc_hidden_proto (_nss_dns_gethostbyname4_r) OK. > libc_hidden_proto (_nss_dns_gethostbyaddr2_r) > libc_hidden_proto (_nss_dns_gethostbyaddr_r) > -libc_hidden_proto (_nss_dns_getnetbyname_r) > +libc_hidden_proto (_nss_dns_gethostbyname2_r) > +libc_hidden_proto (_nss_dns_gethostbyname3_r) > +libc_hidden_proto (_nss_dns_gethostbyname4_r) > +libc_hidden_proto (_nss_dns_gethostbyname_r) OK. Sort. > libc_hidden_proto (_nss_dns_getnetbyaddr_r) > +libc_hidden_proto (_nss_dns_getnetbyname_r) > + > +void __nss_dns_functions (nss_module_functions_untyped pointers) > + attribute_hidden; OK. > > #endif > diff --git a/nss/nss_files_functions.c b/nss/nss_files_functions.c > index 85720b4311..46040fff70 100644 > --- a/nss/nss_files_functions.c > +++ b/nss/nss_files_functions.c > @@ -34,10 +34,4 @@ __nss_files_functions (nss_module_functions_untyped pointers) > #undef DEFINE_NSS_FUNCTION > #define DEFINE_NSS_FUNCTION(x) *fptr++ = _nss_files_##x; > #include "function.def" > - > -#ifdef PTR_MANGLE > - void **end = fptr; > - for (fptr = pointers; fptr != end; ++fptr) > - PTR_MANGLE (*fptr); > -#endif OK. No more PTR_MANGLE handling required because we are not loaded. > } > diff --git a/nss/nss_module.c b/nss/nss_module.c > index 7b42c585a4..a4a66866fb 100644 > --- a/nss/nss_module.c > +++ b/nss/nss_module.c > @@ -26,11 +26,13 @@ > #include > #include > #include > +#include > +#include OK. > #include > #include > #include > #include > -#include > +#include > > /* Suffix after .so of NSS service modules. This is a bit of magic, > but we assume LIBNSS_FILES_SO looks like "libnss_files.so.2" and we > @@ -111,18 +113,12 @@ static const function_name nss_function_name_array[] = > #include "function.def" > }; > > +/* Loads a built-in module, binding the symbols using the supplied > + callback function. Always returns true. */ > static bool > -module_load_nss_files (struct nss_module *module) > +module_load_builtin (struct nss_module *module, > + void (*bind) (nss_module_functions_untyped)) > { > - if (is_nscd) > - { > - void (*cb) (size_t, struct traced_file *) = nscd_init_cb; > -# ifdef PTR_DEMANGLE > - PTR_DEMANGLE (cb); > -# endif > - _nss_files_init (cb); > - } > - > /* Initialize the function pointers, following the double-checked > locking idiom. */ > __libc_lock_lock (nss_module_list_lock); > @@ -130,7 +126,13 @@ module_load_nss_files (struct nss_module *module) > { > case nss_module_uninitialized: > case nss_module_failed: > - __nss_files_functions (module->functions.untyped); > + bind (module->functions.untyped); > + > +#ifdef PTR_MANGLE > + for (int i = 0; i < nss_module_functions_count; ++i) > + PTR_MANGLE (module->functions.untyped[i]); > +#endif > + > module->handle = NULL; > /* Synchronizes with unlocked __nss_module_load atomic_load_acquire. */ > atomic_store_release (&module->state, nss_module_loaded); > @@ -143,12 +145,37 @@ module_load_nss_files (struct nss_module *module) > return true; > } > > +/* Loads the built-in nss_files module. */ > +static bool > +module_load_nss_files (struct nss_module *module) > +{ > + if (is_nscd) > + { > + void (*cb) (size_t, struct traced_file *) = nscd_init_cb; > +# ifdef PTR_DEMANGLE > + PTR_DEMANGLE (cb); > +# endif > + _nss_files_init (cb); > + } > + return module_load_builtin (module, __nss_files_functions); > +} OK. > + > +/* Loads the built-in nss_dns module. */ > +static bool > +module_load_nss_dns (struct nss_module *module) > +{ > + return module_load_builtin (module, __nss_dns_functions); > +} OK. > + > + > /* Internal implementation of __nss_module_load. */ > static bool > module_load (struct nss_module *module) > { > if (strcmp (module->name, "files") == 0) > return module_load_nss_files (module); > + if (strcmp (module->name, "dns") == 0) > + return module_load_nss_dns (module); OK. Handle specifically. > > void *handle; > { > @@ -396,7 +423,9 @@ __nss_module_freeres (void) > struct nss_module *current = nss_module_list; > while (current != NULL) > { > - if (current->state == nss_module_loaded && current->handle != NULL) > + /* Ignore built-in modules (which have a NULL handle). */ > + if (current->state == nss_module_loaded > + && current->handle != NULL) OK. > __libc_dlclose (current->handle); > > struct nss_module *next = current->next; > diff --git a/nss/nss_module.h b/nss/nss_module.h > index c1a1d90b60..b52c2935d2 100644 > --- a/nss/nss_module.h > +++ b/nss/nss_module.h > @@ -33,10 +33,16 @@ struct nss_module_functions > #include "function.def" > }; > > +/* Number of elements of the nss_module_functions_untyped array. */ > +enum > + { > + nss_module_functions_count = (sizeof (struct nss_module_functions) > + / sizeof (void *)) > + }; OK. > + > /* Untyped version of struct nss_module_functions, for consistent > processing purposes. */ > -typedef void *nss_module_functions_untyped[sizeof (struct nss_module_functions) > - / sizeof (void *)]; > +typedef void *nss_module_functions_untyped[nss_module_functions_count]; OK. > > /* Locate the nss_files functions, as if by dlopen/dlsym. */ > void __nss_files_functions (nss_module_functions_untyped pointers) > diff --git a/resolv/Makefile b/resolv/Makefile > index dd0a98c74f..31d27454b4 100644 > --- a/resolv/Makefile > +++ b/resolv/Makefile > @@ -48,6 +48,7 @@ routines := \ > ns_name_unpack \ > ns_samename \ > nsap_addr \ > + nss_dns_functions \ OK. > res-close \ > res-name-checking \ > res-state \ > diff --git a/resolv/nss_dns_functions.c b/resolv/nss_dns_functions.c > new file mode 100644 > index 0000000000..158dafec90 > --- /dev/null > +++ b/resolv/nss_dns_functions.c > @@ -0,0 +1,40 @@ > +/* Direct access for nss_dns functions for NSS module loading. OK. > + Copyright (C) 2021 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 > + > +void > +__nss_dns_functions (nss_module_functions_untyped pointers) > +{ > + struct nss_module_functions typed = > + { > + .getcanonname_r = &_nss_dns_getcanonname_r, > + .gethostbyname3_r = &_nss_dns_gethostbyname3_r, > + .gethostbyname2_r = &_nss_dns_gethostbyname2_r, > + .gethostbyname_r = &_nss_dns_gethostbyname_r, > + .gethostbyname4_r = &_nss_dns_gethostbyname4_r, > + .gethostbyaddr2_r = &_nss_dns_gethostbyaddr2_r, > + .gethostbyaddr_r = &_nss_dns_gethostbyaddr_r, > + .getnetbyname_r = &_nss_dns_getnetbyname_r, > + .getnetbyaddr_r = &_nss_dns_getnetbyaddr_r, > + }; > + > + memcpy (pointers, &typed, sizeof (nss_module_functions_untyped)); OK. > +} > -- Cheers, Carlos.