Open thewtex opened 1 year ago
This came up in a migration from wasmer to wasmtime: https://github.com/InsightSoftwareConsortium/itk-wasm/pull/773
To some extent this is a difference between how Unix and Windows work. In some cases though, and perhaps this one, it's possible to paper over the differences. My guess is that the WASI support for preopening files holds open handles and the open handles are preventing deletion. This can probably be fixed by either:
Is the issue here that the embedding is trying to remove the temporary directory before destroying the WasiCtx which is holding a handle to the temporary directory?
There may be flags to open directories with which allow concurrent deletion. I think files have this flag and this may need to be a patch to Wasmtime's WASI support.
Agreed, I think this would be ideal.
embedding is trying to remove the temporary directory before destroying the WasiCtx which is holding a handle to the temporary directory?
We have a situation where something outside the embedding tries to remove the directory before the WasiCtx is destroyed.
Currently, Wasmtime deliberately sets the flags that prevent concurrent deletion, on the interpretation that preopens are granted to a WASI context in the form of handles, rather than paths. Wasmtime thinks the job of the engine is to ensure that the guest has access to what's referenced by the handles, not to other things that might occupy the same path over time if the original contents are deleted.
For example, if the directory is created by something like Python's tempfile
, the generated names are not guaranteed to be collision-proof, so if something outside the embedding removes the directory before the WasiCtx
is destroyed, and we allow guest code to continue to access the referent of the original path, and some future tempfile
invocation happens to pick that same path, that could allow guest code to gain access to directory contents it shouldn't have access to.
I'm open to making changes here, but it's important for us to guard against this kind of unintended pathname reuse bug.
@sunfishcode, ah, I see your point on the need to not provided unintended access to a directory.
I do not know how wasmtime is implemented, but is it possible to retain the preopen identifier as a handle, so we do not provide access to a "different" directory, but not set the flags to prevent deletion outside of the wasmtime engine? Or something similar?
The main Windows APIs lack a way to do something analogous to a Unix openat
, taking a base handle, so currently the implementation on Windows concatenates the base directory path with the relative path and opens the result, to emulate openat
. I don't know of a way to make that robust without preventing concurrent renaming or deletion of the base.
The Windows kernel API does have a function called NtCreateFile
which does have the ability to take a base handle. We've begun porting some parts of our Windows implementation to use this. However, this is subtle work, and we'll have to find ways to make every function in the WASI filesystem API use this before we could stop setting the flags to prevent concurrent deletion, and I don't yet know if that's possible.
The preopen_dir WASI support seems to hold onto Windows directory resources, which prevents removal of the directory, etc.
This is demonstrated by #131 , which will result in: