diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 46c49dbc7cb..87c244a72ce 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -838,20 +838,8 @@ impl Chmoder { // Use safe traversal to change the mode if let Err(e) = dir_fd.chmod_at(entry_name, new_mode, symlink_behavior) { - if self.verbose { - println!( - "failed to change mode of {} to {new_mode:o}", - file_path.quote(), - ); - } - - return Err(match e.kind() { - std::io::ErrorKind::ReadOnlyFilesystem => { - ChmodError::ChangingPermissions(file_path.into(), e) - } - _ => ChmodError::PermissionDenied(file_path.into()), - } - .into()); + self.report_failed_change(file_path, e, current_mode, new_mode); + return Ok(()); } // Report the change using the helper method @@ -907,32 +895,39 @@ impl Chmoder { return Ok(()); } - self.change_file(fperm, self.fmode.unwrap_or(new_mode), file)?; - - self.report_umask_conflict(file, new_mode, naively_expected_new_mode); + if self.change_file(fperm, self.fmode.unwrap_or(new_mode), file) { + self.report_umask_conflict(file, new_mode, naively_expected_new_mode); + } Ok(()) } - fn change_file(&self, fperm: u32, mode: u32, file: &Path) -> UResult<()> { + /// Returns false if the mode could not be changed (already reported). + fn change_file(&self, fperm: u32, mode: u32, file: &Path) -> bool { // Always issue the chmod(2) call, even when the bits are unchanged: the // syscall can still fail (e.g. lacking permission on the file) and that // failure must be reported, matching GNU. if let Err(err) = fs::set_permissions(file, fs::Permissions::from_mode(mode)) { - if self.verbose { - println!( - "failed to change mode of file {} from {fperm:04o} ({}) to {mode:04o} ({})", - file.quote(), - display_permissions_unix(fperm, false), - display_permissions_unix(mode, false) - ); - } - return Err(ChmodError::ChangingPermissions(file.into(), err).into()); + self.report_failed_change(file, err, fperm, mode); + return false; } // Use the helper method for consistent reporting self.report_permission_change(file, fperm, mode); - Ok(()) + true + } + + /// Report a failed chmod: the error first, then the `-v` line, like GNU. + fn report_failed_change(&self, file: &Path, err: std::io::Error, old_mode: u32, new_mode: u32) { + self.report_failure(ChmodError::ChangingPermissions(file.into(), err).into()); + if self.verbose { + println!( + "failed to change mode of {} from {old_mode:04o} ({}) to {new_mode:04o} ({})", + file.quote(), + display_permissions_unix(old_mode, false), + display_permissions_unix(new_mode, false) + ); + } } } diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 11f8ac65f52..8f43cf447c5 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -596,6 +596,36 @@ fn test_chmod_readonly_filesystem() { scene.umount_temp_fs(); } +#[test] +fn test_chmod_recursive_reports_real_error_below_operand() { + use rustix::process::geteuid; + + // '-L' makes chmod follow the link to a root-owned file: EPERM + let target = "/etc/passwd"; + let Ok(meta) = metadata(target) else { return }; + if geteuid().is_root() || meta.uid() != 0 { + return; + } + let mode = meta.mode() & 0o7777; + + let ts = TestScenario::new(util_name!()); + ts.fixtures.mkdir("tree"); + ts.fixtures.symlink_file(target, "tree/link"); + + ts.ucmd() + .args(&["-R", "-L", "u+r", "tree"]) + .fails_with_code(1) + .stderr_is("chmod: changing permissions of 'tree/link': Operation not permitted\n"); + + ts.ucmd() + .args(&["-v", "-R", "-L", "u+r", "tree"]) + .fails_with_code(1) + .stdout_contains(format!( + "failed to change mode of 'tree/link' from {mode:04o} (" + )) + .stdout_contains(format!(") to {:04o} (", mode | 0o400)); +} + #[test] #[allow(clippy::unreadable_literal)] fn test_chmod_recursive_correct_exit_code() { @@ -1894,7 +1924,7 @@ fn test_chmod_reports_every_failing_operand() { .args(&["-v", "-f", "0", "/", "/"]) .fails_with_code(1) .no_stderr() - .stdout_contains("failed to change mode of file '/' from"); + .stdout_contains("failed to change mode of '/' from"); } #[test]