rm: on linux use the safe traversal in all cases

Author: sylvestre

src/uu/rm/src/rm.rs

@@ -394,7 +394,7 @@ fn is_readable_metadata(metadata: &Metadata) -> bool {
 }
 
 /// Whether the given file or directory is readable.
-#[cfg(unix)]
+#[cfg(all(unix, not(target_os = "linux")))]
 fn is_readable(path: &Path) -> bool {
     match fs::metadata(path) {
         Err(_) => false,
@@ -590,19 +590,14 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
         return false;
     }
 
-    // Use secure traversal on Linux for long paths
+    // Use secure traversal on Linux to prevent TOCTOU attacks
     #[cfg(target_os = "linux")]
-    {
-        if let Some(s) = path.to_str() {
-            if s.len() > 1000 {
-                return safe_remove_dir_recursive(path, options);
-            }
-        }
-    }
+    return safe_remove_dir_recursive(path, options);
 
-    // Fallback for non-Linux or shorter paths
+    // Fallback for non-Linux systems
     #[cfg(not(target_os = "linux"))]
     {
+        // Handle very long paths with remove_dir_all
         if let Some(s) = path.to_str() {
             if s.len() > 1000 {
                 match fs::remove_dir_all(path) {
@@ -617,62 +612,63 @@ fn remove_dir_recursive(path: &Path, options: &Options) -> bool {
                 }
             }
         }
-    }
 
-    // Recursive case: this is a directory.
-    let mut error = false;
-    match fs::read_dir(path) {
-        Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {
-            // This is not considered an error.
-        }
-        Err(_) => error = true,
-        Ok(iter) => {
-            for entry in iter {
-                match entry {
-                    Err(_) => error = true,
-                    Ok(entry) => {
-                        let child_error = remove_dir_recursive(&entry.path(), options);
-                        error = error || child_error;
+        // Recursive case: this is a directory.
+        let mut error = false;
+        match fs::read_dir(path) {
+            Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {
+                // This is not considered an error.
+            }
+            Err(_) => error = true,
+            Ok(iter) => {
+                for entry in iter {
+                    match entry {
+                        Err(_) => error = true,
+                        Ok(entry) => {
+                            let child_error = remove_dir_recursive(&entry.path(), options);
+                            error = error || child_error;
+                        }
                     }
                 }
             }
         }
-    }
-
-    // Ask the user whether to remove the current directory.
-    if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) {
-        return false;
-    }
 
-    // Try removing the directory itself.
-    match fs::remove_dir(path) {
-        Err(_) if !error && !is_readable(path) => {
-            // For compatibility with GNU test case
-            // `tests/rm/unread2.sh`, show "Permission denied" in this
-            // case instead of "Directory not empty".
-            show_error!("cannot remove {}: Permission denied", path.quote());
-            error = true;
-        }
-        Err(e) if !error => {
-            let e =
-                e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote()));
-            show_error!("{e}");
-            error = true;
+        // Ask the user whether to remove the current directory.
+        if options.interactive == InteractiveMode::Always && !prompt_dir(path, options) {
+            return false;
         }
-        Err(_) => {
-            // If there has already been at least one error when
-            // trying to remove the children, then there is no need to
-            // show another error message as we return from each level
-            // of the recursion.
+
+        // Try removing the directory itself.
+        match fs::remove_dir(path) {
+            Err(_) if !error && !is_readable(path) => {
+                // For compatibility with GNU test case
+                // `tests/rm/unread2.sh`, show "Permission denied" in this
+                // case instead of "Directory not empty".
+                show_error!("cannot remove {}: Permission denied", path.quote());
+                error = true;
+            }
+            Err(e) if !error => {
+                let e = e.map_err_context(
+                    || translate!("rm-error-cannot-remove", "file" => path.quote()),
+                );
+                show_error!("{e}");
+                error = true;
+            }
+            Err(_) => {
+                // If there has already been at least one error when
+                // trying to remove the children, then there is no need to
+                // show another error message as we return from each level
+                // of the recursion.
+            }
+            Ok(_) if options.verbose => println!(
+                "{}",
+                translate!("rm-verbose-removed-directory", "file" => normalize(path).quote())
+            ),
+            Ok(_) => {}
         }
-        Ok(_) if options.verbose => println!(
-            "{}",
-            translate!("rm-verbose-removed-directory", "file" => normalize(path).quote())
-        ),
-        Ok(_) => {}
-    }
 
-    error
+        error
+    }
 }
 
 fn handle_dir(path: &Path, options: &Options) -> bool {