From patchwork Fri Aug 23 15:46:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gary Benson X-Patchwork-Id: 34256 Received: (qmail 48100 invoked by alias); 23 Aug 2019 15:47:05 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 48067 invoked by uid 89); 23 Aug 2019 15:47:03 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LOTSOFHASH, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=stick, sk:update-, sk:update, five X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 23 Aug 2019 15:46:59 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 92F9C307CB3F for ; Fri, 23 Aug 2019 15:46:58 +0000 (UTC) Received: from blade.nx (ovpn-117-31.ams2.redhat.com [10.36.117.31]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0F9775DA8C for ; Fri, 23 Aug 2019 15:46:58 +0000 (UTC) Received: from blade.com (localhost [127.0.0.1]) by blade.nx (Postfix) with ESMTP id 38FC4816CC28 for ; Fri, 23 Aug 2019 16:46:57 +0100 (BST) From: Gary Benson To: gdb-patches@sourceware.org Subject: [PATCH] Fix Gnulib glob.c resource leaks found by Coverity Date: Fri, 23 Aug 2019 16:46:55 +0100 Message-Id: <1566575215-10022-1-git-send-email-gbenson@redhat.com> MIME-Version: 1.0 X-IsSubscribed: yes Hi all, Coverity discovered a number of resource leaks in Gnulib's glob.c. This commit backports the five Gnulib commits that fix the leaks. Regression tested on the buildbot. Ok to commit? Thanks, Gary --- gnulib/ChangeLog: * patches/0003-Fix-glob-c-Coverity-issues.patch: New file. * update-gnulib.sh: List the above. * import/glob.c: Rebuild. --- gnulib/ChangeLog | 6 + gnulib/import/glob.c | 84 +++++-- .../patches/0003-Fix-glob-c-Coverity-issues.patch | 279 +++++++++++++++++++++ gnulib/update-gnulib.sh | 1 + 4 files changed, 347 insertions(+), 23 deletions(-) create mode 100644 gnulib/patches/0003-Fix-glob-c-Coverity-issues.patch diff --git a/gnulib/import/glob.c b/gnulib/import/glob.c index 4b04b90..416d210 100644 --- a/gnulib/import/glob.c +++ b/gnulib/import/glob.c @@ -734,6 +734,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), pwtmpbuf = malloc (pwbuflen); if (pwtmpbuf == NULL) { + if (__glibc_unlikely (malloc_name)) + free (name); retval = GLOB_NOSPACE; goto out; } @@ -762,6 +764,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), if (newp == NULL) { free (malloc_pwtmpbuf); + if (__glibc_unlikely (malloc_name)) + free (name); retval = GLOB_NOSPACE; goto out; } @@ -797,23 +801,30 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), malloc_home_dir = 1; } memcpy (home_dir, p->pw_dir, home_dir_len); - - free (pwtmpbuf); } } + free (malloc_pwtmpbuf); + } + else + { + if (__glibc_unlikely (malloc_name)) + free (name); } } if (home_dir == NULL || home_dir[0] == '\0') { + if (__glibc_unlikely (malloc_home_dir)) + free (home_dir); if (flags & GLOB_TILDE_CHECK) { - if (__glibc_unlikely (malloc_home_dir)) - free (home_dir); retval = GLOB_NOMATCH; goto out; } else - home_dir = (char *) "~"; /* No luck. */ + { + home_dir = (char *) "~"; /* No luck. */ + malloc_home_dir = 0; + } } # endif /* WINDOWS32 */ # endif @@ -855,6 +866,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), dirname = newp; dirlen += home_len - 1; malloc_dirname = !use_alloca; + + if (__glibc_unlikely (malloc_home_dir)) + free (home_dir); } dirname_modified = 1; } @@ -1027,9 +1041,12 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), free (malloc_pwtmpbuf); if (flags & GLOB_TILDE_CHECK) - /* We have to regard it as an error if we cannot find the - home directory. */ - return GLOB_NOMATCH; + { + /* We have to regard it as an error if we cannot find the + home directory. */ + retval = GLOB_NOMATCH; + goto out; + } } } } @@ -1059,7 +1076,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), free (pglob->gl_pathv); pglob->gl_pathv = NULL; pglob->gl_pathc = 0; - return GLOB_NOSPACE; + retval = GLOB_NOSPACE; + goto out; } new_gl_pathv = realloc (pglob->gl_pathv, @@ -1077,12 +1095,19 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), p = mempcpy (pglob->gl_pathv[newcount], dirname, dirlen); p[0] = '/'; p[1] = '\0'; + if (__glibc_unlikely (malloc_dirname)) + free (dirname); } else { - pglob->gl_pathv[newcount] = strdup (dirname); - if (pglob->gl_pathv[newcount] == NULL) - goto nospace; + if (__glibc_unlikely (malloc_dirname)) + pglob->gl_pathv[newcount] = dirname; + else + { + pglob->gl_pathv[newcount] = strdup (dirname); + if (pglob->gl_pathv[newcount] == NULL) + goto nospace; + } } pglob->gl_pathv[++newcount] = NULL; ++pglob->gl_pathc; @@ -1092,7 +1117,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), } /* Not found. */ - return GLOB_NOMATCH; + retval = GLOB_NOMATCH; + goto out; } meta = __glob_pattern_type (dirname, !(flags & GLOB_NOESCAPE)); @@ -1138,7 +1164,10 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), if (status != 0) { if ((flags & GLOB_NOCHECK) == 0 || status != GLOB_NOMATCH) - return status; + { + retval = status; + goto out; + } goto no_matches; } @@ -1157,7 +1186,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), if (interrupt_state) { globfree (&dirs); - return GLOB_ABORTED; + retval = GLOB_ABORTED; + goto out; } } #endif /* SHELL. */ @@ -1176,7 +1206,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), globfree (&dirs); globfree (pglob); pglob->gl_pathc = 0; - return status; + retval = status; + goto out; } /* Stick the directory on the front of each name. */ @@ -1187,7 +1218,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), globfree (&dirs); globfree (pglob); pglob->gl_pathc = 0; - return GLOB_NOSPACE; + retval = GLOB_NOSPACE; + goto out; } } @@ -1209,7 +1241,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), { nospace2: globfree (&dirs); - return GLOB_NOSPACE; + retval = GLOB_NOSPACE; + goto out; } new_gl_pathv = realloc (pglob->gl_pathv, @@ -1224,7 +1257,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), globfree (&dirs); globfree (pglob); pglob->gl_pathc = 0; - return GLOB_NOSPACE; + retval = GLOB_NOSPACE; + goto out; } ++pglob->gl_pathc; @@ -1236,7 +1270,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), else { globfree (&dirs); - return GLOB_NOMATCH; + retval = GLOB_NOMATCH; + goto out; } } @@ -1282,7 +1317,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), flags = orig_flags; goto no_matches; } - return status; + retval = status; + goto out; } if (dirlen > 0) @@ -1294,7 +1330,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), { globfree (pglob); pglob->gl_pathc = 0; - return GLOB_NOSPACE; + retval = GLOB_NOSPACE; + goto out; } } } @@ -1319,7 +1356,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), { globfree (pglob); pglob->gl_pathc = 0; - return GLOB_NOSPACE; + retval = GLOB_NOSPACE; + goto out; } strcpy (&new[len - 2], "/"); pglob->gl_pathv[i] = new; diff --git a/gnulib/patches/0003-Fix-glob-c-Coverity-issues.patch b/gnulib/patches/0003-Fix-glob-c-Coverity-issues.patch new file mode 100644 index 0000000..f976bb9 --- /dev/null +++ b/gnulib/patches/0003-Fix-glob-c-Coverity-issues.patch @@ -0,0 +1,279 @@ +Coverity discovered a number of resource leaks in Gnulib. +The solution is to backport the following Gnulib commits: + + commit 0eee3ccaae5bb3d0016a0da8b8e5108767c02748 + Author: Bruno Haible + Date: Fri Mar 31 22:41:38 2017 +0200 + + glob: Fix memory leaks. + + * lib/glob.c (glob): Free allocated memory before returning. + Reported by Coverity via Tim Rühsen. + + commit b1d7f3165ba1c7a44a29017eb80491094aa240ba + Author: Bruno Haible + Date: Fri Mar 31 22:43:35 2017 +0200 + + glob: Fix invalid free() call. + + * lib/glob.c (glob): Reset malloc_home_dir when assigning a pointer to + static storage to home_dir. + Reported by Coverity via Tim Rühsen. + + commit 1540f3441555f756558f3a18e5f68914c0b72227 + Author: Bruno Haible + Date: Sat Apr 1 15:15:18 2017 +0200 + + glob: Fix more memory leaks. + + * lib/glob.c (glob): Free allocated memory before returning. + Reported by Coverity via Tim Rühsen. + + commit b19cb256c9a4d3a138c27181cffee5513edb0e81 + Author: Bruno Haible + Date: Thu Jul 6 23:21:49 2017 +0200 + + glob: Fix more memory leaks. + + * lib/glob.c (glob): Free dirname before returning. + Reported by Coverity and Tim Rühsen. + + commit 8cb994d1fc4a957359780e1a4187b4f250c1cea5 + Author: Tim Rühsen + Date: Mon Jul 10 19:02:19 2017 +0200 + + glob: Fix more memory leaks. + + * lib/glob.c (glob): Use 'goto out' in order to free dirname before + returning. + Reported by Tim Rühsen. + +diff --git a/gnulib/import/glob.c b/gnulib/import/glob.c +index 4b04b90..416d210 100644 +--- a/gdb/gnulib/import/glob.c ++++ b/gdb/gnulib/import/glob.c +@@ -734,6 +734,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + pwtmpbuf = malloc (pwbuflen); + if (pwtmpbuf == NULL) + { ++ if (__glibc_unlikely (malloc_name)) ++ free (name); + retval = GLOB_NOSPACE; + goto out; + } +@@ -762,6 +764,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + if (newp == NULL) + { + free (malloc_pwtmpbuf); ++ if (__glibc_unlikely (malloc_name)) ++ free (name); + retval = GLOB_NOSPACE; + goto out; + } +@@ -797,23 +801,30 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + malloc_home_dir = 1; + } + memcpy (home_dir, p->pw_dir, home_dir_len); +- +- free (pwtmpbuf); + } + } ++ free (malloc_pwtmpbuf); ++ } ++ else ++ { ++ if (__glibc_unlikely (malloc_name)) ++ free (name); + } + } + if (home_dir == NULL || home_dir[0] == '\0') + { ++ if (__glibc_unlikely (malloc_home_dir)) ++ free (home_dir); + if (flags & GLOB_TILDE_CHECK) + { +- if (__glibc_unlikely (malloc_home_dir)) +- free (home_dir); + retval = GLOB_NOMATCH; + goto out; + } + else +- home_dir = (char *) "~"; /* No luck. */ ++ { ++ home_dir = (char *) "~"; /* No luck. */ ++ malloc_home_dir = 0; ++ } + } + # endif /* WINDOWS32 */ + # endif +@@ -855,6 +866,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + dirname = newp; + dirlen += home_len - 1; + malloc_dirname = !use_alloca; ++ ++ if (__glibc_unlikely (malloc_home_dir)) ++ free (home_dir); + } + dirname_modified = 1; + } +@@ -1027,9 +1041,12 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + free (malloc_pwtmpbuf); + + if (flags & GLOB_TILDE_CHECK) +- /* We have to regard it as an error if we cannot find the +- home directory. */ +- return GLOB_NOMATCH; ++ { ++ /* We have to regard it as an error if we cannot find the ++ home directory. */ ++ retval = GLOB_NOMATCH; ++ goto out; ++ } + } + } + } +@@ -1059,7 +1076,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + free (pglob->gl_pathv); + pglob->gl_pathv = NULL; + pglob->gl_pathc = 0; +- return GLOB_NOSPACE; ++ retval = GLOB_NOSPACE; ++ goto out; + } + + new_gl_pathv = realloc (pglob->gl_pathv, +@@ -1077,12 +1095,19 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + p = mempcpy (pglob->gl_pathv[newcount], dirname, dirlen); + p[0] = '/'; + p[1] = '\0'; ++ if (__glibc_unlikely (malloc_dirname)) ++ free (dirname); + } + else + { +- pglob->gl_pathv[newcount] = strdup (dirname); +- if (pglob->gl_pathv[newcount] == NULL) +- goto nospace; ++ if (__glibc_unlikely (malloc_dirname)) ++ pglob->gl_pathv[newcount] = dirname; ++ else ++ { ++ pglob->gl_pathv[newcount] = strdup (dirname); ++ if (pglob->gl_pathv[newcount] == NULL) ++ goto nospace; ++ } + } + pglob->gl_pathv[++newcount] = NULL; + ++pglob->gl_pathc; +@@ -1092,7 +1117,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + } + + /* Not found. */ +- return GLOB_NOMATCH; ++ retval = GLOB_NOMATCH; ++ goto out; + } + + meta = __glob_pattern_type (dirname, !(flags & GLOB_NOESCAPE)); +@@ -1138,7 +1164,10 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + if (status != 0) + { + if ((flags & GLOB_NOCHECK) == 0 || status != GLOB_NOMATCH) +- return status; ++ { ++ retval = status; ++ goto out; ++ } + goto no_matches; + } + +@@ -1157,7 +1186,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + if (interrupt_state) + { + globfree (&dirs); +- return GLOB_ABORTED; ++ retval = GLOB_ABORTED; ++ goto out; + } + } + #endif /* SHELL. */ +@@ -1176,7 +1206,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + globfree (&dirs); + globfree (pglob); + pglob->gl_pathc = 0; +- return status; ++ retval = status; ++ goto out; + } + + /* Stick the directory on the front of each name. */ +@@ -1187,7 +1218,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + globfree (&dirs); + globfree (pglob); + pglob->gl_pathc = 0; +- return GLOB_NOSPACE; ++ retval = GLOB_NOSPACE; ++ goto out; + } + } + +@@ -1209,7 +1241,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + { + nospace2: + globfree (&dirs); +- return GLOB_NOSPACE; ++ retval = GLOB_NOSPACE; ++ goto out; + } + + new_gl_pathv = realloc (pglob->gl_pathv, +@@ -1224,7 +1257,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + globfree (&dirs); + globfree (pglob); + pglob->gl_pathc = 0; +- return GLOB_NOSPACE; ++ retval = GLOB_NOSPACE; ++ goto out; + } + + ++pglob->gl_pathc; +@@ -1236,7 +1270,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + else + { + globfree (&dirs); +- return GLOB_NOMATCH; ++ retval = GLOB_NOMATCH; ++ goto out; + } + } + +@@ -1282,7 +1317,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + flags = orig_flags; + goto no_matches; + } +- return status; ++ retval = status; ++ goto out; + } + + if (dirlen > 0) +@@ -1294,7 +1330,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + { + globfree (pglob); + pglob->gl_pathc = 0; +- return GLOB_NOSPACE; ++ retval = GLOB_NOSPACE; ++ goto out; + } + } + } +@@ -1319,7 +1356,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int), + { + globfree (pglob); + pglob->gl_pathc = 0; +- return GLOB_NOSPACE; ++ retval = GLOB_NOSPACE; ++ goto out; + } + strcpy (&new[len - 2], "/"); + pglob->gl_pathv[i] = new; diff --git a/gnulib/update-gnulib.sh b/gnulib/update-gnulib.sh index 0e1df22..0c8357c 100755 --- a/gnulib/update-gnulib.sh +++ b/gnulib/update-gnulib.sh @@ -171,6 +171,7 @@ apply_patches () apply_patches "patches/0001-Fix-PR-gdb-23558-Use-system-s-getcwd-when-cross-comp.patch" apply_patches "patches/0002-mkostemp-mkostemps-Fix-compilation-error-in-C-mode-o.patch" +apply_patches "patches/0003-Fix-glob-c-Coverity-issues.patch" # Regenerate all necessary files... aclocal -Iimport/m4 -I../config &&