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

aborted modules must be poisoned #3687

Open
workingjubilee opened this issue Nov 4, 2023 · 5 comments
Open

aborted modules must be poisoned #3687

workingjubilee opened this issue Nov 4, 2023 · 5 comments
Labels

Comments

@workingjubilee
Copy link

workingjubilee commented Nov 4, 2023

Using wasm-bindgen, a wasm module can be resumed after abort by calling into it again from JavaScript. This violates Rust's soundness preconditions: abort must terminate forward progress.

The wasm component model prevents wasm modules from being reused in this way by locking out calls into them after a module has trapped. However, wasm bindgen runs on wasm itself, which lacks this safeguard.

It is likely that wasm-bindgen should be implementing the check that poisons calls into it, or shuts off access in any other way. Even if this were implemented upstream for all users of wasm32-unknown-unknown, it would be best for wasm-bindgen to deprecate any interfaces that serve primarily to expose this route to unsound interactions to userspace. The ability to throw JavaScript exceptions has been called out as a likely example.

Also see:

@daxpedda
Copy link
Collaborator

daxpedda commented Nov 9, 2023

I left some additional questions/comments in the Zulip discussions, but to summarize: I'm not entirely convinced that this is a problem in wasm-bindgen because throwing a JS exception doesn't call abort().

@RalfJung
Copy link

RalfJung commented Nov 9, 2023

What is the type of throwing a JS exception in Rust? AFAIK wasm is a no-unwind target. So every function that returns ! is like abort -- no code must ever be executed in this thread any more.

@daxpedda
Copy link
Collaborator

daxpedda commented Nov 9, 2023

So every function that returns ! is like abort -- no code must ever be executed in this thread any more.

It is indeed !. Is this really required by Rust? It is hard to find any documentation around this.
As far as I can tell this isn't practically unsafe: even though no-unwind Wasm throws, it doesn't clear the stack where it throws. Obviously this can and will cause issues, but it shouldn't be unsafe or cause UB.

If this is just a "semantic" issue, I would like to propose to change that in Rust: not require ! to be an abort(), or at least on some platforms, e.g. Wasm.

@RalfJung
Copy link

The concern is around optimizations Rust performs that need this rule, so it's not something we can fix by just changing some wording somewhere.

I'll need to better understand what this function does and doesn't do... once I have time I'll catch up on Zulip and have some questions there.

@daxpedda
Copy link
Collaborator

So conclusion:
This is definitely unsafe and Rust should implement some sort of module poisoning when we abort.

From wasm-bindgens side, we could definitely just deprecate any throwing function, though I'm not sure if this does make sense because it's definitely a widely used and probably necessary future, so alternatively we might want to add poisoning to wasm-bindgen as well or when we add poisoning to Rust we should add it in a way that can cover issues like that.

I would prefer to wait for now, I will definitely find time to make some experiments in Rust and see how we can implement poisoning there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants