Message ID | 1502463044-4042-7-git-send-email-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | None | expand |
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 <eggert@cs.ucla.edu> 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 <eggert@cs.ucla.edu> + + 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 <bruno@clisp.org> 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 <flexmember.h> #include <glob_internal.h> #include <scratch_buffer.h> - -#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
diff --git a/posix/glob.c b/posix/glob.c index 99e9c54..3a74758 100644 --- a/posix/glob.c +++ b/posix/glob.c @@ -91,10 +91,8 @@ #include <glob_internal.h> #include <scratch_buffer.h> -#ifdef _SC_LOGIN_NAME_MAX -# define GET_LOGIN_NAME_MAX() sysconf (_SC_LOGIN_NAME_MAX) -#else -# define GET_LOGIN_NAME_MAX() (-1) +#ifndef LOGIN_NAME_MAX +# define LOGIN_NAME_MAX 256 #endif static const char *next_brace_sub (const char *begin, int flags) __THROWNL; @@ -667,28 +665,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), if (home_dir == NULL || home_dir[0] == '\0') { 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 - { - name = malloc (buflen); - if (name == NULL) - { - retval = GLOB_NOSPACE; - goto out; - } - malloc_name = 1; - } + char user_name[LOGIN_NAME_MAX]; - success = __getlogin_r (name, buflen) == 0; + success = __getlogin_r (user_name, sizeof (user_name)) == 0; if (success) { struct passwd *p; @@ -698,7 +677,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), struct scratch_buffer pwtmpbuf; scratch_buffer_init (&pwtmpbuf); - while (getpwnam_r (name, &pwbuf, + while (getpwnam_r (user_name, &pwbuf, pwtmpbuf.data, pwtmpbuf.length, &p) != 0) { @@ -730,11 +709,6 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), } scratch_buffer_free (&pwtmpbuf); } - else - { - if (__glibc_unlikely (malloc_name)) - free (name); - } } if (home_dir == NULL || home_dir[0] == '\0') {