Windows Named pipes invalid memory access

[radekvit]

Version
tokio v1.36.0

Platform
Windows 10 Enterprise 64-bit
version 10.0.19045

Description
When running a simple named pipes tokio program with Application Verifier with heap turned on, invalid memory access is detected in ScheduledIo::set_readiness.

Reproduction code:

Cargo.toml:

[package]
name = "tokio-heaptest"
version = "0.1.0"
edition = "2021"

[dependencies]
tokio = { version = "1.36.0", features = ["net", "macros", "rt", "rt-multi-thread", "time"] }

main.rs:

use std::time::Duration;

use tokio::net::windows::named_pipe::{ClientOptions, ServerOptions};

const PIPE_NAME: &str = r"\\.\pipe\tokio-heap-issue";

#[tokio::main]
async fn main() {
    let connect_client = tokio::spawn(connect_client());
    tokio::spawn(client_fn());

    let connected = tokio::time::timeout(Duration::from_millis(1000), connect_client)
        .await
        .unwrap_or(Ok(Ok(false)))
        .unwrap_or(Ok(false))
        .unwrap_or(false);
    assert!(connected);
}

async fn connect_client() -> std::io::Result<bool> {
    let server = ServerOptions::new()
        .first_pipe_instance(true)
        .create(PIPE_NAME)?;

    server.connect().await?;

    Ok(true)
}

async fn client_fn() {
    // five tries to connect
    for _ in 0..5 {
        tokio::time::sleep(Duration::from_millis(20)).await;
        match ClientOptions::new().read(true).write(true).open(PIPE_NAME) {
            // Return after we've connected
            Ok(_) => return,
            Err(_) => {
                // pipe not created yet or busy, sleep and wait
                continue;
            }
        };
    }
}

With the following settings in Application Verifier

You can produce crash dumps by adding the following settings in the registry (see this article):
In HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\Windows Error Reporting\LocalDumps:

• string DumpFolder: C:\path\to\dump\folder
• DWORD DumpCount: The number of dumps to keep
• DWORD DumpType: 2 for a full dump

I expected to see this happen: The program would run without accessing invalid memory. This happens in both debug and release builds.

Instead, this happened: The application crashes with STATUS_ACCESS_VIOLATION. The dump shows that the program is trying to access invalid memory in ScheduledIo::set_readiness when trying to load an atomic variable, which should mean that ScheduledIo itself has likely been freed. We observed this access violation in our larger application when a named pipe client disconnects.

[radekvit]

The ScheduledIo is freed earlier in io::registration_set::RegistrationSet::release, as the Arc is stored in reduces its reference count to 0 (but is still used later in io::Driver::turn).

Looks like the safety assumption at runtime::io::Driver::turn:178 is broken somewhere.

Here's the stack trace where the ScheduledIo is freed:

