cfallin opened PR #8541 from cfallin:oob-table-index-during-prescan
to bytecodealliance:main
:
…ng prescan.
Call-indirect caching requires a "prescan" of a module's code section during compilation in order to know which tables are possibly written, and to count call-indirect callsites so that each separate function compilation can enable caching if possible and use the right slot range.
This prescan is not integrated with the usual validation logic (nor should it be, probably, for performance), so it's possible that an out-of-bounds table index or other illegal instruction could be present. We previously indexed into an internal data structure (
table_plans
) with this index, allowing for a compilation panic on certain invalid modules before validation would have caught it. This PR fixes that with a one-off check at the single point that we interpret a parameter (the table index) from an instruction during the prescan.Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=68614.
No test case because this requires compilation with non-default parameters, so the usual WAST
assert_invalid
form can't be used in a WAST test, and requires an invalid module, so a compilation disas test can't be used. Could build out either if desired but that seems like a much larger change.
cfallin requested wasmtime-core-reviewers for a review on PR #8541.
cfallin requested alexcrichton for a review on PR #8541.
cfallin edited PR #8541.
cfallin edited PR #8541:
Call-indirect caching requires a "prescan" of a module's code section during compilation in order to know which tables are possibly written, and to count call-indirect callsites so that each separate function compilation can enable caching if possible and use the right slot range.
This prescan is not integrated with the usual validation logic (nor should it be, probably, for performance), so it's possible that an out-of-bounds table index or other illegal instruction could be present. We previously indexed into an internal data structure (
table_plans
) with this index, allowing for a compilation panic on certain invalid modules before validation would have caught it. This PR fixes that with a one-off check at the single point that we interpret a parameter (the table index) from an instruction during the prescan.Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=68614.
No test case because this requires compilation with non-default parameters, so the usual WAST
assert_invalid
form can't be used in a WAST test, and requires an invalid module, so a compilation disas test can't be used. Could build out either if desired but that seems like a much larger change.
alexcrichton submitted PR review:
Thanks!
Can you add a test similar to https://github.com/bytecodealliance/wasmtime/pull/8544? Basically just assert
Module::new
returns an error. I was thinking when writing that PR we should add support for custom CLI flags on the*.wast
test suite, but that can be added orthogonally to this.
cfallin updated PR #8541.
cfallin commented on PR #8541:
Ah, right, that's the straightforward and obvious way to test this -- thanks! Added.
cfallin has enabled auto merge for PR #8541.
cfallin merged PR #8541.
Last updated: Dec 23 2024 at 12:05 UTC