Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panic assert failed left: 4096 right: 0 - when sending > 4096 bytes in windows pipe message mode #6460

Open
MolotovCherry opened this issue Apr 4, 2024 · 9 comments · May be fixed by tokio-rs/mio#1772 or tokio-rs/mio#1778
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-net Module: tokio/net

Comments

@MolotovCherry
Copy link

MolotovCherry commented Apr 4, 2024

Version
1.37.0

Platform
Edition Windows 11 Pro
Version 23H2
Installed on ‎10/‎5/‎2022
OS build 22631.3296
Experience Windows Feature Experience Pack 1000.22687.1000.0

Description
Sending a message in the pipe larger than 4096 causes an assertion panic.

use tokio::{
    io::{AsyncReadExt as _, AsyncWriteExt},
    net::windows::named_pipe::{ClientOptions, ServerOptions},
    task,
};

#[tokio::main]
async fn main() {
    const BUFFER_SIZE: usize = 4096;

    const PIPE_NAME: &str = r"\\.\pipe\foobar";

    let h = task::spawn(async {
        let mut buf = vec![0; BUFFER_SIZE];

        loop {
            let mut server = ServerOptions::new()
                .access_inbound(true)
                .access_outbound(true)
                .reject_remote_clients(true)
                .pipe_mode(tokio::net::windows::named_pipe::PipeMode::Message)
                .in_buffer_size(BUFFER_SIZE as u32)
                .out_buffer_size(BUFFER_SIZE as u32)
                .create(PIPE_NAME)
                .unwrap();

            if server.connect().await.is_err() {
                continue;
            }

            _ = server.read(&mut buf).await.unwrap();
        }
    });

    let c = task::spawn(async {
        tokio::time::sleep(std::time::Duration::from_millis(50)).await;
        let mut client = ClientOptions::new().open(PIPE_NAME).unwrap();

        client.write_all(b"osrckodsjvetogpzljchmxgydobdxcvnuycomvmqigmdyhcwoviukhtugkdsrxvnjpiwvddlkaoxncdjosuyetvzymzywkpohlfvbascszryatuvycbuvuzhlnurwxwynjhvhuxjpsosvvtuqfynxnspzjblpjsjkbxfvwwehcxcloojqunfgltxivfsvzegellljmrzxfshatwfjpuoggvqhaclpvmsdbhjjpeamdxvzsmzzqaxpfygbcjmehgbvkrbjbhohvkwcjgbslhrhjblvdqnsjlqtliocomcbubzqxjtahgikfdhbqrrkllewwepdcwguccammwqwscxxndoxrtxaklizcazmeillcztrfkgmvajkpkuytgpaxinkarermlhartmmfnruyweebpmfefmawpdjayfdvupyrbngvdqekhgixpzmgnbfvxvlsxxngvozwkulyoyfwvoqgjijpwzjmmlnjesfamndspeofpfscgejgexyxzdxbyvxysftscenbqroocahwtqkyllxcbivecmoxrfbatnllhxwotapvnniiptcebdqbjchpkhgbqktemykczsatdsxzhtezeypdfwslonqjzcehfcloldoonjpcwufvullfmqlgymqrumeanyffaykakvcqimmrvqmjuahjjozrwgunwlstnhhgdxttamfeerecxzvrhkixjsviwyuihdvekbkucuipqqzekipwutbzofnumzcpflhskjneszjhsbhdtripilzprgcuemgvkvumldktiydmvlehmhvhxybubvsshhoydounbcxzzbqsiytlwdjrwuerhknkjjskjcohznmmbdfsrhyoijxtxeboppbcyvvmeyymlwlzqlnalbgaowlpdvldqagyukvyuuumyozkpbykcqlrbdhyyixaessdgpswepmwacaenioouvumztmwaoihjrxltmavktsduvvmiuytocjxxerndpyixkjfeoncbsgbllkxetmssrcjurccjcjfqwnfwejhxdmlielbfhhbanmozvbuudvdraydpfwgshspvimhvhsooporslglkcnlpjxsbnfndsdxejuyonlgzwakbjebeqmxchdwpdvnkymwmkbtohmxgvilyqckzlhtsbmlosemblxzivjhdchndvhxsbxnjsjsmcrbhyjsrengmgyxsrzkvrmgslwqwyvtacqpotcutsczdsmmbclwjlxvtcyrhwnteyirgmpwqkvehqazhtaqcixaafbiyjbptttjlcblamzbdribivcfimbgrzwdhvjjtjkxibpbvorecuuqpxsfpzxcurysqfczppsbubwaemvglubrncwbccorntogpxspcziabbsnciqehzkapjozxsujnkjepretcczzviiztonpojcswmbkfonsdzdefihvpixmvraepoyyrkzbdyppcdeivksvxfdlvgxpivpfhzlkbuhmxpozzqjbqvlgvxefqiurhspjblgxxeqcjmzhybrlwxcypifprgqkwbncumvqezijdnahyxnwowclohwhhoiyajsymlikvlzyftxcckjfqueodhyhibdpjmpfbqikqvvbpmllevtgimcteryhiqgugzvjwvlbztditseqgtfffkdwavavaqxfbaeehddgwuhktkyfzwrpahoqpmmmsgpkiehkxxwbommitjvinzyctmgurlakpysitavtfxdcbxyhthedegmdbcguugbwjmvukfzygylrbbvrpmwvffpwqttrdkgxsqhgcfqbqvcaednudmglpvcqqkxkbspvrmdntldejyskonznrpzluojqonocuqptqbifxfzbhjxpfatyobpfeocqpelayxotfmruepgmjjtkudfdbgnwpnvlgxpmkwjkxsnsnjkhqharpbuauuobzlqssqfzmtnsmydabkauxdqsthxjiifymagtsswythqhpptzljrtzajhdpgqhjcvobezzqwxzcxevnwscaoailhnppvovgvlakhwzlwuygpkgjhztsgdzfoavbysbacruhzdodipoyqvsuqmtjsjdqfapsqcnngqqfrqaykqmfnlwllcflddvatvwboaaychpgrfvelfzscjmchywakkichpedhrdqbrabslwuqrkofaomzfftaezchbddhqtemwdelhuvfoddfhdtkcshophgslwocghcuyoakfrjsuryicgruiqcllrnhmsnvbfkunyeuzwgqulplmdgvxirhafcmlxwpferuauecitaoxeqbsnmouskhypntxgdcznbppghqvuoeavsrrmdlevqmcgonmbbwgajewftwublgddjrmkmksicqzfitvtabrqovjjhdkryemzbqgqpyeicrndwvsqnaxilbqduwrrctjnbysmjdbqtpqptscztjjzdzzrlakosvoftspewclfkdjnakogmuhtzdmvzosgfbyudrtejjvoalspebaiaikqsslbxfnejpxclvvyvxzclzyhfjvzxprpmejhsxvaebgfzyqnfymybyukuawsouijaqdigvciqndnltkipivgnrnwnvsxihnxhmultkazeddyepsvysbgsnbmfsjaspehncokeyqmcprjqxvrlxoyduoabcakftxjvevlladzmfxvgndzgrfmbokwpkuxzpyqyoeevdicuxsbmtahwuiptqruvkjnuxrtsdjshckoluylzgbmqqjphhclblesgwvnxupulztpwpvjlqtehncmlhzijyhrmovbncxjisyywrrjczwxoycnhyyvbimtdzoixjocpyrwfzfpikteyfrcadhiqmlibpsnsphxksgxlcxxlrzmcczubixhtdoevlasnjcvxjyxyqfajdcpmfmnpxcpoizhxlfjynjcxrijnifyoguabidgkzxhdcqfnnmzzbxgbnzikejfaoogucovgjvlmohddjsubfofdarmzqzwyidsvzidcjwrmagscmzrfqbpbutqrtmlxfttiuywlhtznfhyogdfqtdvvvqoamldklleubwplhdncjsfnccincrnirfbcanthwiiibthxceissbelfurrvbfqonxsxjrwytjfweqwedvzrnizjcmxxpfrennqmdhxpkygdgicckvvoudorqcxmfdipiqtbmrncuwjhqhklercxqzscwfwmkswmxveiosskpizhoglbqpqwewzqwlkftcrrdfbtvrxspkligxpjjjmanlqspaqzseftecqimeullspepcqsztktaapyrhznwutjuqgjkdioueoixppxfnkktbfkyiqlaccastkjupqgvhmmiczadfrarhrwrekhfxwiufhpbhgaykyghltuhlrnosegrihiivnfhkxwjhewbtylfralhkawiazxmqgtewirwjtkavhdbizujklwrtoxatguiqgromiibhvyxoihwanrpryevvopzmtwogqwjrnwlezhrywctcgdouowuaxwdtngbglibbqvrcgqpkhplhpwniuetixppcpcsfwclkqbfwfgvtthqtjomtiolumvazlwgtlbpacilmgnknmdvzdhsannfoazlitsgjumouigmvywvwdsetozojhhbbajwdrienxgqxbbcjrxlwwshbinjjvjcnvgoxnkvppffkaljlvcoxokyazcfqgzxcastntssmuzayllzcnljapjgeygnxgxwjeydybnwkelqybaxtunnuuzcxslddihcrxgpjxbiescrygmxpmtwviwffypbnpgjmipmfaajdymaqggdscopyempytgcioqyjtbemyvdwkfdclfcessaqiepbavmiguxasnuydmpcyuhxiwyicpdwvqhpvzhpivnrgsrcsttjsisqdvdlgecaevqqdssazhchsbxbixzlpctmhsmicmlspczdoghocgioxjlkmkvxjgegmufchblogabsuigjzddcagqseowsnkvtbbatbrpibnyvincfmousmlpoosxzjyloddnyjxxviivpyxipcoinxywhbuaersyovypgt_overwrite").await.unwrap();
        client.write_all(b"some_data").await.unwrap();
    });

    h.await.unwrap();
    c.await.unwrap();
}