0:004> !heap -p -a 00000270E3249FD0
    address 00000270e3249fd0 found in
    _DPH_HEAP_ROOT @ 270dc9b1000
    in free-ed allocation (  DPH_HEAP_BLOCK:         VirtAddr         VirtSize)
                                270dc9f8958:      270e3249000             2000
    00007ff9ce6e9234 ntdll!RtlDebugFreeHeap+0x0000000000000038
    00007ff9ce615cc1 ntdll!RtlpFreeHeap+0x00000000000000c1
    00007ff9ce615b74 ntdll!RtlpFreeHeapInternal+0x0000000000000464
    00007ff9ce6147b1 ntdll!RtlFreeHeap+0x0000000000000051
    00007ff988d3c4fc vrfcore!VfCoreRtlFreeHeap+0x000000000000002c
    00007ff981aa39af vfbasics!AVrfpRtlFreeHeap+0x000000000000011f
    00007ff981aa6fe8 vfbasics!AVrfpHeapFree+0x0000000000000108
    00007ff70b33ab57 tokio_heaptest!alloc::alloc::impl$1::deallocate+0x0000000000000087 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112\library\alloc\src\alloc.rs @ 256]
    00007ff70b339ba0 tokio_heaptest!core::alloc::impl$2::deallocate<alloc::alloc::Global>+0x0000000000000020 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112\library\core\src\alloc\mod.rs @ 387]
    00007ff70b2bf018 tokio_heaptest!alloc::sync::impl$42::drop<tokio::runtime::io::scheduled_io::ScheduledIo,ref$<alloc::alloc::Global> >+0x0000000000000198 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112\library\alloc\src\sync.rs @ 3001]
    00007ff70b2d139e tokio_heaptest!core::ptr::drop_in_place<alloc::sync::Weak<tokio::runtime::io::scheduled_io::ScheduledIo,ref$<alloc::alloc::Global> > >+0x000000000000000e [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112\library\core\src\ptr\mod.rs @ 498]
    00007ff70b2bc1e2 tokio_heaptest!alloc::sync::Arc<tokio::runtime::io::scheduled_io::ScheduledIo,alloc::alloc::Global>::drop_slow<tokio::runtime::io::scheduled_io::ScheduledIo,alloc::alloc::Global>+0x0000000000000042 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112\library\alloc\src\sync.rs @ 1759]
    00007ff70b2bd75c tokio_heaptest!alloc::sync::impl$33::drop<tokio::runtime::io::scheduled_io::ScheduledIo,alloc::alloc::Global>+0x000000000000007c [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112\library\alloc\src\sync.rs @ 2408]
    00007ff70b2d537e tokio_heaptest!core::ptr::drop_in_place<alloc::sync::Arc<tokio::runtime::io::scheduled_io::ScheduledIo,alloc::alloc::Global> >+0x000000000000000e [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112\library\core\src\ptr\mod.rs @ 498]
    00007ff70b2e9510 tokio_heaptest!tokio::runtime::io::registration_set::RegistrationSet::release+0x0000000000000110 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\io\registration_set.rs @ 108]
    00007ff70b3127d8 tokio_heaptest!tokio::runtime::io::driver::Handle::release_pending_registrations+0x0000000000000068 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\io\driver.rs @ 255]
    00007ff70b3120dc tokio_heaptest!tokio::runtime::io::driver::Driver::turn+0x00000000000000ac [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\io\driver.rs @ 143]
    00007ff70b311e00 tokio_heaptest!tokio::runtime::io::driver::Driver::park_timeout+0x0000000000000070 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\io\driver.rs @ 128]
    00007ff70b2fe385 tokio_heaptest!enum2$<tokio::runtime::driver::IoStack>::park_timeout+0x0000000000000065 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\driver.rs @ 180]
    00007ff70b2ff07d tokio_heaptest!tokio::runtime::time::Driver::park_thread_timeout+0x000000000000001d [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\time\mod.rs @ 243]
    00007ff70b2fee86 tokio_heaptest!tokio::runtime::time::Driver::park_internal+0x0000000000000316 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\time\mod.rs @ 198]
    00007ff70b2fea95 tokio_heaptest!tokio::runtime::time::Driver::park+0x0000000000000025 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\time\mod.rs @ 149]
    00007ff70b2fe716 tokio_heaptest!enum2$<tokio::runtime::driver::TimeDriver>::park+0x0000000000000036 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\driver.rs @ 329]
    00007ff70b2fdbb3 tokio_heaptest!tokio::runtime::driver::Driver::park+0x0000000000000013 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\driver.rs @ 70]
    00007ff70b2d74df tokio_heaptest!tokio::runtime::scheduler::multi_thread::park::Inner::park_driver+0x000000000000008f [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\scheduler\multi_thread\park.rs @ 184]
    00007ff70b2d7035 tokio_heaptest!tokio::runtime::scheduler::multi_thread::park::Inner::park+0x00000000000000e5 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\scheduler\multi_thread\park.rs @ 117]
    00007ff70b2d6c95 tokio_heaptest!tokio::runtime::scheduler::multi_thread::park::Parker::park+0x0000000000000025 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\scheduler\multi_thread\park.rs @ 68]
    00007ff70b2e1a98 tokio_heaptest!tokio::runtime::scheduler::multi_thread::worker::Context::park_timeout+0x0000000000000208 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\scheduler\multi_thread\worker.rs @ 723]
    00007ff70b2e1775 tokio_heaptest!tokio::runtime::scheduler::multi_thread::worker::Context::park+0x0000000000000185 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\scheduler\multi_thread\worker.rs @ 693]
    00007ff70b2e0729 tokio_heaptest!tokio::runtime::scheduler::multi_thread::worker::Context::run+0x00000000000004b9 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\scheduler\multi_thread\worker.rs @ 544]
    00007ff70b2e01a5 tokio_heaptest!tokio::runtime::scheduler::multi_thread::worker::run::closure$0::closure$0+0x0000000000000055 [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\scheduler\multi_thread\worker.rs @ 491]
    00007ff70b2cf35b tokio_heaptest!tokio::runtime::context::scoped::Scoped<enum2$<tokio::runtime::scheduler::Context> >::set<enum2$<tokio::runtime::scheduler::Context>,tokio::runtime::scheduler::multi_thread::worker::run::closure$0::closure_env$0,tuple$<> >+0x000000000000007b [C:\Users\JohnDoe\.cargo\registry\src\index.crates.io-6f17d22bba15001f\tokio-1.36.0\src\runtime\context\scoped.rs @ 40]

[radekvit]

I also tried older versions of tokio and found out that this worked fine with up to 1.29.1 and broke in 1.30.

[daira]

tokio-rs/mio#1760 is marked as fixing this bug, but note that tokio's (optional) dependency on mio is still at 0.8.9 which was vulnerable, e.g.:

tokio/tokio/Cargo.toml

Line 98 in f6d0619

mio = { version = "0.8.9", optional = true, default-features = false }

I believe this shouldn't be considered fixed in tokio until no tokio crates depend on any vulnerable version of mio (or they otherwise mitigate the vulnerability).

This is important for downstream projects dependent on tokio since adding a dependency on mio is not the right fix for them, assuming they do not depend on it directly. The right fix is to update their tokio dependency/dependencies to a release that has updated its mio dependencies.

(I know that mio 0.8.11 is a compatible point-release relative to the version that tokio depends on, so in most cases it would be sufficient for downstream projects that are not libraries to just do a cargo update to update their pin on mio. The issue is with having a well-defined version of tokio that is not vulnerable, given how widely depended on tokio is. Upgrading to that version is when downstream projects will stop getting automated security alerts, for instance.)

[Thomasdezeeuw]

@daira you can run cargo update -p mio and you're good.

[daira]

I'm aware of that; see the last paragraph of my comment. (Also see dependabot/dependabot-core#9210 which would fix an issue with getting potentially misleading Dependabot security alerts.)