Wasmtime GitHub notifications bot (May 26 2023 at 00:22): Wasmtime GitHub notifications bot (May 26 2023 at 00:22):pchickey opened PR #6463 from bytecodealliance:pch/wasi_read_write_rights to bytecodealliance:main:
Based on #6390
This is an upstreaming of #6462
https://github.com/bytecodealliance/wasmtime/pull/6265 introduced a regression with programs using wasi-libc, reported at https://github.com/WebAssembly/wasi-libc/issues/415.
Wasi-libc read the rights of the base directory (using fd_fdstat_get) and used those to mask the rights requested to path_open. In 6265, I changed the behavior of fdstat_get to always report and empty set of rights. This means that Wasi-libc will always pass an empty set of rights to path_open, which is a problem because the FD_READ and FD_WRITE rights are how path_open determines if a descriptor is to be opened for reading, writing, or both.
The fix is as follows:
directories always return the full set of rights in fd_fdstat_get.
we record the access mode that a file is opened with, and use that to set the FD_READ and FD_WRITE bits in fs_rights_base for a file's fd_fdstat_get.A test demonstrates the behavior of the fdstat rights bits, and that opening for reading, writing, or reading and writing behaves correctly when calling fd_read and fd_write
- This PR is just for the release-9.0.0 branch, I will work on upstreaming it to main but the situation there is slightly more complex because the test also needs to pass under the preview 2 implementation.
Preview 2's wit definition was unintentionally (per @sunfishcode) missing a way for
filesystem.{read,write,append}-via-streamto return anerror-codewhen a descriptor cannot be used to create such a stream. I added this to the wit definition in order to make the preview 2 behavior match the legacy implementation.
Wasmtime GitHub notifications bot (May 26 2023 at 00:22):pchickey edited PR #6463:
Based on #6390
This is an upstreaming of #6462
https://github.com/bytecodealliance/wasmtime/pull/6265 introduced a regression with programs using wasi-libc, reported at https://github.com/WebAssembly/wasi-libc/issues/415.
Wasi-libc read the rights of the base directory (using fd_fdstat_get) and used those to mask the rights requested to path_open. In 6265, I changed the behavior of fdstat_get to always report and empty set of rights. This means that Wasi-libc will always pass an empty set of rights to path_open, which is a problem because the FD_READ and FD_WRITE rights are how path_open determines if a descriptor is to be opened for reading, writing, or both.
The fix is as follows:
- directories always return the full set of rights in fd_fdstat_get.
- legacy implementation records the access mode that a file is opened with, and use that to set the FD_READ and FD_WRITE bits in fs_rights_base for a file's fd_fdstat_get. The preview 2 implementation already did this.
- A test demonstrates the behavior of the fdstat rights bits, and that opening for reading, writing, or reading and writing behaves correctly when calling fd_read and fd_write
Preview 2's wit definition was unintentionally (per @sunfishcode) missing a way for
filesystem.{read,write,append}-via-streamto return anerror-codewhen a descriptor cannot be used to create such a stream. I added this to the wit definition in order to make the preview 2 behavior match the legacy implementation.
Wasmtime GitHub notifications bot (May 26 2023 at 00:25):pchickey edited PR #6463:
Based on #6390 because I needed to edit wits - will land after that merges.
This is an upstreaming of #6462
https://github.com/bytecodealliance/wasmtime/pull/6265 introduced a regression with programs using wasi-libc, reported at https://github.com/WebAssembly/wasi-libc/issues/415.
Wasi-libc read the rights of the base directory (using fd_fdstat_get) and used those to mask the rights requested to path_open. In 6265, I changed the behavior of fdstat_get to always report and empty set of rights. This means that Wasi-libc will always pass an empty set of rights to path_open, which is a problem because the FD_READ and FD_WRITE rights are how path_open determines if a descriptor is to be opened for reading, writing, or both.
The fix is as follows:
- directories always return the full set of rights in fd_fdstat_get.
- legacy implementation records the access mode that a file is opened with, and use that to set the FD_READ and FD_WRITE bits in fs_rights_base for a file's fd_fdstat_get. The preview 2 implementation already did this.
- A test demonstrates the behavior of the fdstat rights bits, and that opening for reading, writing, or reading and writing behaves correctly when calling fd_read and fd_write
Preview 2's wit definition was unintentionally (per @sunfishcode) missing a way for
filesystem.{read,write,append}-via-streamto return anerror-codewhen a descriptor cannot be used to create such a stream. I added this to the wit definition in order to make the preview 2 behavior match the legacy implementation.
Wasmtime GitHub notifications bot (May 26 2023 at 17:47):pchickey updated PR #6463.
Wasmtime GitHub notifications bot (May 26 2023 at 17:50):pchickey edited PR #6463:
This is an upstreaming of #6462
https://github.com/bytecodealliance/wasmtime/pull/6265 introduced a regression with programs using wasi-libc, reported at https://github.com/WebAssembly/wasi-libc/issues/415.
Wasi-libc read the rights of the base directory (using fd_fdstat_get) and used those to mask the rights requested to path_open. In 6265, I changed the behavior of fdstat_get to always report and empty set of rights. This means that Wasi-libc will always pass an empty set of rights to path_open, which is a problem because the FD_READ and FD_WRITE rights are how path_open determines if a descriptor is to be opened for reading, writing, or both.
The fix is as follows:
- directories always return the full set of rights in fd_fdstat_get.
- legacy implementation records the access mode that a file is opened with, and use that to set the FD_READ and FD_WRITE bits in fs_rights_base for a file's fd_fdstat_get. The preview 2 implementation already did this.
- A test demonstrates the behavior of the fdstat rights bits, and that opening for reading, writing, or reading and writing behaves correctly when calling fd_read and fd_write
Preview 2's wit definition was unintentionally (per @sunfishcode) missing a way for
filesystem.{read,write,append}-via-streamto return anerror-codewhen a descriptor cannot be used to create such a stream. I added this to the wit definition in order to make the preview 2 behavior match the legacy implementation.
Wasmtime GitHub notifications bot (May 26 2023 at 17:50):pchickey has marked PR #6463 as ready for review.
Wasmtime GitHub notifications bot (May 26 2023 at 17:50):pchickey requested itsrainy for a review on PR #6463.
Wasmtime GitHub notifications bot (May 26 2023 at 17:50):pchickey requested wasmtime-core-reviewers for a review on PR #6463.
Wasmtime GitHub notifications bot (May 26 2023 at 17:54):pchickey requested wasmtime-default-reviewers for a review on PR #6463.
Wasmtime GitHub notifications bot (May 26 2023 at 17:54):pchickey updated PR #6463.
Wasmtime GitHub notifications bot (May 26 2023 at 17:56):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (May 26 2023 at 17:58):pchickey has enabled auto merge for PR #6463.
Wasmtime GitHub notifications bot (May 26 2023 at 20:32):pchickey updated PR #6463.
Wasmtime GitHub notifications bot (May 26 2023 at 20:33):pchickey updated PR #6463.
Wasmtime GitHub notifications bot (May 26 2023 at 20:33):pchickey updated PR #6463.
Wasmtime GitHub notifications bot (May 30 2023 at 20:23):pchickey updated PR #6463.
Wasmtime GitHub notifications bot (May 30 2023 at 21:08):pchickey updated PR #6463.
Wasmtime GitHub notifications bot (May 30 2023 at 22:21):pchickey updated PR #6463.
Wasmtime GitHub notifications bot (May 30 2023 at 22:24):pchickey updated PR #6463.
Wasmtime GitHub notifications bot (May 30 2023 at 22:35):pchickey requested sunfishcode for a review on PR #6463.
Wasmtime GitHub notifications bot (May 30 2023 at 22:49):sunfishcode submitted PR review.
Wasmtime GitHub notifications bot (May 30 2023 at 23:34):pchickey merged PR #6463.
Wasmtime GitHub notifications bot (Jul 23 2024 at 19:32):squeek502 commented on PR #6463:
The bug that this fixed has regressed in certain ways. Reproduction here: https://github.com/WebAssembly/wasi-libc/issues/415#issuecomment-2244254211
Last updated: Oct 23 2024 at 20:03 UTC