Skip to content

Fix filesystem streams and no-follow symlink checks - #2448

Open
landonxjames wants to merge 2 commits into
bytecodealliance:mainfrom
landonxjames:wasm32-wasip3-fixes
Open

landonxjames wants to merge 2 commits into
bytecodealliance:mainfrom
landonxjames:wasm32-wasip3-fixes

Conversation

@landonxjames

Copy link
Copy Markdown
Contributor

Part of the fixes for #2447

This fixes the wasm32-wasip3 filesystem problems 2, 3, 4 and 6 from #2447, and the same stream-drop problem in read-directory. Problem 1 was already fixed in #2437. Problem 5 is not addressed here, because since #2412 exit doesn't drain output.

Dropping a descriptor during a stream operation (problem 2)

Descriptor[Symbol.dispose] closed the descriptor's FileHandle while a write-via-stream, append-via-stream or read-via-stream operation could still be running in the filesystem worker with the raw fd. The remaining writes then failed with an EBADF error, or used a file that reused the fd number. Rust's std::fs::write drops the descriptor this way once the stream has accepted the last bytes, which left files short.

The new SharedFileHandle owns the FileHandle. Stream operations hold it while they run, and closing it waits until the last one settles. This applies to explicit disposes and the finalizer. Under Wasmtime, the stream similarly holds its own reference to the file.

Concurrent reads sharing one buffer (problem 3)

The worker read every read-via-stream chunk into one module-level buffer. A written chunk is only copied to the reader's thread when the reader pulls it, so another read in flight could overwrite it first, and the guest received bytes from the wrong file or offset. Each chunk now gets its own buffer.

Reads that outlive their stream (problem 6 and read-directory)

read-via-stream and read-directory created their StreamReader with the default preventCancel: true. A guest that dropped the stream early left the worker blocked on its next write, and the future didn't settle. With the change for problem 2 this would also have kept the file open. Both now pass { preventCancel: false } (like tcp-socket.receive) and the worker ends the operation when the reader cancels. Then the future resolves successfully, matching Wasmtime's Ok(()) when the reader drops the stream.

No-follow path operations on a symlink that leaves the directory (problem 4)

#ensureSandboxedPath resolved a symlink in the final path component even for operations that do not follow it. On a link such as sub/loop -> .., stat-at and metadata-hash-at without symlink-follow, readlink-at and unlink-file-at failed with not-permitted. Rust's std::fs::read_dir hits this through metadata-hash-at.

#ensureSandboxedPath now takes a required followFinal argument, and each caller states whether the operation follows the final component. Intermediate components are always checked. open-at, and the source path of link-at, still check the final component, because Windows has no O_NOFOLLOW and link follows symlinks on macOS.

Two related behaviour changes:

  • readlink-at rejects an absolute target with not-permitted like cap-std. So the relaxed check does not expose host paths. Previously an absolute target inside the directory was returned as the absolute host path
  • set-times-at without symlink-follow keeps the symlink's timestamp for no-change, instead of copying the target's

Test

test/filesystem.js adds 16 tests:

  • writing, appending and reading a 4 MiB file after its descriptor is dropped, checking the full contents
  • four concurrent readViaStream reads of different files
  • dropping readViaStream and readDirectory streams after one chunk or entry, both through the async iterator's return(), as generated bindings do, and through StreamReader.close()
  • no-follow operations on sub/loop -> .., an absolute readlink-at target, and operations that follow the link or pass through it, which must still be rejected

Keep a file open until stream operations started from its descriptor finish

Give each read-via-stream chunk its own buffer so concurrent reads cannot corrupt each other

End read-via-stream and read-directory in the worker when the guest drops the stream

Skip the sandbox check on a final symlink for path operations that do not follow it

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant