Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
GH-130397: remove special-casing of C stack depth for WASI#134469
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Removed special-casing for WASI when setting C stack depth limits. Since WASI has its own C stack checking this isn't a security risk.Also disabled some tests that stopped passing. They all happened to have already been disabled under Emscripten.
ad42dc1 intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thanks@brettcannon for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…honGH-134469)Removed special-casing for WASI when setting C stack depth limits. Since WASI has its own C stack checking this isn't a security risk.Also disabled some tests that stopped passing. They all happened to have already been disabled under Emscripten.(cherry picked from commitad42dc1)Co-authored-by: Brett Cannon <brett@python.org>
GH-134547 is a backport of this pull request to the3.14 branch. |
…-134469) (GH-134547)GH-130397: remove special-casing of C stack depth for WASI (GH-134469)Removed special-casing for WASI when setting C stack depth limits. Since WASI has its own C stack checking this isn't a security risk.Also disabled some tests that stopped passing. They all happened to have already been disabled under Emscripten.(cherry picked from commitad42dc1)Co-authored-by: Brett Cannon <brett@python.org>
One of us is misunderstanding the way stacks work in C compiled to webassembly. As I understand it, there are two stacks: the built in webassembly stack which does have built in protected and and additional stack for arrays and other objects that can have their address taken on the stack. This second stack has no built in protection. Removing the protection we provide makes it possible to trash the heap and could perhaps introduce security vulnerabilities. @hoodmane is this correct? |
What I think we should be doing is reducing the size of the webassembly stack and increasing the size of the in-heap stack, such that the majority of stack overflows are caught by our stack protection mechanism and reported as an exception. |
Quite possibly (and it very well could be me).
Maybe, but I also need to be able to parse Python code in WASI in a debug build and this was the only way I could find how without sinking days into trying to make this all work (assuming it would as I already spent days up to this point).
If you can explain how to make that happen or make a PR for that then that would be great. But I tried tweaking all of the numbers I had available to me and it never seemed to turn out to a result that didn't cause some critical test failure because the stack got too small. |
Uh oh!
There was an error while loading.Please reload this page.
Along the way disable some tests which now fail (which were already disabled under Emscripten).
This doesn't pose any sercurity risk thanks to WebAssembly's stack protection.