Make unlink on directory as root set errno.

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.

3 files changed, 38 insertions, 2 deletions

diff --git a/AUTHORS b/AUTHORS
index 1e0a166ca3..9bf9b04fed 100644
--- 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 4c929b1c31..56d33b2c62 100644
--- 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--;
diff --git a/t/io/fs.t b/t/io/fs.t
index 062a9c0c8b..857b091eb4 100644
--- 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; }