windows projection for NtWriteFile/NtReadFile seems to fail occasionally.

[chyyran]

Summary

When using NtWriteFile and NtReadFile, perhaps other ntifs.h headers, through windows, it does not seem to properly fill out the provided IO_STATUS_BLOCK information leading to reading of potentially uninitialized memory. This issue does not seem to occur with windows-sys.

I also tried with master on windows thinking #2646 would be helpful, but it exhibits the same behaviour.

Crate manifest

windows = { version = "0.51.0", features = ["Win32_Foundation", "Win32_System_LibraryLoader", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_WindowsProgramming", "Win32_System_Console", "Win32_System_IO",
    "Wdk_Foundation", "Wdk_Storage_FileSystem", "Wdk_System_IO", "Wdk_Storage_FileSystem_Minifilters", "Wdk_System_SystemServices", "Win32_System_Ioctl", "Win32_System_SystemServices"] }
windows-sys = { version = "0.52.0", features = ["Win32_Foundation", "Win32_System_LibraryLoader", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_WindowsProgramming", "Win32_System_Console", "Win32_System_IO",
    "Wdk_Foundation", "Wdk_Storage_FileSystem", "Wdk_System_IO", "Wdk_Storage_FileSystem_Minifilters", "Wdk_System_SystemServices", "Win32_System_Ioctl", "Win32_System_SystemServices"], git = "https://github.com/microsoft/windows-rs",  branch = "master" }

Crate code

Consider this version with windows, which does not work at all times, occasionally after the call to NtWriteFile, iosb is still uninitialized. This can be seen by changing to MaybeUninit::uninit instead of MaybeUninit::zeroed

thread_local! {
    static LFS_EVENT: HANDLE =  unsafe { CreateEventW(None, true, false, PCWSTR::null()) }.unwrap();
}
pub trait ToNtStatus {
    fn to_ntstatus(&self) -> NTSTATUS;
}
impl ToNtStatus for HRESULT {
    fn to_ntstatus(&self) -> NTSTATUS {
        NTSTATUS(&self.0 & !(1 << 28))
    }
}

pub fn lfs_write_file(handle: HANDLE, buffer: &[u8], offset: u64, bytes_transferred: &mut u32) -> Result<()> {
    LFS_EVENT.with(|event| {
        let mut iosb: MaybeUninit<IO_STATUS_BLOCK> = MaybeUninit::zeroed();
        let offset: i64 = offset as i64;

        let result = unsafe {
            NtWriteFile(
                handle,
                *event,
                None,
                None,
                iosb.as_mut_ptr(),
                buffer.as_ptr() as *const _,
                buffer.len() as u32,
                Some(&offset),
                None,
            )
        };

        if result.as_ref().is_err_and(|e| e.code().to_ntstatus() == STATUS_PENDING) {
            unsafe {
                WaitForSingleObject(*event, INFINITE);
            }
            let iosb = unsafe { iosb.assume_init() };
            let code = unsafe { iosb.Anonymous.Status };

            *bytes_transferred = iosb.Information as u32;

            if code.is_err() {
                return Err(windows::core::Error::from(code));
            }
        }


        let iosb = unsafe { iosb.assume_init() };
        *bytes_transferred = iosb.Information as u32;
        Ok(result?)
    })
}

This version using windows-sys, which is intended to be equivalent, does not exhibit this behaviour (mixing windows code for Result types etc.)

pub fn lfs_write_file(
    handle: HANDLE,
    buffer: &[u8],
    offset: u64,
    bytes_transferred: &mut u32,
) -> Result<()> {
    LFS_EVENT.with(|event| {
        let mut iosb: MaybeUninit<_> = MaybeUninit::zeroed();
        let mut offset = offset as i64;

        let mut result = unsafe {
            NTSTATUS(windows_sys::Wdk::Storage::FileSystem::NtWriteFile(
                handle.0,
                event.0,
                None,
                std::ptr::null(),
                iosb.as_mut_ptr(),
                buffer.as_ptr() as *const _,
                buffer.len() as u32,
                &mut offset,
                std::ptr::null_mut(),
            ))
        };

        if result == STATUS_PENDING {
            unsafe {
                WaitForSingleObject(event.clone(), INFINITE);
            }
            let iosb = unsafe { iosb.assume_init() };
            let code = unsafe { iosb.Anonymous.Status };
            *bytes_transferred = iosb.Information as u32;

            result = NTSTATUS(code);
        }

        if result != STATUS_SUCCESS {
            return Err(windows::core::Error::from(NTSTATUS(result)));
        }

        let iosb = unsafe { iosb.assume_init() };
        *bytes_transferred = iosb.Information as u32;

        Ok(())
    })
}

Since this is an UB issue I apologize for the hard to reproduce example here. It does seem like the projection, either due to the NTSTATUS to Error transformation or some other issue, is causing some sort of UB where NtWriteFile et. al is not working correctly.

[ChrisDenton]

Ok, this was figured out on discord. STATUS_PENDING (0x104) is a "success" status code, not an error. But the trouble is that the windows crate doesn't return a status code on success so it can't be checked.

[chyyran]

The only NTSTATUS status code that should be transformed into Ok should be STATUS_SUCCESS (0x00000000) imho, every other return value could possibly require additional handling.

I'll close this issue and file one with an easier repro to keep things clean here.

[chyyran]

Root caused as, and closing in favour of #2656