Merge pull request #9016 from naoNao89/fix/dd-oflag-direct-isolated

fix(dd): handle O_DIRECT partial block writes

Author: sylvestre

src/uu/dd/src/dd.rs

@@ -686,6 +686,54 @@ fn is_sparse(buf: &[u8]) -> bool {
     buf.iter().all(|&e| e == 0u8)
 }
 
+/// Handle O_DIRECT write errors by temporarily removing the flag and retrying.
+/// This follows GNU dd behavior for partial block writes with O_DIRECT.
+#[cfg(any(target_os = "linux", target_os = "android"))]
+fn handle_o_direct_write(f: &mut File, buf: &[u8], original_error: io::Error) -> io::Result<usize> {
+    use nix::fcntl::{FcntlArg, OFlag, fcntl};
+
+    // Get current flags using nix
+    let oflags = match fcntl(&mut *f, FcntlArg::F_GETFL) {
+        Ok(flags) => OFlag::from_bits_retain(flags),
+        Err(_) => return Err(original_error),
+    };
+
+    // If O_DIRECT is set, try removing it temporarily
+    if oflags.contains(OFlag::O_DIRECT) {
+        let flags_without_direct = oflags - OFlag::O_DIRECT;
+
+        // Remove O_DIRECT flag using nix
+        if fcntl(&mut *f, FcntlArg::F_SETFL(flags_without_direct)).is_err() {
+            return Err(original_error);
+        }
+
+        // Retry the write without O_DIRECT
+        let write_result = f.write(buf);
+
+        // Restore O_DIRECT flag using nix (GNU doesn't restore it, but we'll be safer)
+        // Log any restoration errors without failing the operation
+        if let Err(os_err) = fcntl(&mut *f, FcntlArg::F_SETFL(oflags)) {
+            // Just log the error, don't fail the whole operation
+            show_error!("Failed to restore O_DIRECT flag: {}", os_err);
+        }
+
+        write_result
+    } else {
+        // O_DIRECT wasn't set, return original error
+        Err(original_error)
+    }
+}
+
+/// Stub for non-Linux platforms - just return the original error.
+#[cfg(not(any(target_os = "linux", target_os = "android")))]
+fn handle_o_direct_write(
+    _f: &mut File,
+    _buf: &[u8],
+    original_error: io::Error,
+) -> io::Result<usize> {
+    Err(original_error)
+}
+
 impl Write for Dest {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
         match self {
@@ -697,7 +745,21 @@ impl Write for Dest {
                 f.seek(SeekFrom::Current(seek_amt))?;
                 Ok(buf.len())
             }
-            Self::File(f, _) => f.write(buf),
+            Self::File(f, _) => {
+                // Try the write first
+                match f.write(buf) {
+                    Ok(len) => Ok(len),
+                    Err(e)
+                        if e.kind() == io::ErrorKind::InvalidInput
+                            && e.raw_os_error() == Some(libc::EINVAL) =>
+                    {
+                        // This might be an O_DIRECT alignment issue.
+                        // Try removing O_DIRECT temporarily and retry.
+                        handle_o_direct_write(f, buf, e)
+                    }
+                    Err(e) => Err(e),
+                }
+            }
             Self::Stdout(stdout) => stdout.write(buf),
             #[cfg(unix)]
             Self::Fifo(f) => f.write(buf),

tests/by-util/test_dd.rs

@@ -1780,3 +1780,53 @@ fn test_wrong_number_err_msg() {
         .fails()
         .stderr_contains("dd: invalid number: '1kBb555'\n");
 }
+
+#[test]
+#[cfg(any(target_os = "linux", target_os = "android"))]
+fn test_oflag_direct_partial_block() {
+    // Test for issue #9003: dd should handle partial blocks with oflag=direct
+    // This reproduces the scenario where writing a partial block with O_DIRECT fails
+
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    // Create input file with size that's not a multiple of block size
+    // This will trigger the partial block write issue
+    let input_file = "test_direct_input.iso";
+    let output_file = "test_direct_output.img";
+    let block_size = 8192; // 8K blocks
+    let input_size = block_size * 3 + 511; // 3 full blocks + 511 byte partial block
+
+    // Create test input file with known pattern
+    let input_data = vec![0x42; input_size]; // Use non-zero pattern for better verification
+    at.write_bytes(input_file, &input_data);
+
+    // Get full paths for the dd command
+    let input_path = at.plus(input_file);
+    let output_path = at.plus(output_file);
+
+    // Test with oflag=direct - should succeed with the fix
+    new_ucmd!()
+        .args(&[
+            format!("if={}", input_path.display()),
+            format!("of={}", output_path.display()),
+            "oflag=direct".to_string(),
+            format!("bs={block_size}"),
+            "status=none".to_string(),
+        ])
+        .succeeds()
+        .stdout_is("")
+        .stderr_is("");
+    assert!(output_path.exists());
+    let output_size = output_path.metadata().unwrap().len() as usize;
+    assert_eq!(output_size, input_size);
+
+    // Verify content matches input
+    let output_content = std::fs::read(&output_path).unwrap();
+    assert_eq!(output_content.len(), input_size);
+    assert_eq!(output_content, input_data);
+
+    // Clean up
+    at.remove(input_file);
+    at.remove(output_file);
+}