Libguestfs - Jan 2011 - [PATCH] Replace File::Path's remove_tree

We were using remove_tree in the RHEV cleanup code. remove_tree seems to use
chdir. Unfortunately, as this code will be running seteuid(36), it is not
unlikely that the current working directory will not be readable. This causes
remove_tree to fail.

This patch replaces remove_tree with a simple recursive remove, obviously
without using chdir.

Fixes RHBZ#670778
---
 lib/Sys/VirtV2V/Connection/RHEVTarget.pm |   56 +++++++++++++++++++-----------
 1 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/lib/Sys/VirtV2V/Connection/RHEVTarget.pm
b/lib/Sys/VirtV2V/Connection/RHEVTarget.pm
index 7ea84f8..b02e9ff 100644
--- a/lib/Sys/VirtV2V/Connection/RHEVTarget.pm
+++ b/lib/Sys/VirtV2V/Connection/RHEVTarget.pm
@@ -243,7 +243,6 @@ sub DESTROY
 
 package Sys::VirtV2V::Connection::RHEVTarget::Vol;
 
-use File::Path qw(remove_tree);
 use File::Spec::Functions;
 use File::Temp qw(tempdir);
 use POSIX;
@@ -373,6 +372,41 @@ sub _move_vols
     $class->_cleanup();
 }
 
+# We used to use remove_tree from File::Path here. Unfortunately it does
+# something unexpected involving chdir which means it will fail bizarrely if
the
+# current directory (not the target directory) is not readable by the current
+# user.
+sub _remove_tree
+{
+    my $dir = shift;
+
+    opendir(my $dh, $dir) or die user_message(__x("Unable to open {dir}
while ".
+                                                  "removing temporary
".
+                                                  "directory:
{error}",
+                                                  dir => $dir,
+                                                  error => $!));
+    while (my $entry = readdir($dh)) {
+        next if $entry eq '.';
+        next if $entry eq '..';
+
+        my $path = $dir.'/'.$entry;
+        if (-d $path) {
+            _remove_tree($path);
+        } else {
+            unlink($path) or die user_message(__x("Unable to remove {path}
".
+                                                  "while removing
temporary ".
+                                                  "directory:
{error}",
+                                                  path => $path,
+                                                  error => $!));
+        }
+    }
+    closedir($dh);
+    rmdir($dir) or die user_message(__x("Unable to remove {path} while
".
+                                        "removing temporary directory:
{error}",
+                                        path => $dir,
+                                        error => $!));
+}
+
 # Must be called in rhev_helper context
 sub _cleanup
 {
@@ -380,25 +414,7 @@ sub _cleanup
 
     return unless (defined($tmpdir));
 
-    my $errors = [];
-    eval {
-        remove_tree($tmpdir, { error => \$errors });
-    };
-    push(@$errors, $@) if ($@);
-
-    if (@$errors > 0) {
-        foreach my $error (@$errors) {
-            foreach my $file (keys(%$error)) {
-                warn(user_message(__x("Error removing {file}:
{error}",
-                                      file => $file,
-                                      error => $error->{$file})));
-            }
-        }
-
-        die(user_message(__x("Unable to remove temporary directory ".
-                             "{dir}", dir => $tmpdir)));
-    }
-
+    _remove_tree($tmpdir);
     $tmpdir = undef;
 }
 
-- 
1.7.3.5

On Wed, Jan 26, 2011 at 05:05:42PM +0000, Matthew Booth
wrote:
> We were using remove_tree in the RHEV cleanup code. remove_tree seems to
use
> chdir. Unfortunately, as this code will be running seteuid(36), it is not
> unlikely that the current working directory will not be readable. This
causes
> remove_tree to fail.
> 
> This patch replaces remove_tree with a simple recursive remove, obviously
> without using chdir.
Recursive removes like this are full of potential fail.

I'd prefer it if you did:

  system ('rm', '-rf', $path);

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/