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

fs::read_dir() fails with error 0 on Linux if directory is on NTFS and contains file with filename > 255 bytes #86649

Closed
ariasuni opened this issue Jun 26, 2021 · 4 comments · Fixed by #92778
Labels
C-bug Category: This is a bug. O-linux Operating system: Linux T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@ariasuni
Copy link
Contributor

  1. Create a filename on an NTFS partition with a name > 255 bytes, e.g. 一二三四五六七八九十一二三四五六七八九十一二三四五六七八九十一二三四五六七八九十一二三四五六七八九十一二三四五六七八九十一二三四五六七八九十一二三四五六七八九十一二三四五六七八九十.
  2. Run this code with the path to the directory containing the file with the very long filename:
use std::env;
use std::fs::{self, ReadDir};

fn main() {
    test().unwrap();
}

fn test() -> std::io::Result<ReadDir> {
    let args: Vec<String> = env::args().collect();
    for result in fs::read_dir(&args[1])? {
        result.map(|entry| entry.path()).unwrap();
    }
    todo!()
}

I expected to see this happen: the code works until it panics on todo!()

Instead, this happened: thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 0, kind: Other, message: "Success" }', test.rs:11:42. Note that in exa, it fails with Bad address (os error 14).

Meta

rustc --version --verbose:

rustc 1.53.0 (53cb7b09b 2021-06-17)
binary: rustc
commit-hash: 53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b
commit-date: 2021-06-17
host: x86_64-unknown-linux-gnu
release: 1.53.0
LLVM version: 12.0.1
@ariasuni ariasuni added the C-bug Category: This is a bug. label Jun 26, 2021
@jonas-schievink jonas-schievink added O-linux Operating system: Linux T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 26, 2021
@ChrisDenton
Copy link
Member

ChrisDenton commented Jun 26, 2021

So NTFS has a limit of 255 UTF-16 code units. However, something here is interpreting that as being a UTF-8 byte limit of 255.

I think this error originates outside of Rust? If libc::opendir is returning a null pointer then I'm not sure what Rust can do?

@ghost
Copy link

ghost commented Jun 27, 2021

I think this error originates outside of Rust? If libc::opendir is returning a null pointer then I'm not sure what Rust can do?

Note that the screenshots in ogham/exa#893 (comment) shows that ls works, and I don't think opendir failed because the line that panicked (line 11) is result.map(|entry| entry.path()).unwrap();. I think it was readdir_r.

I guess it reported strange errors because this is not correct:

return Some(Err(Error::last_os_error()));

According to https://manpages.debian.org/unstable/manpages-dev/readdir_r.3.en.html#RETURN_VALUE, readdir_r "returns a positive error number" on error. It does not mention that it'll set errno, so I think Error::last_os_error() might not return the correct error.

However, the code does check the return value != 0, so I think readdir_r indeed failed. I'm not sure why, but https://manpages.debian.org/unstable/manpages-dev/readdir_r.3.en.html#DESCRIPTION and #34668 list some limitations related to long directory entry names.

@RalfJung
Copy link
Member

RalfJung commented Jul 3, 2021

It does not mention that it'll set errno, so I think Error::last_os_error() might not return the correct error code.

Ah, I was already wondering why the error message says Success. 😂

m-ou-se pushed a commit to m-ou-se/rust that referenced this issue Sep 1, 2021
POSIX says:

> If successful, the readdir_r() function shall return zero; otherwise,
> an error number shall be returned to indicate the error.

But we were previously using errno instead of the return value.  This
led to issue rust-lang#86649.
m-ou-se added a commit to m-ou-se/rust that referenced this issue Sep 1, 2021
Use the return value of readdir_r() instead of errno

POSIX says:

> If successful, the readdir_r() function shall return zero; otherwise,
> an error number shall be returned to indicate the error.

But we were previously using errno instead of the return value.  This
led to issue rust-lang#86649.
@tavianator
Copy link
Contributor

So this should no longer say "success". It should say something like "file name too long" instead now. A complete fix is just replacing readdir_r() with readdir() on LInux, which I'll do unless someone beats me to it. See #40021 for some historical details.

tavianator added a commit to tavianator/rust that referenced this issue Jan 11, 2022
readdir() is preferred over readdir_r() on Linux and many other
platforms because it more gracefully supports long file names.  Both
glibc and musl (and presumably all other Linux libc implementations)
guarantee that readdir() is thread-safe as long as a single DIR* is not
accessed concurrently, which is enough to make a readdir()-based
implementation of ReadDir safe.  This implementation is already used for
some other OSes including Fuchsia, Redox, and Solaris.

See rust-lang#40021 for more details.  Fixes rust-lang#86649.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 11, 2022
…shtriplett

fs: Use readdir() instead of readdir_r() on Linux

readdir() is preferred over readdir_r() on Linux and many other
platforms because it more gracefully supports long file names.  Both
glibc and musl (and presumably all other Linux libc implementations)
guarantee that readdir() is thread-safe as long as a single DIR* is not
accessed concurrently, which is enough to make a readdir()-based
implementation of ReadDir safe.  This implementation is already used for
some other OSes including Fuchsia, Redox, and Solaris.

See rust-lang#40021 for more details.  Fixes rust-lang#86649.
tavianator added a commit to tavianator/rust that referenced this issue Jan 11, 2022
readdir() is preferred over readdir_r() on Linux and many other
platforms because it more gracefully supports long file names.  Both
glibc and musl (and presumably all other Linux libc implementations)
guarantee that readdir() is thread-safe as long as a single DIR* is not
accessed concurrently, which is enough to make a readdir()-based
implementation of ReadDir safe.  This implementation is already used for
some other OSes including Fuchsia, Redox, and Solaris.

See rust-lang#40021 for more details.  Fixes rust-lang#86649.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 14, 2022
…shtriplett

fs: Use readdir() instead of readdir_r() on Linux

readdir() is preferred over readdir_r() on Linux and many other
platforms because it more gracefully supports long file names.  Both
glibc and musl (and presumably all other Linux libc implementations)
guarantee that readdir() is thread-safe as long as a single DIR* is not
accessed concurrently, which is enough to make a readdir()-based
implementation of ReadDir safe.  This implementation is already used for
some other OSes including Fuchsia, Redox, and Solaris.

See rust-lang#40021 for more details.  Fixes rust-lang#86649.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 26, 2022
…shtriplett

fs: Use readdir() instead of readdir_r() on Linux and Android

See rust-lang#40021 for more details.  Fixes rust-lang#86649.  Fixes rust-lang#34668.
@bors bors closed this as completed in bc04a4e Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-linux Operating system: Linux T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants