From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70326 invoked by alias); 20 May 2018 02:48:49 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 69375 invoked by uid 89); 20 May 2018 02:46:26 -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,RCVD_IN_DNSWL_NONE,TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=readability X-HELO: mail-qt0-f194.google.com 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=V8CDV6+SxiniGADgfLz6vStbG50im8cvXmzoVOLTEHk=; b=KlD30wE6X9fokKWLVZVbDB9NNuFTB0hG+NWhEnGNRIrfOsghYsGGhREbcNPH8B3Ty1 i1UHuhJYop0Bv8SIUQjSOpYsHGFJGBdpzUHjOsZm4VpR6LTjagFh6/Vx8y+wKH4ymswB MthuGfvy/kEuFwKcmmwWykGL7XKp27PhEVS+OIOg3Gfcwt1NimfPndvn/gDujcjO1Rna GN+BNWt6UXWTpeaWZll7HZcPOd6T3dtVU7DGkV8No/J69FYs3JAt5POhXYEkvi8EsVDd KxhbiPqhtUzW+AIE3mv2qIzsHDxDKcudOggBw48/kpazuJ5tPXgpxOr2VgRSP/ZcNwlF 5LwA== X-Gm-Message-State: ALKqPweNhaKLxPUrhvYGQnZ6vuEwVyCdCZjzt4G5i0mcopxMaggGElkK ba1p9KKuM59FIbJ2PFguckdA8EykE3k= X-Google-Smtp-Source: AB8JxZquxm/b6Mn1oldprvrONt4B3M/v2CVPlKtSF3BhTDAIavMQ9uxskHqeTd2OojzPo/HC8mLk0w== X-Received: by 2002:ac8:2b37:: with SMTP id 52-v6mr14179040qtu.97.1526784359826; Sat, 19 May 2018 19:45:59 -0700 (PDT) Subject: Re: [RFC][PATCH v1 3/5] elf/dl-object.c: Implement a helper function to clone link_map entries To: =?UTF-8?Q?Vivek_Das=c2=a0Mohapatra?= , libc-alpha@sourceware.org Cc: =?UTF-8?Q?Vivek_Das=c2=a0Mohapatra?= References: <20180516171124.24962-1-vivek@collabora.com> <20180516171124.24962-4-vivek@collabora.com> From: Carlos O'Donell Openpgp: preference=signencrypt Message-ID: Date: Sun, 20 May 2018 02:48:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180516171124.24962-4-vivek@collabora.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2018-05/txt/msg00683.txt.bz2 On 05/16/2018 01:11 PM, Vivek Das Mohapatra wrote: > From: Vivek Das Mohapatra > > Provides the minimal functionality needed to take an existing > link_map entry and create a clone of it in the specified namespace. > --- > elf/dl-object.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++ > sysdeps/generic/ldsodefs.h | 6 ++++ > 2 files changed, 83 insertions(+) > > diff --git a/elf/dl-object.c b/elf/dl-object.c > index b37bcc1295..144ba6c993 100644 > --- a/elf/dl-object.c > +++ b/elf/dl-object.c > @@ -21,6 +21,7 @@ > #include > #include > #include > +#include > > #include > > @@ -50,6 +51,82 @@ _dl_add_to_namespace_list (struct link_map *new, Lmid_t nsid) > __rtld_lock_unlock_recursive (GL(dl_load_write_lock)); > } > > +/* Clone an existing link map entry into a new link map. */ Existing comment about the use of "clone" apply here too. > +/* This is based on _dl_new_object, skipping the steps we know we won't need Both of these comments should be merged into one. > + because this is mostly just a shell for the l_real pointer holding the real > + link map entry (normally l == l->l_real, but not for ld.so in non-main > + link maps or RTLD_SHARED clones). > + It also flags the clone by setting l_clone, and sets the the no-delete > + flag in the original. */ > +struct link_map * > +_dl_clone_object (struct link_map *old, int mode, Lmid_t nsid) > +{ > + const char *name; > + struct link_map *new; > + struct libname_list *newname; > +#ifdef SHARED > + /* See _dl_new_object for how this number is arrived at: */ > + unsigned int na = GLRO(dl_naudit) ?: ((mode & __RTLD_OPENEXEC) ? DL_NNS : 0); This is too complex. It should be expanded out for readability, and likely so anywhere else you see it, but let's start by making this copy easy to read. > + size_t audit_space = na * sizeof (new->l_audit[0]); > +#else > +# define audit_space 0 > +#endif > + > + name = old->l_name; > + > + /* Don't clone a clone: Go to the progenitor. */ I think we'd probably say "Find the original link map from the proxy." > + while (old && old->l_clone) > + old = old->l_real; > + > + if (old == NULL) > + _dl_signal_error (EINVAL, name, NULL, N_("cannot clone NULL link_map")); > + > + /* Object already exists in the target namespace. This should get handled > + by dl_open_worker but just in case we get this far, handle it: */ > + if (__glibc_unlikely (old->l_ns == nsid)) > + { > + /* Not actually possible, given the sanity checks above. */ > + if (old->l_clone) > + return old; > + > + _dl_signal_error (EEXIST, name, NULL, > + N_("object cannot be demoted to a clone")); > + } > + > + /* Now duplicate as little of _dl_new_object as possible to get a > + working cloned object in the target link map. */ > + new = (struct link_map *) calloc (sizeof (*new) + audit_space > + + sizeof (struct link_map *) > + + sizeof (*newname) + PATH_MAX, 1); calloc could fail, you need to _dl_signal_error ENOMEM. > + > + /* Specific to the clone. */ > + new->l_real = old; > + new->l_clone = 1; > + new->l_ns = nsid; > + > + /* Copied from the origin. */ > + new->l_libname = old->l_libname; > + new->l_name = old->l_name; > + new->l_type = old->l_type; > + > + if (__glibc_unlikely (mode & RTLD_NODELETE)) > + new->l_flags_1 |= DF_1_NODELETE; > + > + /* Specific to the origin. Ideally we'd do some accounting here but > + for now it's easier to pin the original so the clone remains valid. */ > + old->l_flags_1 |= DF_1_NODELETE; > + > + /* Fix up the searchlist so that relocations work. */ > + /* TODO: figure out if we should call _dl_map_object_deps > + or copy the contents of l_scope, l_searchlist et al. */ > + _dl_map_object_deps (new, NULL, 0, 0, > + mode & (__RTLD_DLOPEN | RTLD_DEEPBIND | __RTLD_AUDIT)); Calling _dl_map_object_deps seems like a conservative thing to do here. > + > + /* And finally put the clone into the target namespace. */ > + _dl_add_to_namespace_list (new, nsid); > + > + return new; > +} > > /* Allocate a `struct link_map' for a new object being loaded, > and enter it into the _dl_loaded list. */ > diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h > index 5e1b24ecb5..573d24f611 100644 > --- a/sysdeps/generic/ldsodefs.h > +++ b/sysdeps/generic/ldsodefs.h > @@ -913,6 +913,12 @@ extern lookup_t _dl_lookup_symbol_x (const char *undef, > extern void _dl_add_to_namespace_list (struct link_map *new, Lmid_t nsid) > attribute_hidden; > > +/* Clone an existing link map entry into a new link map */ > +extern struct link_map *_dl_clone_object (struct link_map *old, > + int mode, > + Lmid_t nsid) > + attribute_hidden; OK. But I think we'd call this _dl_proxy_object (...); > + > /* Allocate a `struct link_map' for a new object being loaded. */ > extern struct link_map *_dl_new_object (char *realname, const char *libname, > int type, struct link_map *loader, > -- Cheers, Carlos.