From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x841.google.com (mail-qt1-x841.google.com [IPv6:2607:f8b0:4864:20::841]) by sourceware.org (Postfix) with ESMTPS id B86E4395185F for ; Tue, 27 Oct 2020 12:59:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B86E4395185F Received: by mail-qt1-x841.google.com with SMTP id r8so822670qtp.13 for ; Tue, 27 Oct 2020 05:59:37 -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:from:to:cc:references:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=8vLRebbTTW9qg30TO/kxWQ9yIbjN6tz3f9hcNV5FSgk=; b=ddcYO7EkuFef7ln1sBZrfwWJsYAbHdmhAUa8CLjjuhuLaRMvC8kTZmAECilOnjVoXl cJgaejMBEczV9+/n4VSr/CKj9OGVeAZ3NHr+jh5hJ8tzWdD0TsSEKJsxvl+m+mZbmkCF A+a29YqRsGMRL3yUa4DfDO7pWffho3snLwPS5P0mkQ4LO9vlyuITmcRCNC06mDYT0g2p CTcI2ujJw1unMz9fRFW5fhpTsDziqYvybui9DCZZkKIzJ3kUD37Cy+Mv6VgCsHsYTSjo FnLqoZDB9oPFcl3vPeY5+tuHwj8h+hiS33udbn39yZd9VecPTBIfHdzmlnsgjFkd2GLr bicA== X-Gm-Message-State: AOAM532TUYrucmRTOz4JCEchKhYHAfcZJthqiaDbOO458Ji08hLZ9MAl KjNjSSWLi5sxLQW21c9cgE57ZHFOWzqSdQ== X-Google-Smtp-Source: ABdhPJwDbvdMfMOQHTQ9UtuyWsJJpJDlRzHTcj9W2NgoaEpzmbY1Hos+N3UAObCT9TY+OdTUqw6MIg== X-Received: by 2002:aed:22fa:: with SMTP id q55mr1852430qtc.229.1603803577138; Tue, 27 Oct 2020 05:59:37 -0700 (PDT) Received: from [192.168.1.4] ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id o14sm725294qto.16.2020.10.27.05.59.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 27 Oct 2020 05:59:36 -0700 (PDT) Subject: Re: [PATCH v3 2/4 v2] stdlib: Use fixed buffer size for realpath [BZ #26241] From: Adhemerval Zanella To: libc-alpha@sourceware.org References: <20200910151915.1982465-1-adhemerval.zanella@linaro.org> <20200910151915.1982465-2-adhemerval.zanella@linaro.org> Autocrypt: addr=adhemerval.zanella@linaro.org; prefer-encrypt=mutual; keydata= mQINBFcVGkoBEADiQU2x/cBBmAVf5C2d1xgz6zCnlCefbqaflUBw4hB/bEME40QsrVzWZ5Nq 8kxkEczZzAOKkkvv4pRVLlLn/zDtFXhlcvQRJ3yFMGqzBjofucOrmdYkOGo0uCaoJKPT186L NWp53SACXguFJpnw4ODI64ziInzXQs/rUJqrFoVIlrPDmNv/LUv1OVPKz20ETjgfpg8MNwG6 iMizMefCl+RbtXbIEZ3TE/IaDT/jcOirjv96lBKrc/pAL0h/O71Kwbbp43fimW80GhjiaN2y WGByepnkAVP7FyNarhdDpJhoDmUk9yfwNuIuESaCQtfd3vgKKuo6grcKZ8bHy7IXX1XJj2X/ BgRVhVgMHAnDPFIkXtP+SiarkUaLjGzCz7XkUn4XAGDskBNfbizFqYUQCaL2FdbW3DeZqNIa nSzKAZK7Dm9+0VVSRZXP89w71Y7JUV56xL/PlOE+YKKFdEw+gQjQi0e+DZILAtFjJLoCrkEX w4LluMhYX/X8XP6/C3xW0yOZhvHYyn72sV4yJ1uyc/qz3OY32CRy+bwPzAMAkhdwcORA3JPb kPTlimhQqVgvca8m+MQ/JFZ6D+K7QPyvEv7bQ7M+IzFmTkOCwCJ3xqOD6GjX3aphk8Sr0dq3 4Awlf5xFDAG8dn8Uuutb7naGBd/fEv6t8dfkNyzj6yvc4jpVxwARAQABtElBZGhlbWVydmFs IFphbmVsbGEgTmV0dG8gKExpbmFybyBWUE4gS2V5KSA8YWRoZW1lcnZhbC56YW5lbGxhQGxp bmFyby5vcmc+iQI3BBMBCAAhBQJXFRpKAhsDBQsJCAcDBRUKCQgLBRYCAwEAAh4BAheAAAoJ EKqx7BSnlIjv0e8P/1YOYoNkvJ+AJcNUaM5a2SA9oAKjSJ/M/EN4Id5Ow41ZJS4lUA0apSXW NjQg3VeVc2RiHab2LIB4MxdJhaWTuzfLkYnBeoy4u6njYcaoSwf3g9dSsvsl3mhtuzm6aXFH /Qsauav77enJh99tI4T+58rp0EuLhDsQbnBic/ukYNv7sQV8dy9KxA54yLnYUFqH6pfH8Lly sTVAMyi5Fg5O5/hVV+Z0Kpr+ZocC1YFJkTsNLAW5EIYSP9ftniqaVsim7MNmodv/zqK0IyDB GLLH1kjhvb5+6ySGlWbMTomt/or/uvMgulz0bRS+LUyOmlfXDdT+t38VPKBBVwFMarNuREU2 69M3a3jdTfScboDd2ck1u7l+QbaGoHZQ8ZNUrzgObltjohiIsazqkgYDQzXIMrD9H19E+8fw kCNUlXxjEgH/Kg8DlpoYJXSJCX0fjMWfXywL6ZXc2xyG/hbl5hvsLNmqDpLpc1CfKcA0BkK+ k8R57fr91mTCppSwwKJYO9T+8J+o4ho/CJnK/jBy1pWKMYJPvvrpdBCWq3MfzVpXYdahRKHI ypk8m4QlRlbOXWJ3TDd/SKNfSSrWgwRSg7XCjSlR7PNzNFXTULLB34sZhjrN6Q8NQZsZnMNs TX8nlGOVrKolnQPjKCLwCyu8PhllU8OwbSMKskcD1PSkG6h3r0AquQINBFcVGkoBEACgAdbR Ck+fsfOVwT8zowMiL3l9a2DP3Eeak23ifdZG+8Avb/SImpv0UMSbRfnw/N81IWwlbjkjbGTu oT37iZHLRwYUFmA8fZX0wNDNKQUUTjN6XalJmvhdz9l71H3WnE0wneEM5ahu5V1L1utUWTyh VUwzX1lwJeV3vyrNgI1kYOaeuNVvq7npNR6t6XxEpqPsNc6O77I12XELic2+36YibyqlTJIQ V1SZEbIy26AbC2zH9WqaKyGyQnr/IPbTJ2Lv0dM3RaXoVf+CeK7gB2B+w1hZummD21c1Laua +VIMPCUQ+EM8W9EtX+0iJXxI+wsztLT6vltQcm+5Q7tY+HFUucizJkAOAz98YFucwKefbkTp eKvCfCwiM1bGatZEFFKIlvJ2QNMQNiUrqJBlW9nZp/k7pbG3oStOjvawD9ZbP9e0fnlWJIsj 6c7pX354Yi7kxIk/6gREidHLLqEb/otuwt1aoMPg97iUgDV5mlNef77lWE8vxmlY0FBWIXuZ yv0XYxf1WF6dRizwFFbxvUZzIJp3spAao7jLsQj1DbD2s5+S1BW09A0mI/1DjB6EhNN+4bDB SJCOv/ReK3tFJXuj/HbyDrOdoMt8aIFbe7YFLEExHpSk+HgN05Lg5TyTro8oW7TSMTk+8a5M kzaH4UGXTTBDP/g5cfL3RFPl79ubXwARAQABiQIfBBgBCAAJBQJXFRpKAhsMAAoJEKqx7BSn lIjvI/8P/jg0jl4Tbvg3B5kT6PxJOXHYu9OoyaHLcay6Cd+ZrOd1VQQCbOcgLFbf4Yr+rE9l mYsY67AUgq2QKmVVbn9pjvGsEaz8UmfDnz5epUhDxC6yRRvY4hreMXZhPZ1pbMa6A0a/WOSt AgFj5V6Z4dXGTM/lNManr0HjXxbUYv2WfbNt3/07Db9T+GZkpUotC6iknsTA4rJi6u2ls0W9 1UIvW4o01vb4nZRCj4rni0g6eWoQCGoVDk/xFfy7ZliR5B+3Z3EWRJcQskip/QAHjbLa3pml xAZ484fVxgeESOoaeC9TiBIp0NfH8akWOI0HpBCiBD5xaCTvR7ujUWMvhsX2n881r/hNlR9g fcE6q00qHSPAEgGr1bnFv74/1vbKtjeXLCcRKk3Ulw0bY1OoDxWQr86T2fZGJ/HIZuVVBf3+ gaYJF92GXFynHnea14nFFuFgOni0Mi1zDxYH/8yGGBXvo14KWd8JOW0NJPaCDFJkdS5hu0VY 7vJwKcyHJGxsCLU+Et0mryX8qZwqibJIzu7kUJQdQDljbRPDFd/xmGUFCQiQAncSilYOcxNU EMVCXPAQTteqkvA+gNqSaK1NM9tY0eQ4iJpo+aoX8HAcn4sZzt2pfUB9vQMTBJ2d4+m/qO6+ cFTAceXmIoFsN8+gFN3i8Is3u12u8xGudcBPvpoy4OoG Message-ID: <0cdc1370-3105-00a5-85e8-55d2b2a6bb7b@linaro.org> Date: Tue, 27 Oct 2020 09:59:34 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20200910151915.1982465-2-adhemerval.zanella@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-15.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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: Tue, 27 Oct 2020 12:59:40 -0000 Ping. On 10/09/2020 12:19, Adhemerval Zanella wrote: > Changes from previous version [1]: > > - I have abandoned the scratch_buffer usage, the PATH_MAX stack usage > is acceptable and it avoid the potentially malloc usage. > > [1] https://sourceware.org/pipermail/libc-alpha/2020-August/117078.html > > --- > > It uses both a fixed internal buffer with PATH_MAX size to read and > copy the results of the readlink call. > > Also, if PATH_MAX is not defined it uses a default value of 1024 > as for other stdlib implementations. > > The expected stack usage is about 8k on Linux where PATH_MAX is > define as 4096 (plus some internal function usage for local > variable). > > Checked on x86_64-linux-gnu and i686-linux-gnu. > --- > stdlib/Makefile | 3 +- > stdlib/canonicalize.c | 29 +++-- > stdlib/tst-canon-bz26341.c | 108 ++++++++++++++++++ > support/support_set_small_thread_stack_size.c | 12 +- > support/xthread.h | 2 + > 5 files changed, 134 insertions(+), 20 deletions(-) > create mode 100644 stdlib/tst-canon-bz26341.c > > diff --git a/stdlib/Makefile b/stdlib/Makefile > index 4615f6dfe7..7093b8a584 100644 > --- a/stdlib/Makefile > +++ b/stdlib/Makefile > @@ -87,7 +87,7 @@ tests := tst-strtol tst-strtod testmb testrand testsort testdiv \ > tst-makecontext-align test-bz22786 tst-strtod-nan-sign \ > tst-swapcontext1 tst-setcontext4 tst-setcontext5 \ > tst-setcontext6 tst-setcontext7 tst-setcontext8 \ > - tst-setcontext9 tst-bz20544 > + tst-setcontext9 tst-bz20544 tst-canon-bz26341 > > tests-internal := tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \ > tst-tls-atexit tst-tls-atexit-nodelete > @@ -102,6 +102,7 @@ LDLIBS-test-atexit-race = $(shared-thread-library) > LDLIBS-test-at_quick_exit-race = $(shared-thread-library) > LDLIBS-test-cxa_atexit-race = $(shared-thread-library) > LDLIBS-test-on_exit-race = $(shared-thread-library) > +LDLIBS-tst-canon-bz26341 = $(shared-thread-library) > > LDLIBS-test-dlclose-exit-race = $(shared-thread-library) $(libdl) > LDFLAGS-test-dlclose-exit-race = $(LDFLAGS-rdynamic) > diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c > index c58439b3fd..6798ed8963 100644 > --- a/stdlib/canonicalize.c > +++ b/stdlib/canonicalize.c > @@ -29,6 +29,14 @@ > #include > #include > > +#ifndef PATH_MAX > +# ifdef MAXPATHLEN > +# define PATH_MAX MAXPATHLEN > +# else > +# define PATH_MAX 1024 > +# endif > +#endif > + > /* Return the canonical absolute name of file NAME. A canonical name > does not contain any ".", ".." components nor any repeated path > separators ('/') or symlinks. All path components must exist. If > @@ -43,10 +51,11 @@ > char * > __realpath (const char *name, char *resolved) > { > - char *rpath, *dest, *extra_buf = NULL; > + char *rpath, *dest; > const char *start, *end, *rpath_limit; > - long int path_max; > + const size_t path_max = PATH_MAX; > int num_links = 0; > + char extra_buf[PATH_MAX]; > > if (name == NULL) > { > @@ -66,14 +75,6 @@ __realpath (const char *name, char *resolved) > return NULL; > } > > -#ifdef PATH_MAX > - path_max = PATH_MAX; > -#else > - path_max = __pathconf (name, _PC_PATH_MAX); > - if (path_max <= 0) > - path_max = 8192; > -#endif > - > if (resolved == NULL) > { > rpath = malloc (path_max); > @@ -170,24 +171,20 @@ __realpath (const char *name, char *resolved) > > if (S_ISLNK (st.st_mode)) > { > - char *buf = __alloca (path_max); > + char buf[PATH_MAX]; > size_t len; > ssize_t n; > > if (++num_links > __eloop_threshold ()) > { > __set_errno (ELOOP); > - goto error; > - } > + goto error; } > > n = __readlink (rpath, buf, path_max - 1); > if (n < 0) > goto error; > buf[n] = '\0'; > > - if (!extra_buf) > - extra_buf = __alloca (path_max); > - > len = strlen (end); > /* Check that n + len + 1 doesn't overflow and is <= path_max. */ > if (n >= SIZE_MAX - len || n + len >= path_max) > diff --git a/stdlib/tst-canon-bz26341.c b/stdlib/tst-canon-bz26341.c > new file mode 100644 > index 0000000000..63474bddaa > --- /dev/null > +++ b/stdlib/tst-canon-bz26341.c > @@ -0,0 +1,108 @@ > +/* Check if realpath does not consume extra stack space based on symlink > + existance in the path (BZ #26341) > + Copyright (C) 2020 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 > + > +#include > +#include > +#include > +#include > +#include > + > +static char *filename; > +static size_t filenamelen; > +static char *linkname; > + > +static int > +maxsymlinks (void) > +{ > +#ifdef MAXSYMLINKS > + return MAXSYMLINKS; > +#else > + long int sysconf_symloop_max = sysconf (_SC_SYMLOOP_MAX); > + return sysconf_symloop_max <= 0 > + ? _POSIX_SYMLOOP_MAX > + : sysconf_symloop_max; > +#endif > +} > + > +#ifndef PATH_MAX > +# define PATH_MAX 1024 > +#endif > + > +static void > +create_link (void) > +{ > + int fd = create_temp_file ("tst-canon-bz26341", &filename); > + TEST_VERIFY_EXIT (fd != -1); > + xclose (fd); > + > + char *prevlink = filename; > + int maxlinks = maxsymlinks (); > + for (int i = 0; i < maxlinks; i++) > + { > + linkname = xasprintf ("%s%d", filename, i); > + xsymlink (prevlink, linkname); > + add_temp_file (linkname); > + prevlink = linkname; > + } > + > + filenamelen = strlen (filename); > +} > + > +static void * > +do_realpath (void *arg) > +{ > + /* Old implementation of realpath allocates a PATH_MAX using alloca > + for each symlink in the path, leading to MAXSYMLINKS times PATH_MAX > + maximum stack usage. > + This stack allocations tries fill the thread allocated stack minus > + both the thread (plus some slack) and the realpath (plus some slack). > + If realpath uses more than 2 * PATH_MAX plus some slack it will trigger > + a stackoverflow. */ > + > + const size_t realpath_usage = 2 * PATH_MAX + 1024; > + const size_t thread_usage = 1 * PATH_MAX + 1024; > + size_t stack_size = support_small_thread_stack_size () > + - realpath_usage - thread_usage; > + char stack[stack_size]; > + char *resolved = stack + stack_size - thread_usage + 1024; > + > + char *p = realpath (linkname, resolved); > + TEST_VERIFY (p != NULL); > + TEST_COMPARE_BLOB (resolved, filenamelen, filename, filenamelen); > + > + return NULL; > +} > + > +static int > +do_test (void) > +{ > + create_link (); > + > + pthread_t th = xpthread_create (support_small_stack_thread_attribute (), > + do_realpath, NULL); > + xpthread_join (th); > + > + return 0; > +} > + > +#include > diff --git a/support/support_set_small_thread_stack_size.c b/support/support_set_small_thread_stack_size.c > index 69d66e97db..74a0e38a72 100644 > --- a/support/support_set_small_thread_stack_size.c > +++ b/support/support_set_small_thread_stack_size.c > @@ -20,8 +20,8 @@ > #include > #include > > -void > -support_set_small_thread_stack_size (pthread_attr_t *attr) > +size_t > +support_small_thread_stack_size (void) > { > /* Some architectures have too small values for PTHREAD_STACK_MIN > which cannot be used for creating threads. Ensure that the stack > @@ -31,5 +31,11 @@ support_set_small_thread_stack_size (pthread_attr_t *attr) > if (stack_size < PTHREAD_STACK_MIN) > stack_size = PTHREAD_STACK_MIN; > #endif > - xpthread_attr_setstacksize (attr, stack_size); > + return stack_size; > +} > + > +void > +support_set_small_thread_stack_size (pthread_attr_t *attr) > +{ > + xpthread_attr_setstacksize (attr, support_small_thread_stack_size ()); > } > diff --git a/support/xthread.h b/support/xthread.h > index 05f8d4a7d9..6ba2f5a18b 100644 > --- a/support/xthread.h > +++ b/support/xthread.h > @@ -78,6 +78,8 @@ void xpthread_attr_setguardsize (pthread_attr_t *attr, > /* Set the stack size in ATTR to a small value, but still large enough > to cover most internal glibc stack usage. */ > void support_set_small_thread_stack_size (pthread_attr_t *attr); > +/* Return the stack size used on support_set_small_thread_stack_size. */ > +size_t support_small_thread_stack_size (void); > > /* Return a pointer to a thread attribute which requests a small > stack. The caller must not free this pointer. */ >