Date: Tue, 25 Nov 2008 19:33:37 -0800 From: Xin LI <delphij@delphij.net> To: insomniac <insomniac@slackware.it> Cc: freebsd-current@freebsd.org, kensmith@freebsd.org Subject: Re: Patch for bin/54446 Message-ID: <492CC391.2070207@delphij.net> In-Reply-To: <20081126032214.03d8517a@slackware.it> References: <20081126032214.03d8517a@slackware.it>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------000809080902040307050305 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, insomniac wrote: > Hi to everyone, > I wrote a patch for the bin/54446 PR, fixing pkg_delete(1) that doesn't > honour symlinks, and portupgrades leads to failing services. > > Actually, this patch fixes that for all the utilities as it acts > directly in the lib. > > I tested the patch on a few x86 machines, ranging from 7.0 to -HEAD. > Testing and further reviewing are welcome and encouraged. > > pkg_delete now seems to work fine; moreover I found other bugs, like > memory leaks, missing checks of function return values, and wrong return > values. > > The patch has already been reviewed by attilio@ , it applies to > src/usr.sbin/pkg_install/lib/plist.c and is located at > > http://insomniac.slackware.it/plist.c.diff I have made a small change: use malloc() here and use strlcpy(). Other parts looks just fine. (BTW I think we need to cc portmgr@ for approval) Cheers, - -- Xin LI <delphij@delphij.net> http://www.delphij.net/ FreeBSD - The Power to Serve! -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (FreeBSD) iEYEARECAAYFAkksw5EACgkQi+vbBBjt66CVaACfRah4OMrOiFZKzJ3DvzjTnl3K sE8AnRQeL3lKC/fSnzJn89IQHMAgoudI =loiW -----END PGP SIGNATURE----- --------------000809080902040307050305 Content-Type: text/x-patch; name="plist.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="plist.diff" Index: usr.sbin/pkg_install/lib/plist.c =================================================================== --- usr.sbin/pkg_install/lib/plist.c (revision 185325) +++ usr.sbin/pkg_install/lib/plist.c (working copy) @@ -544,45 +544,90 @@ delete_package(Boolean ign_err, Boolean nukedirs, int delete_hierarchy(const char *dir, Boolean ign_err, Boolean nukedirs) { - char *cp1, *cp2; + char *cp1, *cp2, *realdir; - cp1 = cp2 = strdup(dir); - if (!fexists(dir)) { + realdir = malloc(FILENAME_MAX); + if (realdir == NULL) { + warnx("Couldn't allocate enough memory\n"); + return (ign_err ? SUCCESS : FAIL); + } + + if (issymlink(dir) && readlink(dir, realdir, FILENAME_MAX-1) == -1) + return (ign_err ? SUCCESS : FAIL); + + strlcpy(realdir, dir, FILENAME_MAX); + + cp1 = cp2 = strdup(realdir); + if (cp1 == NULL) { + warnx("Couldn't allocate enough memory\n"); + return (ign_err ? SUCCESS : FAIL); + } + + if (!fexists(realdir)) { if (!ign_err) warnx("%s '%s' doesn't exist", - isdir(dir) ? "directory" : "file", dir); - return !ign_err; + isdir(realdir) ? "directory" : "file", realdir); + free(cp1); + free(realdir); + return (ign_err ? SUCCESS : FAIL); } else if (nukedirs) { - if (vsystem("%s -r%s %s", REMOVE_CMD, (ign_err ? "f" : ""), dir)) - return 1; + if (vsystem("%s -r%s %s", REMOVE_CMD, (ign_err ? "f" : ""), realdir)) { + free(cp1); + free(realdir); + return (ign_err ? SUCCESS : FAIL); + } } - else if (isdir(dir) && !issymlink(dir)) { - if (RMDIR(dir) && !ign_err) - return 1; + else if (isdir(realdir)) { + if (RMDIR(realdir)) { + free(cp1); + free(realdir); + return (ign_err ? SUCCESS : FAIL); + } } else { - if (REMOVE(dir, ign_err)) - return 1; + if (REMOVE(realdir, ign_err)) { + free(cp1); + free(realdir); + return (ign_err ? SUCCESS : FAIL); + } } - if (!nukedirs) - return 0; + if (!nukedirs) { + free(cp1); + free(realdir); + return (SUCCESS); + } 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; + if (!isemptydir(realdir)) { + free(cp1); + free(realdir); + return (SUCCESS); } + if (RMDIR(realdir) && !ign_err) { + if (!fexists(realdir)) { + warnx("directory '%s' doesn't exist", realdir); + return (SUCCESS); + } else { + free(cp1); + free(realdir); + return (FAIL); + } + } /* back up the pathname one component */ if (cp2) { - cp1 = strdup(dir); + free(cp1); + cp1 = strdup(realdir); + if (cp1 == NULL) { + warnx("Couldn't allocate enough memory\n"); + return (ign_err ? SUCCESS : FAIL); + } } } - return 0; + free(cp1); + free(realdir); + return (SUCCESS); } + --------------000809080902040307050305--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?492CC391.2070207>