From patchwork Sat Sep 2 22:50:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Eggert X-Patchwork-Id: 22578 Received: (qmail 117987 invoked by alias); 2 Sep 2017 22:50:59 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 117967 invoked by uid 89); 2 Sep 2017 22:50:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=insist, Obviously X-HELO: zimbra.cs.ucla.edu From: Paul Eggert Subject: Re: [PATCH 06/18] posix: Remove glob GET_LOGIN_NAME_MAX usage To: Adhemerval Zanella Cc: libc-alpha@sourceware.org, Gnulib bugs References: <1502463044-4042-1-git-send-email-adhemerval.zanella@linaro.org> <1502463044-4042-7-git-send-email-adhemerval.zanella@linaro.org> Message-ID: <2151b336-5d5c-e35e-3309-8e227dae0966@cs.ucla.edu> Date: Sat, 2 Sep 2017 15:50:36 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <1502463044-4042-7-git-send-email-adhemerval.zanella@linaro.org> On 08/11/2017 07:50 AM, Adhemerval Zanella wrote: > There is no actual login to resize the buffer in case of the resizing > the buffer in case of ERANGE, so a static buffer using glibc default > LOGIN_NAME_MAX is suffice. Although I had trouble parsing that, I think you're saying that because the current glob.c goes awry when sysconf (_SC_LOGIN_NAME_MAX) < 0, it's OK if we change glob.c to insist on a fixed-size limit of 255 bytes on user name length so that glob continues to mishandle (presumably mostly-theoretical) environments with longer user names. But that's not the GNU style, which is to avoid arbitrary limits. Instead, let's fix glob.c so that it doesn't need to know the user name length limit. Obviously glob should use heap allocation for anything large, which suggests that it should use a scratch buffer for the login name. I looked into this, and it's easy enough to change glob.c to use the tail of the scratch buffer that it's already using for getpwnam_r (given your previously-proposed patches), and this simplifies glob's memory-allocation code. I installed the attached patch into Gnulib to do that. Please take a look at it for your next go-round with glibc. Thanks. This is mostly-theoretical stuff, of course, as this code is exercised only when $HOME is unset or empty. > + char user_name[LOGIN_NAME_MAX]; A nit: that array needs to be one byte bigger, for the trailing NULL. This point is irrelevant to the attached Gnulib patch, which doesn't use LOGIN_NAME_MAX. From 47688d5de93a7baf7b203a7b687d3f4809667dcd Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 2 Sep 2017 15:39:16 -0700 Subject: [PATCH] glob: fix bugs with long login names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem reported by Adhemerval Zanella in: https://sourceware.org/ml/libc-alpha/2017-08/msg00455.html * lib/glob.c (GET_LOGIN_NAME_MAX): Remove. (glob): Use the same scratch buffer for both getlogin_r and getpwnam_r. Don’t require preallocation of the login name. This simplifies storage allocation, and corrects the handling of long login names. --- ChangeLog | 11 ++++++++ lib/glob.c | 88 +++++++++++++++++++++----------------------------------------- 2 files changed, 41 insertions(+), 58 deletions(-) diff --git a/ChangeLog b/ChangeLog index b67d21799..351495b2f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2017-09-02 Paul Eggert + + glob: fix bugs with long login names + Problem reported by Adhemerval Zanella in: + https://sourceware.org/ml/libc-alpha/2017-08/msg00455.html + * lib/glob.c (GET_LOGIN_NAME_MAX): Remove. + (glob): Use the same scratch buffer for both getlogin_r and + getpwnam_r. Don’t require preallocation of the login name. This + simplifies storage allocation, and corrects the handling of + long login names. + 2017-09-02 Bruno Haible dirent: Update doc. diff --git a/lib/glob.c b/lib/glob.c index 7ca11361e..8eb2b9730 100644 --- a/lib/glob.c +++ b/lib/glob.c @@ -75,12 +75,6 @@ #include #include #include - -#ifdef _SC_LOGIN_NAME_MAX -# define GET_LOGIN_NAME_MAX() sysconf (_SC_LOGIN_NAME_MAX) -#else -# define GET_LOGIN_NAME_MAX() (-1) -#endif static const char *next_brace_sub (const char *begin, int flags) __THROWNL; @@ -611,67 +605,45 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), else home_dir = "c:/users/default"; /* poor default */ #else - int success; - char *name; - int malloc_name = 0; - size_t buflen = GET_LOGIN_NAME_MAX () + 1; - - if (buflen == 0) - /* 'sysconf' does not support _SC_LOGIN_NAME_MAX. Try - a moderate value. */ - buflen = 20; - if (glob_use_alloca (alloca_used, buflen)) - name = alloca_account (buflen, alloca_used); - else + int err; + struct passwd *p; + struct passwd pwbuf; + struct scratch_buffer s; + scratch_buffer_init (&s); + while (true) { - name = malloc (buflen); - if (name == NULL) + p = NULL; + err = __getlogin_r (s.data, s.length); + if (err == 0) { - retval = GLOB_NOSPACE; - goto out; - } - malloc_name = 1; - } - - success = __getlogin_r (name, buflen) == 0; - if (success) - { - struct passwd *p; - struct scratch_buffer pwtmpbuf; - scratch_buffer_init (&pwtmpbuf); # if defined HAVE_GETPWNAM_R || defined _LIBC - struct passwd pwbuf; - - while (getpwnam_r (name, &pwbuf, - pwtmpbuf.data, pwtmpbuf.length, &p) - == ERANGE) - { - if (!scratch_buffer_grow (&pwtmpbuf)) - { - retval = GLOB_NOSPACE; - goto out; - } - } + size_t ssize = strlen (s.data) + 1; + err = getpwnam_r (s.data, &pwbuf, s.data + ssize, + s.length - ssize, &p); # else - p = getpwnam (name); + p = getpwnam (s.data); + if (p == NULL) + err = errno; # endif - if (p != NULL) + } + if (err != ERANGE) + break; + if (!scratch_buffer_grow (&s)) { - home_dir = strdup (p->pw_dir); - malloc_home_dir = 1; - if (home_dir == NULL) - { - scratch_buffer_free (&pwtmpbuf); - retval = GLOB_NOSPACE; - goto out; - } + retval = GLOB_NOSPACE; + goto out; } - scratch_buffer_free (&pwtmpbuf); } - else + if (err == 0) + { + home_dir = strdup (p->pw_dir); + malloc_home_dir = 1; + } + scratch_buffer_free (&s); + if (err == 0 && home_dir == NULL) { - if (__glibc_unlikely (malloc_name)) - free (name); + retval = GLOB_NOSPACE; + goto out; } #endif /* WINDOWS32 */ } -- 2.13.5