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

[9.0.0] fix Wasi rights system to work with wasi-libc #6462

Merged
merged 5 commits into from
May 26, 2023

Conversation

pchickey
Copy link
Collaborator

@pchickey pchickey commented May 25, 2023

#6265 introduced a regression with programs using wasi-libc, reported at WebAssembly/wasi-libc#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.

Pat Hickey added 4 commits May 25, 2023 14:16
@github-actions
Copy link

Subscribe to Label Action

cc @kubkon

This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

  • kubkon: wasi

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@pchickey pchickey force-pushed the pch/release-9-wasi-rights-read-write branch from 4012e49 to 0d8a260 Compare May 26, 2023 00:23
@pchickey pchickey merged commit bf98dca into release-9.0.0 May 26, 2023
33 checks passed
@pchickey pchickey deleted the pch/release-9-wasi-rights-read-write branch May 26, 2023 16:50
pchickey pushed a commit that referenced this pull request May 26, 2023
required in order to make 9.0.2 a valid patch release.

* `struct FdStat` is public, so I backed out changes to it, and manually
applied the rights bits in the implementation of fd_fdstat_get.

* new `FileAccessMode` changed to be pub(crate), to make sure it doesnt
appear in any pub interfaces. WasiCtx::insert_file and push_file
reverted to their original type signature. Private helper insert_file_
used for the set_{stdio} implementations.
pchickey pushed a commit that referenced this pull request May 26, 2023
required in order to make 9.0.2 a valid patch release.

* `struct FdStat` is public, so I backed out changes to it, and manually
applied the rights bits in the implementation of fd_fdstat_get.

* new `FileAccessMode` changed to be pub(crate), to make sure it doesnt
appear in any pub interfaces. WasiCtx::insert_file and push_file
reverted to their original type signature. Private helper insert_file_
used for the set_{stdio} implementations.
pchickey pushed a commit that referenced this pull request May 26, 2023
* fixes to #6462: make wasi-common API compatible with 9.0.1

required in order to make 9.0.2 a valid patch release.

* `struct FdStat` is public, so I backed out changes to it, and manually
applied the rights bits in the implementation of fd_fdstat_get.

* new `FileAccessMode` changed to be pub(crate), to make sure it doesnt
appear in any pub interfaces. WasiCtx::insert_file and push_file
reverted to their original type signature. Private helper insert_file_
used for the set_{stdio} implementations.

* add release notes for 9.0.1 and 9.0.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants