Make unlink on directory as root set errno.
authorEvan Zacks <zackse@cpan.org>
Tue, 26 Nov 2013 14:56:11 +0000 (09:56 -0500)
committerCraig A. Berry <craigberry@mac.com>
Tue, 3 Dec 2013 04:45:36 +0000 (22:45 -0600)
If unlink is called on an existing directory while running as root without -U
(PL_unsafe), the unlink call fails but does not set $!, because unlink(2) is
not actually called in this case.

If unlink is called as a user (or as root with -U), unlink(2) is invoked, so
attempting to remove a directory would set errno to EISDIR as expected. If
running as root without -U (PL_unsafe is false), lstat and S_ISDIR are called
instead. If the lstat succeeds and S_ISDIR returns true, the argument is
skipped without warning and without setting $!, meaning Perl's unlink can
return failure while leaving the previous value of errno in place.

This commit sets errno to EISDIR for this case.

AUTHORS
doio.c
t/io/fs.t

index 1e0a166..9bf9b04 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -373,6 +373,7 @@ Eryq                                <eryq@zeegee.com>
 Etienne Grossman               <etienne@isr.isr.ist.utl.pt>
 Eugene Alterman                        <Eugene.Alterman@bremer-inc.com>
 Evan Miller                    <eam@frap.net>
+Evan Zacks                     <zackse@cpan.org>
 Fabien Tassin                  <tassin@eerie.fr>
 Father Chrysostomos            <sprout@cpan.org>
 Felipe Gasper                  <felipe@felipegasper.com>
diff --git a/doio.c b/doio.c
index 4c929b1..56d33b2 100644 (file)
--- a/doio.c
+++ b/doio.c
@@ -1821,8 +1821,12 @@ nothing in the core.
                    tot--;
            }
            else {      /* don't let root wipe out directories without -U */
-               if (PerlLIO_lstat(s,&PL_statbuf) < 0 || S_ISDIR(PL_statbuf.st_mode))
+               if (PerlLIO_lstat(s,&PL_statbuf) < 0)
                    tot--;
+               else if (S_ISDIR(PL_statbuf.st_mode)) {
+                   tot--;
+                   SETERRNO(EISDIR, SS$_NOPRIV);
+               }
                else {
                    if (UNLINK(s))
                        tot--;
index 062a9c0..857b091 100644 (file)
--- a/t/io/fs.t
+++ b/t/io/fs.t
@@ -46,7 +46,7 @@ $needs_fh_reopen = 1 if (defined &Win32::IsWin95 && Win32::IsWin95());
 my $skip_mode_checks =
     $^O eq 'cygwin' && $ENV{CYGWIN} !~ /ntsec/;
 
-plan tests => 52;
+plan tests => 55;
 
 my $tmpdir = tempfile();
 my $tmpdir1 = tempfile();
@@ -457,5 +457,36 @@ ok(-d $tmpdir1, "rename on directories working");
     ok(1, "extend sp in pp_chown");
 }
 
+# Calling unlink on a directory as root without -U will always fail, but
+# it should set errno to EISDIR even though unlink(2) is never called.
+SKIP: {
+    skip "only test unlink(dir) when running as root", 3 if $> != 0;
+
+    require Errno;
+
+    my $tmpdir = tempfile();
+    if (($^O eq 'MSWin32') || ($^O eq 'NetWare')) {
+        `mkdir $tmpdir`;
+    }
+    elsif ($^O eq 'VMS') {
+        `create/directory [.$tmpdir]`;
+    }
+    else {
+        `mkdir $tmpdir 2>/dev/null`;
+    }
+
+    # errno should be set even though unlink(2) is not called
+    local $!;
+    is(unlink($tmpdir), 0, "can't unlink directory as root without -U");
+    is(0+$!, Errno::EISDIR(), "unlink directory as root without -U sets errno");
+
+    rmdir $tmpdir;
+
+    # errno should be set by failed lstat(2) call
+    $! = 0;
+    unlink($tmpdir);
+    is(0+$!, Errno::ENOENT(), "unlink non-existent directory as root without -U sets ENOENT");
+}
+
 # need to remove $tmpdir if rename() in test 28 failed!
 END { rmdir $tmpdir1; rmdir $tmpdir; }