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

File::set_len uses an unsafe cast on Unix #63326

Closed
marmistrz opened this issue Aug 6, 2019 · 1 comment · Fixed by #63332 or bytecodealliance/wasmtime#2164
Closed

File::set_len uses an unsafe cast on Unix #63326

marmistrz opened this issue Aug 6, 2019 · 1 comment · Fixed by #63332 or bytecodealliance/wasmtime#2164
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@marmistrz
Copy link
Contributor

marmistrz commented Aug 6, 2019

Precisely, size as off64_t. The conversion occurs here
On my system off64_t is i64. This means that the number will overflow if larger than i64::max_value() and become negative. In such case, ftruncate will return an EINVAL: (from man ftruncate:

       EINVAL The argument length is negative or larger than the maximum file size.

Should we manually return an error should an overflow occur or is the EINVAL returned by ftruncate the intended behavior? I would return an io::ErrorKind::InvalidInput with the payload indicating that an overflow occured (or even just TryFromIntError(()) which try_into returns)

This issue was noticed while discussing CraneStation/wasi-common#47.
cc @alexcrichton

I will implement the fix once we've decided what kind of behavior we want.

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Aug 6, 2019
@alexcrichton
Copy link
Member

I'd personally be in favor of returning the error ourselves, relying on try_into to generate an error which we then convert to an io::Error quickly

marmistrz added a commit to marmistrz/wasi-common that referenced this issue Aug 6, 2019
The check will be removed when rust-lang/rust#63326 is fixed
pietroalbini added a commit to pietroalbini/rust that referenced this issue Aug 7, 2019
Add an overflow check in truncate implementation for Unix.

Closes rust-lang#63326.
cc @alexcrichton
Centril added a commit to Centril/rust that referenced this issue Aug 8, 2019
Add an overflow check in truncate implementation for Unix.

Closes rust-lang#63326.
cc @alexcrichton
marmistrz added a commit to marmistrz/wasi-common that referenced this issue Aug 8, 2019
The check will be removed when rust-lang/rust#63326 is fixed
marmistrz added a commit to marmistrz/wasi-common that referenced this issue Aug 8, 2019
The check will be removed when rust-lang/rust#63326 is fixed
kubkon pushed a commit to CraneStation/wasi-common that referenced this issue Aug 8, 2019
The check will be removed when rust-lang/rust#63326 is fixed
pchickey added a commit to bytecodealliance/wasmtime that referenced this issue Aug 26, 2020
issue now closed: rust-lang/rust#63326
fix landed in a rollup: rust-lang/rust#63380 merged aug 8, 2019
rust 1.38 beta cut on aug 15, 2019: https://blog.rust-lang.org/2019/08/15/Rust-1.37.0.html
pchickey added a commit to bytecodealliance/wasmtime that referenced this issue Aug 26, 2020
issue now closed: rust-lang/rust#63326
fix landed in a rollup: rust-lang/rust#63380 merged aug 8, 2019
rust 1.38 beta cut on aug 15, 2019: https://blog.rust-lang.org/2019/08/15/Rust-1.37.0.html
minimum supported rust version for this repo is currently 1.41.
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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
3 participants