On Tue, Mar 3, 2009 at 9:08 AM, Jim Meyering <j...@meyering.net> wrote: > > Thanks for the report. I read Casper's reply. > > That function removes PRIV_SYS_LINKDIR for the sake of security. > Without it, there's a guaranteed race condition that may lead to > unlinking a non-empty directory. > > It's ironic that Solaris' privilege system does not accommodate that. > But if you explain to them why we're doing it, maybe they'll > recognize the utility of it and relax the model. >
Hi Jim, I've confirmed that this is also a problem on Solaris 10, so even if Sun were to agree to change it there would still be many systems affected. How does the attached patch look? The idea is to restore the PRIV_SYS_LINKDIR privilege if chmod fails and try the chmod again. If it looks ok, I'll split the patch into separate gnulib and tar patches and add a changelog entry. -- David
diff -uraN tar-1.21.orig/lib/unlinkdir.c tar-1.21/lib/unlinkdir.c --- tar-1.21.orig/lib/unlinkdir.c 2007-11-18 05:46:57.000000000 -0500 +++ tar-1.21/lib/unlinkdir.c 2009-03-04 04:25:16.277473318 -0500 @@ -1,6 +1,6 @@ /* unlinkdir.c - determine (and maybe change) whether we can unlink directories - Copyright (C) 2005, 2006 Free Software Foundation, Inc. + Copyright (C) 2005, 2006, 2009 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -15,7 +15,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ -/* Written by Paul Eggert and Jim Meyering. */ +/* Written by Paul Eggert, Jim Meyering, and David Bartley. */ #include <config.h> @@ -28,6 +28,11 @@ #if ! UNLINK_CANNOT_UNLINK_DIR +static bool initialized; +# if defined PRIV_EFFECTIVE && defined PRIV_SYS_LINKDIR +static bool had_sys_linkdir; +#endif + /* Return true if we cannot unlink directories, false if we might be able to unlink directories. If possible, tell the kernel we don't want to be able to unlink directories, so that we can return true. */ @@ -35,7 +40,6 @@ bool cannot_unlink_dir (void) { - static bool initialized; static bool cannot; if (! initialized) @@ -47,11 +51,15 @@ priv_set_t *pset = priv_allocset (); if (pset) { - cannot = - (getppriv (PRIV_EFFECTIVE, pset) == 0 - && (! priv_ismember (pset, PRIV_SYS_LINKDIR) - || (priv_delset (pset, PRIV_SYS_LINKDIR) == 0 - && setppriv (PRIV_SET, PRIV_EFFECTIVE, pset) == 0))); + if (getppriv (PRIV_EFFECTIVE, pset) == 0) + { + if (priv_ismember (pset, PRIV_SYS_LINKDIR)) + cannot = had_sys_linkdir = + (priv_delset (pset, PRIV_SYS_LINKDIR) == 0 + && setppriv (PRIV_SET, PRIV_EFFECTIVE, pset) == 0); + else + cannot = true; + } priv_freeset (pset); } # else @@ -64,4 +72,38 @@ return cannot; } + +/* Return true if the kernel restored the ability of unlink to remove + directories. */ + +bool +restore_unlink_dir (void) +{ + bool restored = false; + +# if defined PRIV_EFFECTIVE && defined PRIV_SYS_LINKDIR + if (initialized) + { + /* Try to re-add the PRIV_SYS_LINKDIR privilege, but only if we + previously removed it. */ + if (had_sys_linkdir) + { + priv_set_t *pset = priv_allocset (); + if (pset) + { + if (getppriv (PRIV_EFFECTIVE, pset) == 0) + { + initialized = ! (priv_addset (pset, PRIV_SYS_LINKDIR) == 0 + && setppriv (PRIV_SET, PRIV_EFFECTIVE, pset) == 0); + } + } + priv_freeset (pset); + restored = ! initialized; + } + } +# endif + + return restored; +} + #endif diff -uraN tar-1.21.orig/lib/unlinkdir.h tar-1.21/lib/unlinkdir.h --- tar-1.21.orig/lib/unlinkdir.h 2007-11-18 05:46:57.000000000 -0500 +++ tar-1.21/lib/unlinkdir.h 2009-03-04 04:25:16.277618403 -0500 @@ -1,6 +1,6 @@ /* unlinkdir.h - determine (and maybe change) whether we can unlink directories - Copyright (C) 2005 Free Software Foundation, Inc. + Copyright (C) 2005, 2009 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -15,12 +15,14 @@ You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ -/* Written by Paul Eggert and Jim Meyering. */ +/* Written by Paul Eggert, Jim Meyering, and David Bartley. */ #include <stdbool.h> #if UNLINK_CANNOT_UNLINK_DIR # define cannot_unlink_dir() true +# define restore_unlink_dir() false #else bool cannot_unlink_dir (void); +bool restore_unlink_dir (void); #endif diff -uraN tar-1.21.orig/src/extract.c tar-1.21/src/extract.c --- tar-1.21.orig/src/extract.c 2008-10-30 10:10:28.000000000 -0400 +++ tar-1.21/src/extract.c 2009-03-04 04:25:26.119402470 -0500 @@ -1,7 +1,7 @@ /* Extract files from a tar archive. Copyright (C) 1988, 1992, 1993, 1994, 1996, 1997, 1998, 1999, 2000, - 2001, 2003, 2004, 2005, 2006, 2007 Free Software Foundation, Inc. + 2001, 2003, 2004, 2005, 2006, 2007, 2009 Free Software Foundation, Inc. Written by John Gilmore, on 1985-11-19. @@ -24,6 +24,7 @@ #include <utimens.h> #include <errno.h> #include <xgetcwd.h> +#include <unlinkdir.h> #include "common.h" @@ -186,7 +187,15 @@ mode = cur_info->st_mode ^ invert_permissions; } - if (chmod (file_name, mode) != 0) + bool failed = (chmod (file_name, mode) != 0); + if (failed && errno == EPERM) + { + /* On Solaris, chmod may fail if we dropped a privilege (this happens + when we call cannot_unlink_dir). */ + if (restore_unlink_dir ()) + failed = (chmod (file_name, mode) != 0); + } + if (failed) chmod_error_details (file_name, mode); }