From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fossa.ash.relay.mailchannels.net (fossa.ash.relay.mailchannels.net [23.83.222.62]) by sourceware.org (Postfix) with ESMTPS id 1891A385802E for ; Wed, 19 May 2021 04:24:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1891A385802E X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id C7C9F342A41; Wed, 19 May 2021 04:24:15 +0000 (UTC) Received: from pdx1-sub0-mail-a61.g.dreamhost.com (100-96-133-103.trex.outbound.svc.cluster.local [100.96.133.103]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id BFBEC34293F; Wed, 19 May 2021 04:24:14 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a61.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384) by 100.96.133.103 (trex/6.2.1); Wed, 19 May 2021 04:24:15 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Snatch-Thread: 3dde855807088477_1621398255283_3256937390 X-MC-Loop-Signature: 1621398255282:1689823012 X-MC-Ingress-Time: 1621398255282 Received: from pdx1-sub0-mail-a61.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a61.g.dreamhost.com (Postfix) with ESMTP id 6821D7E672; Tue, 18 May 2021 21:24:14 -0700 (PDT) Received: from [192.168.1.111] (unknown [1.186.101.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a61.g.dreamhost.com (Postfix) with ESMTPSA id 5BC5B7E663; Tue, 18 May 2021 21:24:12 -0700 (PDT) Subject: Re: [PATCH] ldconfig: Fix memory leaks To: Adhemerval Zanella , libc-alpha@sourceware.org References: <20210511171627.360100-1-siddhesh@sourceware.org> <1ba04d0c-6b76-94e4-4c60-ad2246aa1357@linaro.org> <65de069f-72f6-e8c5-b5e6-1930bdfae607@gotplt.org> <2cf48d15-86b7-4def-fc9f-80e7fd5a5dae@sourceware.org> <8bb09ee0-c85c-1e16-aa56-7a4b57982c71@linaro.org> X-DH-BACKEND: pdx1-sub0-mail-a61 From: Siddhesh Poyarekar Message-ID: <50a9c721-1e3f-5b16-1e4c-22fe0be80d3c@sourceware.org> Date: Wed, 19 May 2021 09:54:07 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <8bb09ee0-c85c-1e16-aa56-7a4b57982c71@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3494.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: , X-List-Received-Date: Wed, 19 May 2021 04:24:22 -0000 On 5/18/21 6:38 PM, Adhemerval Zanella wrote: > I think the straightforwards fix would be the below. My understanding > is chroot_canon tries to support a path larger than PATH_MAX, since > it basically prepends the CHROOT argument and then resolve the NAME. OK, now I understand what you meant. > I am not sure if this is really necessary, maybe we just allocate a > PATH_MAX buffer and use realpath on the [chroot,name] string. But that's not what you're doing... > diff --git a/elf/chroot_canon.c b/elf/chroot_canon.c > index 045611e730..1b10a3037f 100644 > --- a/elf/chroot_canon.c > +++ b/elf/chroot_canon.c > @@ -15,17 +15,10 @@ > You should have received a copy of the GNU General Public License > along with this program; if not, see . */ > > -#include > -#include > -#include > -#include > -#include > #include > -#include > -#include > - > -#include > #include > +#include > +#include > > #ifndef PATH_MAX > #define PATH_MAX 1024 > @@ -40,138 +33,18 @@ > char * > chroot_canon (const char *chroot, const char *name) > { > - char *rpath; > - char *dest; > - char *extra_buf = NULL; > - char *rpath_root; > - const char *start; > - const char *end; > - const char *rpath_limit; > - int num_links = 0; > size_t chroot_len = strlen (chroot); > - > if (chroot_len < 1) > { > __set_errno (EINVAL); > return NULL; > } > - > - rpath = xmalloc (chroot_len + PATH_MAX); > - > - rpath_limit = rpath + chroot_len + PATH_MAX; > - > - rpath_root = (char *) mempcpy (rpath, chroot, chroot_len) - 1; > - if (*rpath_root != '/') > - *++rpath_root = '/'; > - dest = rpath_root + 1; > - > - for (start = end = name; *start; start = end) > + char *rpath = xmalloc (chroot_len + PATH_MAX); > + char *rpath_root = (char *) mempcpy (rpath, chroot, chroot_len) - 1; > + if (realpath (name, rpath_root) == NULL) NAME is not a valid absolute path outside of the chroot; maybe what you need is: // XXX Free rpath before return. char *rpath = xmalloc (chroot_len + PATH_MAX + 1); char *rpath_ret = xmalloc (PATH_MAX); char *rpath_root = (char *) mempcpy (rpath, chroot, chroot_len) - 1; if (*rpath_root != '/') *++rpath_root = '/'; memcpy (rpath_root + 1, path, PATH_MAX); if (realpath (rpath, rpath_ret) == NULL) ... return rpath_ret; > { > - struct stat64 st; > - > - /* Skip sequence of multiple path-separators. */ > - while (*start == '/') > - ++start; > - > - /* Find end of path component. */ > - for (end = start; *end && *end != '/'; ++end) > - /* Nothing. */; > - > - if (end - start == 0) > - break; > - else if (end - start == 1 && start[0] == '.') > - /* nothing */; > - else if (end - start == 2 && start[0] == '.' && start[1] == '.') > - { > - /* Back up to previous component, ignore if at root already. */ > - if (dest > rpath_root + 1) > - while ((--dest)[-1] != '/'); > - } > - else > - { > - size_t new_size; > - > - if (dest[-1] != '/') > - *dest++ = '/'; > - > - if (dest + (end - start) >= rpath_limit) > - { > - ptrdiff_t dest_offset = dest - rpath; > - char *new_rpath; > - > - new_size = rpath_limit - rpath; > - if (end - start + 1 > PATH_MAX) > - new_size += end - start + 1; > - else > - new_size += PATH_MAX; > - new_rpath = (char *) xrealloc (rpath, new_size); > - rpath = new_rpath; > - rpath_limit = rpath + new_size; > - > - dest = rpath + dest_offset; > - } > - > - dest = mempcpy (dest, start, end - start); > - *dest = '\0'; > - > - if (lstat64 (rpath, &st) < 0) > - { > - if (*end == '\0') > - goto done; > - goto error; > - } > - > - if (S_ISLNK (st.st_mode)) > - { > - char *buf = alloca (PATH_MAX); > - size_t len; > - > - if (++num_links > __eloop_threshold ()) > - { > - __set_errno (ELOOP); > - goto error; > - } > - > - ssize_t n = readlink (rpath, buf, PATH_MAX - 1); > - if (n < 0) > - { > - if (*end == '\0') > - goto done; > - goto error; > - } > - buf[n] = '\0'; > - > - if (!extra_buf) > - extra_buf = alloca (PATH_MAX); > - > - len = strlen (end); > - if (len >= PATH_MAX - n) > - { > - __set_errno (ENAMETOOLONG); > - goto error; > - } > - > - /* Careful here, end may be a pointer into extra_buf... */ > - memmove (&extra_buf[n], end, len + 1); > - name = end = memcpy (extra_buf, buf, n); > - > - if (buf[0] == '/') > - dest = rpath_root + 1; /* It's an absolute symlink */ > - else > - /* Back up to previous component, ignore if at root already: */ > - if (dest > rpath_root + 1) > - while ((--dest)[-1] != '/'); > - } > - } > + free (rpath); > + return NULL; > } > - done: > - if (dest > rpath_root + 1 && dest[-1] == '/') > - --dest; > - *dest = '\0'; > - > return rpath; > - > - error: > - free (rpath); > - return NULL; > } >