Panic on zkVM argv and env overflows#158018
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
| arg_len.checked_next_multiple_of(WORD_SIZE).expect("argument length overflowed"); | ||
| assert!(arg_len_rounded <= isize::MAX as usize, "argument length is too large"); | ||
| let arg_len_words = arg_len_rounded / WORD_SIZE; | ||
| let words = unsafe { abi::sys_alloc_words(arg_len_words) }; |
There was a problem hiding this comment.
sys_alloc_words appears to be marked deprecated upstream; the replacement (I assume) is https://docs.rs/risc0-zkvm-platform/latest/risc0_zkvm_platform/syscall/fn.sys_alloc_aligned.html. It's suspiciously documented as safe, which seems a bit odd to me.
It seems like zkvm supports a global allocator based on the bare Vec::with_capacity above, is there a reason we're not using that here? That would avoid needing to reference the allocation primitives directly and Layout will take take of the isize restriction (if you use it in safe code).
|
Reminder, once the PR becomes ready for a review, use |
Fixes #158016
I'm not sure if the trade-of is worth, waiting for other people opinions on that. My take was that because this overflow is directly impacted from the host it should be avoided at all cost since this overflow can ultimately lead to
slice::from_raw_parts(ptr, len)with the huge length