Date: Thu, 24 May 2012 01:48:06 GMT From: Garrett Cooper <yanegomi@gmail.com> To: freebsd-gnats-submit@FreeBSD.org Subject: misc/168289: [patch] [pkg_install] fix memory leaks and potential CPU burning in lib/plist.c:delete_hierarchy Message-ID: <201205240148.q4O1m6c8047094@red.freebsd.org> Resent-Message-ID: <201205240150.q4O1o2ib082161@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 168289
>Category: misc
>Synopsis: [patch] [pkg_install] fix memory leaks and potential CPU burning in lib/plist.c:delete_hierarchy
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu May 24 01:50:01 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator: Garrett Cooper
>Release: 9-STABLE
>Organization:
EMC Isilon
>Environment:
FreeBSD forza.west.isilon.com 9.0-STABLE FreeBSD 9.0-STABLE #4 r235133: Mon May 7 10:31:22 PDT 2012 root@forza.isilon.com:/usr/obj/usr/src/sys/FORZA amd64
>Description:
A case was noted internally where input in delete_hierarchy could leak memory and burn CPU cycles when trying to delete a directory hierarchy with two or more contiguous slashes next to them. Example of the CPU burning is as follows:
$ cat ~/infinite_plist.c
#include <libgen.h>
#include <stdio.h>
#include <string.h>
int
main(void)
{
char *paths[] = {
"foo//bar",
"/root/foo.bar",
"///root/foo.bar/baz",
"a.b.c.d",
};
char *cp1, *cp2;
int i;
#ifdef STUCK
for (i = 0; i < sizeof(paths)/sizeof(paths[0]); i++) {
cp1 = cp2 = strdup(paths[i]);
printf("%s... ", cp2);
fflush(stdout);
while (cp2) {
if ((cp2 = strrchr(cp1, '/')) != NULL)
*cp2 = '\0';
if (cp2)
cp1 = strdup(paths[0]);
}
}
#else
for (i = 0; i < sizeof(paths)/sizeof(paths[0]); i++) {
cp1 = paths[i];
for (;;) {
if ((cp2 = dirname(cp2)) == NULL || strcmp("/", cp2) == 0 || strcmp(".", cp2) == 0)
break;
}
printf("%s: %s\n", paths[i], cp2);
}
#endif
return (0);
}
$ gcc -DSTUCK -o ~/infinite_plist ~/infinite_plist.c
$ ~/infinite_plist
foo//bar... ^C
$ gcc -o ~/infinite_plist ~/infinite_plist.c
$ ~/infinite_plist
foo//bar: .
/root/foo.bar: .
///root/foo.bar/baz: .
a.b.c.d: .
$
Reported by Miles Ohlrich <miles.ohlrich!AT-SPAMFREE!isilon.com> (replace !AT-SPAMFREE! with '@').
>How-To-Repeat:
Call delete_hierarchy with dir == "/usr/local//foo-1.0", or trigger any of the error conditions with the file hierarchy tests.
>Fix:
Patch attached with submission follows:
Index: usr.sbin/pkg_install/lib/plist.c
===================================================================
--- usr.sbin/pkg_install/lib/plist.c (revision 235484)
+++ usr.sbin/pkg_install/lib/plist.c (working copy)
@@ -23,6 +23,7 @@
#include "lib.h"
#include <err.h>
+#include <libgen.h>
#include <md5.h>
/* Add an item to a packing list */
@@ -548,9 +549,8 @@
int
delete_hierarchy(const char *dir, Boolean ign_err, Boolean nukedirs)
{
- char *cp1, *cp2;
+ char *cp;
- cp1 = cp2 = strdup(dir);
if (!fexists(dir) && !issymlink(dir)) {
if (!ign_err)
warnx("%s '%s' doesn't exist",
@@ -572,21 +572,21 @@
if (!nukedirs)
return 0;
- while (cp2) {
- if ((cp2 = strrchr(cp1, '/')) != NULL)
- *cp2 = '\0';
- if (!isemptydir(dir))
- return 0;
- if (RMDIR(dir) && !ign_err) {
- if (!fexists(dir))
- warnx("directory '%s' doesn't exist", dir);
- else
- return 1;
+
+ cp = (char*)dir;
+ for (;;) {
+ if ((cp = dirname(cp)) == NULL)
+ return 1;
+ if (strcmp("/", cp) == 0 || strcmp(".", cp) == 0)
+ break;
+ if (!isemptydir(cp))
+ break;
+ if (RMDIR(cp) && !ign_err) {
+ if (!fexists(cp))
+ warnx("directory '%s' doesn't exist", cp);
+ return 1;
}
- /* back up the pathname one component */
- if (cp2) {
- cp1 = strdup(dir);
- }
}
+
return 0;
}
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201205240148.q4O1m6c8047094>