Stream: git-wasmtime

Topic: wasmtime / PR #5063 fix `validate_atomic_addr()`


view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2022 at 14:16):

haraldh opened PR #5063 from fix_validate_atomic_addr to main:

Apparently addr is a pointer to real memory.

Signed-off-by: Harald Hoyer <harald@profian.com>

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2022 at 14:48):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2022 at 14:48):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2022 at 14:48):

abrown created PR review comment:

checked_add?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 08:39):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 08:39):

bjorn3 created PR review comment:

Does this also need to account for access sizes larger than 1? For example a 8 byte access where the address is the last byte of the memory should not be allowed.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 09:29):

12101111 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 09:29):

12101111 created PR review comment:

Misaligned access will trap. This check is done in JIT code: https://github.com/bytecodealliance/wasmtime/blob/32a7593c949a17f20c2a718e1cbf0d8c449b0c47/cranelift/wasm/src/code_translator.rs#L1070-L1071

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 09:38):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 09:38):

bjorn3 created PR review comment:

That would necessitate all users of this function to trap on misalignment. Currently all callers of this function seem to immediately be followed by an unimplemented trap. Once they are implemented it is easy to miss this requirement as misaligned atomic accesses in rust are not guaranteed to panic.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:00):

haraldh updated PR #5063 from fix_validate_atomic_addr to main.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:02):

haraldh requested abrown for a review on PR #5063.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:02):

haraldh requested 12101111 for a review on PR #5063.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:02):

haraldh requested bjorn3 for a review on PR #5063.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:02):

haraldh requested abrown for a review on PR #5063.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:04):

12101111 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:04):

12101111 created PR review comment:

            .checked_add(mem.current_length())

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:07):

haraldh submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:07):

haraldh created PR review comment:

fixed

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:07):

haraldh updated PR #5063 from fix_validate_atomic_addr to main.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:07):

haraldh requested 12101111 for a review on PR #5063.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:07):

haraldh requested bjorn3 for a review on PR #5063.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 13:08):

haraldh requested 12101111 for a review on PR #5063.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 18 2022 at 16:52):

12101111 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 11 2022 at 11:03):

haraldh closed without merge PR #5063.


Last updated: Nov 22 2024 at 17:03 UTC