Output

   Compiling tester v0.1.0
    Finished dev [unoptimized + debuginfo] target(s) in 0.49s
     Running `R:\rust\debug\tester.exe`
thread 'tokio-runtime-worker' panicked at ~\.cargo\registry\src\index.crates.io-6f17d22bba15001f\mio-0.8.11\src\sys\windows\named_pipe.rs:881:17:
assertion `left == right` failed
  left: 4096
 right: 0
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
worker thread panicking; aborting process
error: process didn't exit successfully: `R:\rust\debug\tester.exe` (exit code: 0xc0000409, STATUS_STACK_BUFFER_OVERRUN)

Granted I'm not well versed in using pipes yet, so perhaps clients should only be send messages of max size MAX_BUF, but on the other hand, it's super easy for a client to crash the server this way
Edit: It's only a debug assert, won't crash in production

@MolotovCherry MolotovCherry added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Apr 4, 2024
@mox692 mox692 added the M-net Module: tokio/net label Apr 4, 2024
@MolotovCherry
Copy link
Author

MolotovCherry commented Apr 4, 2024

This seems to be unrelated to the actual buffer size. If the sent message is more than 4096 at all, this happens.

Perhaps this is related to #5307

@MolotovCherry MolotovCherry changed the title panic assert failed left: 4096 right: 0 - when sending large windows pipe message mode panic assert failed left: 4096 right: 0 - when sending > 4096 bytes in windows pipe message mode Apr 4, 2024
@MolotovCherry MolotovCherry changed the title panic assert failed left: 4096 right: 0 - when sending > 4096 bytes in windows pipe message mode panic assert failed left: 4096 right: 0 - when sending > 4096 bytes in windows pipe message mode Apr 4, 2024
@Thomasdezeeuw
Copy link
Contributor

Debug assertion that fails: https://github.com/tokio-rs/mio/blob/15050b9a24ffb528e97c61aa789aa338093d6059/src/sys/windows/named_pipe.rs#L881

For some reason we read 4096 bytes and also got an error. Mio is not expecting that.

I don't have a Windows machine to debug this, but it would be nice to know what the error is. Maybe it's an error we can actually ignore? @MolotovCherry can you run with a fork of Mio where the error is logged?

@ChrisDenton
Copy link

I've not looked into it but this documentation seems related:

If a named pipe is being read in message mode and the next message is longer than the nNumberOfBytesToRead parameter specifies, ReadFile returns FALSE and GetLastError returns ERROR_MORE_DATA. The remainder of the message can be read by a subsequent call to the ReadFile or PeekNamedPipe function.

@MolotovCherry
Copy link
Author

MolotovCherry commented Apr 5, 2024

@Thomasdezeeuw The problem is probably what ChrisDenton said, but yes I can run with a fork of Mio where it's logged and report back with the result. How do I do this?

Edit: ^^ Nevermind. Running the original example in release mode and logging the reply of _ = server.read(&mut buf).await.unwrap(); reveals ERROR_MORE_DATA which is normal windows behavior in pipe message mode. I guess the main point is that the library doesn't properly handle this case (at least in the debug_assert)

