Skip to content

Commit 351e8b2

Browse files
committed
feat(rm): implement one-file-system check for safe recursive removal
- Track device IDs (st_dev) during safe directory traversal to respect `--one-file-system` and `--preserve-root=all` options. - Pass the parent device ID through recursive calls in `safe_remove_dir_recursive_impl`. - Refactor `show_one_fs_error` into a reusable function to unify error reporting. - Ensure the parent directory is not removed if any child removal fails or is skipped.
1 parent 4556d29 commit 351e8b2

File tree

2 files changed

+59
-23
lines changed

2 files changed

+59
-23
lines changed

src/uu/rm/src/platform/unix.rs

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use indicatif::ProgressBar;
1111
use std::ffi::OsStr;
1212
use std::fs;
1313
use std::io::{IsTerminal, stdin};
14+
use std::os::unix::fs::MetadataExt;
1415
use std::os::unix::fs::PermissionsExt;
1516
use std::path::Path;
1617
use uucore::display::Quotable;
@@ -21,9 +22,9 @@ use uucore::show_error;
2122
use uucore::translate;
2223

2324
use super::super::{
24-
InteractiveMode, Options, is_dir_empty, is_readable_metadata, prompt_descend, remove_file,
25-
show_permission_denied_error, show_removal_error, verbose_removed_directory,
26-
verbose_removed_file,
25+
InteractiveMode, Options, PreserveRoot, is_dir_empty, is_readable_metadata, prompt_descend,
26+
remove_file, show_one_fs_error, show_permission_denied_error, show_removal_error,
27+
verbose_removed_directory, verbose_removed_file,
2728
};
2829

2930
#[inline]
@@ -279,11 +280,11 @@ pub fn safe_remove_dir_recursive(
279280
) -> bool {
280281
// Base case 1: this is a file or a symbolic link.
281282
// Use lstat to avoid race condition between check and use
282-
let initial_mode = match fs::symlink_metadata(path) {
283+
let (initial_mode, initial_dev) = match fs::symlink_metadata(path) {
283284
Ok(metadata) if !metadata.is_dir() => {
284285
return remove_file(path, options, progress_bar);
285286
}
286-
Ok(metadata) => metadata.permissions().mode(),
287+
Ok(metadata) => (metadata.permissions().mode(), metadata.dev()),
287288
Err(e) => {
288289
return show_removal_error(e, path);
289290
}
@@ -309,7 +310,7 @@ pub fn safe_remove_dir_recursive(
309310
}
310311
};
311312

312-
let error = safe_remove_dir_recursive_impl(path, &dir_fd, options);
313+
let error = safe_remove_dir_recursive_impl(path, &dir_fd, options, Some(initial_dev));
313314

314315
// After processing all children, remove the directory itself
315316
if error {
@@ -346,7 +347,12 @@ pub fn safe_remove_dir_recursive(
346347
}
347348

348349
#[cfg(not(target_os = "redox"))]
349-
pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Options) -> bool {
350+
pub fn safe_remove_dir_recursive_impl(
351+
path: &Path,
352+
dir_fd: &DirFd,
353+
options: &Options,
354+
parent_dev: Option<u64>,
355+
) -> bool {
350356
// Read directory entries using safe traversal
351357
let entries = match dir_fd.read_dir() {
352358
Ok(entries) => entries,
@@ -380,11 +386,22 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt
380386
let is_dir = ((entry_stat.st_mode as libc::mode_t) & libc::S_IFMT) == libc::S_IFDIR;
381387

382388
if is_dir {
389+
if options.one_fs || options.preserve_root == PreserveRoot::YesAll {
390+
if let Some(p_dev) = parent_dev {
391+
if entry_stat.st_dev as u64 != p_dev {
392+
show_one_fs_error(&entry_path, options);
393+
error = true;
394+
continue;
395+
}
396+
}
397+
}
398+
383399
// Ask user if they want to descend into this directory
384400
if options.interactive == InteractiveMode::Always
385401
&& !is_dir_empty(&entry_path)
386402
&& !prompt_descend(&entry_path)
387403
{
404+
error = true;
388405
continue;
389406
}
390407

@@ -408,25 +425,35 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt
408425
}
409426
};
410427

411-
let child_error = safe_remove_dir_recursive_impl(&entry_path, &child_dir_fd, options);
412-
error |= child_error;
428+
let child_error = safe_remove_dir_recursive_impl(
429+
&entry_path,
430+
&child_dir_fd,
431+
options,
432+
Some(entry_stat.st_dev as u64)
433+
);
434+
435+
// If a child could not be removed, the parent directory cannot be removed either.
436+
if child_error {
437+
error = true;
438+
continue;
439+
}
413440

414441
// Ask user permission if needed for this subdirectory
415-
if !child_error
416-
&& options.interactive == InteractiveMode::Always
442+
if options.interactive == InteractiveMode::Always
417443
&& !prompt_dir_with_mode(&entry_path, entry_stat.st_mode as libc::mode_t, options)
418444
{
445+
error = true;
419446
continue;
420447
}
421448

422449
// Remove the now-empty subdirectory using safe unlinkat
423-
if !child_error {
424-
error |= handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, true, options);
425-
}
450+
error |= handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, true, options);
426451
} else {
427452
// Remove file - check if user wants to remove it first
428453
if prompt_file_with_stat(&entry_path, &entry_stat, options) {
429454
error |= handle_unlink(dir_fd, entry_name.as_ref(), &entry_path, false, options);
455+
} else {
456+
error = true;
430457
}
431458
}
432459
}
@@ -435,7 +462,12 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt
435462
}
436463

437464
#[cfg(target_os = "redox")]
438-
pub fn safe_remove_dir_recursive_impl(_path: &Path, _dir_fd: &DirFd, _options: &Options) -> bool {
465+
pub fn safe_remove_dir_recursive_impl(
466+
_path: &Path,
467+
_dir_fd: &DirFd,
468+
_options: &Options,
469+
_parent_dev: Option<u64>,
470+
) -> bool {
439471
// safe_traversal stat_at is not supported on Redox
440472
// This shouldn't be called on Redox, but provide a stub for compilation
441473
true // Return error

src/uu/rm/src/rm.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -586,21 +586,25 @@ fn is_writable_metadata(_metadata: &Metadata) -> bool {
586586
true
587587
}
588588

589+
pub(crate) fn show_one_fs_error(path: &Path, options: &Options) {
590+
show_error!(
591+
"skipping {}, since it's on a different device",
592+
path.quote()
593+
);
594+
595+
if !options.one_fs && options.preserve_root == PreserveRoot::YesAll {
596+
show_error!("and --preserve-root=all is in effect");
597+
}
598+
}
599+
589600
/// Helper function to check fs and report errors if necessary.
590601
/// Returns true if the operation should be skipped/returned (i.e., on error).
591602
fn check_and_report_one_fs(path: &Path, options: &Options) -> bool {
592603
if let Err(additional_reason) = check_one_fs(path, options) {
593604
if !additional_reason.is_empty() {
594605
show_error!("{}", additional_reason);
595606
}
596-
show_error!(
597-
"skipping {}, since it's on a different device",
598-
path.quote()
599-
);
600-
601-
if options.preserve_root == PreserveRoot::YesAll {
602-
show_error!("and --preserve-root=all is in effect");
603-
}
607+
show_one_fs_error(path, options);
604608

605609
return true;
606610
}

0 commit comments

Comments
 (0)