Though when implementing this in a full read() loop, ERROR_MORE_DATA happens, but there's no way to access the current message. The next read returns the remainder of the data though. I'm unsure if this is the fault of windows or a bug with tokio and if there's any actual winapi way to get the original data; but if there is a winapi way to get the original data, then it might be a tokio bug (depending on the expected behavior of writing to buf when there's an error). The current message is not placed into the buffer passed to read(). So this makes it impossible to handle the ERROR_MORE_DATA case properly, and thus makes a proper implementation of Message mode impossible

@Thomasdezeeuw
Copy link
Contributor

@MolotovCherry can you try tokio-rs/mio#1772 ? You can set the following in your project's dependencies:

mio = { git = "https://github.com/tokio-rs/mio", rev = "ac406235e8e12b95f9b500c6200410ffd89ff10f" }

@MolotovCherry
Copy link
Author

MolotovCherry commented Apr 5, 2024

@MolotovCherry can you try tokio-rs/mio#1772 ? You can set the following in your project's dependencies:

mio = { git = "https://github.com/tokio-rs/mio", rev = "ac406235e8e12b95f9b500c6200410ffd89ff10f" }

I seem to be unable to test this out due to the different versions (1.0 vs 0.8.11). So tokio isn't taking the new dependency

Edit: I cloned it and patched the version manually. You indeed did fix the problem. I now have access to both!

❯ cargo run
   Compiling f v0.1.0 (C:\Users\roeja\Desktop\f)
    Finished dev [unoptimized + debuginfo] target(s) in 0.50s
     Running `R:\rust\debug\f.exe`
4096: buf: "osrckodsjvetogpzljchmxgydobdxcvnuycomvmqigmdyhcwoviukhtugkdsrxvnjpiwvddlkaoxncdjosuyetvzymzywkpohlfvbascszryatuvycbuvuzhlnurwxwynjhvhuxjpsosvvtuqfynxnspzjblpjsjkbxfvwwehcxcloojqunfgltxivfsvzegellljmrzxfshatwfjpuoggvqhaclpvmsdbhjjpeamdxvzsmzzqaxpfygbcjmehgbvkrbjbhohvkwcjgbslhrhjblvdqnsjlqtliocomcbubzqxjtahgikfdhbqrrkllewwepdcwguccammwqwscxxndoxrtxaklizcazmeillcztrfkgmvajkpkuytgpaxinkarermlhartmmfnruyweebpmfefmawpdjayfdvupyrbngvdqekhgixpzmgnbfvxvlsxxngvozwkulyoyfwvoqgjijpwzjmmlnjesfamndspeofpfscgejgexyxzdxbyvxysftscenbqroocahwtqkyllxcbivecmoxrfbatnllhxwotapvnniiptcebdqbjchpkhgbqktemykczsatdsxzhtezeypdfwslonqjzcehfcloldoonjpcwufvullfmqlgymqrumeanyffaykakvcqimmrvqmjuahjjozrwgunwlstnhhgdxttamfeerecxzvrhkixjsviwyuihdvekbkucuipqqzekipwutbzofnumzcpflhskjneszjhsbhdtripilzprgcuemgvkvumldktiydmvlehmhvhxybubvsshhoydounbcxzzbqsiytlwdjrwuerhknkjjskjcohznmmbdfsrhyoijxtxeboppbcyvvmeyymlwlzqlnalbgaowlpdvldqagyukvyuuumyozkpbykcqlrbdhyyixaessdgpswepmwacaenioouvumztmwaoihjrxltmavktsduvvmiuytocjxxerndpyixkjfeoncbsgbllkxetmssrcjurccjcjfqwnfwejhxdmlielbfhhbanmozvbuudvdraydpfwgshspvimhvhsooporslglkcnlpjxsbnfndsdxejuyonlgzwakbjebeqmxchdwpdvnkymwmkbtohmxgvilyqckzlhtsbmlosemblxzivjhdchndvhxsbxnjsjsmcrbhyjsrengmgyxsrzkvrmgslwqwyvtacqpotcutsczdsmmbclwjlxvtcyrhwnteyirgmpwqkvehqazhtaqcixaafbiyjbptttjlcblamzbdribivcfimbgrzwdhvjjtjkxibpbvorecuuqpxsfpzxcurysqfczppsbubwaemvglubrncwbccorntogpxspcziabbsnciqehzkapjozxsujnkjepretcczzviiztonpojcswmbkfonsdzdefihvpixmvraepoyyrkzbdyppcdeivksvxfdlvgxpivpfhzlkbuhmxpozzqjbqvlgvxefqiurhspjblgxxeqcjmzhybrlwxcypifprgqkwbncumvqezijdnahyxnwowclohwhhoiyajsymlikvlzyftxcckjfqueodhyhibdpjmpfbqikqvvbpmllevtgimcteryhiqgugzvjwvlbztditseqgtfffkdwavavaqxfbaeehddgwuhktkyfzwrpahoqpmmmsgpkiehkxxwbommitjvinzyctmgurlakpysitavtfxdcbxyhthedegmdbcguugbwjmvukfzygylrbbvrpmwvffpwqttrdkgxsqhgcfqbqvcaednudmglpvcqqkxkbspvrmdntldejyskonznrpzluojqonocuqptqbifxfzbhjxpfatyobpfeocqpelayxotfmruepgmjjtkudfdbgnwpnvlgxpmkwjkxsnsnjkhqharpbuauuobzlqssqfzmtnsmydabkauxdqsthxjiifymagtsswythqhpptzljrtzajhdpgqhjcvobezzqwxzcxevnwscaoailhnppvovgvlakhwzlwuygpkgjhztsgdzfoavbysbacruhzdodipoyqvsuqmtjsjdqfapsqcnngqqfrqaykqmfnlwllcflddvatvwboaaychpgrfvelfzscjmchywakkichpedhrdqbrabslwuqrkofaomzfftaezchbddhqtemwdelhuvfoddfhdtkcshophgslwocghcuyoakfrjsuryicgruiqcllrnhmsnvbfkunyeuzwgqulplmdgvxirhafcmlxwpferuauecitaoxeqbsnmouskhypntxgdcznbppghqvuoeavsrrmdlevqmcgonmbbwgajewftwublgddjrmkmksicqzfitvtabrqovjjhdkryemzbqgqpyeicrndwvsqnaxilbqduwrrctjnbysmjdbqtpqptscztjjzdzzrlakosvoftspewclfkdjnakogmuhtzdmvzosgfbyudrtejjvoalspebaiaikqsslbxfnejpxclvvyvxzclzyhfjvzxprpmejhsxvaebgfzyqnfymybyukuawsouijaqdigvciqndnltkipivgnrnwnvsxihnxhmultkazeddyepsvysbgsnbmfsjaspehncokeyqmcprjqxvrlxoyduoabcakftxjvevlladzmfxvgndzgrfmbokwpkuxzpyqyoeevdicuxsbmtahwuiptqruvkjnuxrtsdjshckoluylzgbmqqjphhclblesgwvnxupulztpwpvjlqtehncmlhzijyhrmovbncxjisyywrrjczwxoycnhyyvbimtdzoixjocpyrwfzfpikteyfrcadhiqmlibpsnsphxksgxlcxxlrzmcczubixhtdoevlasnjcvxjyxyqfajdcpmfmnpxcpoizhxlfjynjcxrijnifyoguabidgkzxhdcqfnnmzzbxgbnzikejfaoogucovgjvlmohddjsubfofdarmzqzwyidsvzidcjwrmagscmzrfqbpbutqrtmlxfttiuywlhtznfhyogdfqtdvvvqoamldklleubwplhdncjsfnccincrnirfbcanthwiiibthxceissbelfurrvbfqonxsxjrwytjfweqwedvzrnizjcmxxpfrennqmdhxpkygdgicckvvoudorqcxmfdipiqtbmrncuwjhqhklercxqzscwfwmkswmxveiosskpizhoglbqpqwewzqwlkftcrrdfbtvrxspkligxpjjjmanlqspaqzseftecqimeullspepcqsztktaapyrhznwutjuqgjkdioueoixppxfnkktbfkyiqlaccastkjupqgvhmmiczadfrarhrwrekhfxwiufhpbhgaykyghltuhlrnosegrihiivnfhkxwjhewbtylfralhkawiazxmqgtewirwjtkavhdbizujklwrtoxatguiqgromiibhvyxoihwanrpryevvopzmtwogqwjrnwlezhrywctcgdouowuaxwdtngbglibbqvrcgqpkhplhpwniuetixppcpcsfwclkqbfwfgvtthqtjomtiolumvazlwgtlbpacilmgnknmdvzdhsannfoazlitsgjumouigmvywvwdsetozojhhbbajwdrienxgqxbbcjrxlwwshbinjjvjcnvgoxnkvppffkaljlvcoxokyazcfqgzxcastntssmuzayllzcnljapjgeygnxgxwjeydybnwkelqybaxtunnuuzcxslddihcrxgpjxbiescrygmxpmtwviwffypbnpgjmipmfaajdymaqggdscopyempytgcioqyjtbemyvdwkfdclfcessaqiepbavmiguxasnuydmpcyuhxiwyicpdwvqhpvzhpivnrgsrcsttjsisqdvdlgecaevqqdssazhchsbxbixzlpctmhsmicmlspczdoghocgioxjlkmkvxjgegmufchblogabsuigjzddcagqseowsnkvtbbatbrpibnyvincfmousmlpoosxzjyloddnyjxxviivpyxipcoinxywhbuaersyovypgt"
10: buf: "_overwrite"

@forxfoo
Copy link

forxfoo commented Apr 18, 2024

Can ERROR_MORE_DATA be ignored? I ignored this error in the read_overlapped method and solved it perfectly.

just add err.raw_os_error() == Some(ERROR_MORE_DATA as i32) ignore this more_data error.

pub unsafe fn read_overlapped(
        &self,
        buf: &mut [u8],
        overlapped: *mut OVERLAPPED,
    ) -> io::Result<Option<usize>> {
        let len = std::cmp::min(buf.len(), u32::MAX as usize) as u32;
        let res = ReadFile(
            self.handle.raw(),
            buf.as_mut_ptr() as *mut _,
            len,
            std::ptr::null_mut(),
            overlapped,
        );
        if res == 0 {
            let err = io::Error::last_os_error();
            if err.raw_os_error() != Some(ERROR_IO_PENDING as i32) {
                return Err(err);
            }
        }

        let mut bytes = 0;
        let res = GetOverlappedResult(self.handle.raw(), overlapped, &mut bytes, 0);
        if res == 0 {
            let err = io::Error::last_os_error();
            if err.raw_os_error() == Some(ERROR_IO_INCOMPLETE as i32) || err.raw_os_error() == Some(ERROR_MORE_DATA as i32) {
                Ok(None)
            } else {
                Err(err)
            }
        } else {
            Ok(Some(bytes as usize))
        }
    }

@Thomasdezeeuw
Copy link
Contributor

@forxfoo I think tokio-rs/mio#1772 will solve the problem.

It's still an open question on how to deal with datagram pipes though.

@forxfoo
Copy link

forxfoo commented Apr 19, 2024

@forxfoo I think tokio-rs/mio#1772 will solve the problem.

It's still an open question on how to deal with datagram pipes though.

I used this pr before, but still encountered some problems.

  • unreachable
  • Ok(n) => debug_assert_eq!(status.bytes_transferred() as usize, n);

so I looked at the code and made the simple fixes as above.

i sorry, i use 0.8.11 version, and need to ignore this more_data_error at this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-net Module: tokio/net
Projects
None yet
5 participants