You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Dec 20, 2023. It is now read-only.
The guest_allocator::free public function has no way to validate its ptr argument and hence should be declared unsafe instead of claiming to be safe and using an unsafe block. Callers of the function, e.g. DNS::query_raw must then assure that the pointer passed is valid and was created by guest_allocator::default_malloc_impl (or at least Box::into_raw) using an unsafe block as they actually have that knowledge as the pointer was returned by a previous host call.
(The same seems to apply to guest_allocator::default_free_impl which should probably be unsafe extern "C" fn but since this is called only from the host runtime, there should be no visible ramifications of this except for the declaration of hostcall_init_mm.)
I am also curious why Vec::into_boxed_slice and Box::into_raw were used instead of std::alloc::alloc? The only difference in effect seems to be that the current approach makes the std::alloc::Layout used, implicitly that of Vec<u8>, i.e. using a alignment of std::mem::align_of::<u8>(), instead of enforcing a fixed minimal alignment suitable for all primitive types as e.g. a malloc implementation on the host would. If the host runtime assumes alignment of returned memory to have suitable alignment for all primitive types like a libc malloc would, then this might even introduce crashes on architectures where unaligned memory access is not allowed.
The text was updated successfully, but these errors were encountered:
I'm not sure why we're not using the std::alloc::alloc calls - personally I was not aware of them until now.
They are what is used in the implementation of Vec (or rather RawVec). In this particular case, it could also improve performance as vec![0u8; size] might have a capacity strictly larger than size (as it is optimizied for armortized cost extension) which needs to be shed by Vec::into_boxed_slice (re)allocating the buffer twice in effect.
We haven't been pushing new changes to Terrarium for a bit because we've been too busy working on other stuff, but we'll try to get this fixed soon.
I think issues like this do not invalidate the approach at all and are easily fixed (as the host calls will come with unsafe blocks in any case). Personally, I am looking forward to seeing where the highly insightful Terrarium project is going.
Yeah, I think the reason is far more mundane: All the alloc stuff got stabilized very soon before we got rolling on Terrarium, so we probably just weren't aware it was available yet.
I really appreciate you reaching out with such insightful comments ❤️
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The
guest_allocator::free
public function has no way to validate itsptr
argument and hence should be declared unsafe instead of claiming to be safe and using an unsafe block. Callers of the function, e.g.DNS::query_raw
must then assure that the pointer passed is valid and was created byguest_allocator::default_malloc_impl
(or at leastBox::into_raw
) using an unsafe block as they actually have that knowledge as the pointer was returned by a previous host call.(The same seems to apply to
guest_allocator::default_free_impl
which should probably beunsafe extern "C" fn
but since this is called only from the host runtime, there should be no visible ramifications of this except for the declaration ofhostcall_init_mm
.)I am also curious why
Vec::into_boxed_slice
andBox::into_raw
were used instead ofstd::alloc::alloc
? The only difference in effect seems to be that the current approach makes thestd::alloc::Layout
used, implicitly that ofVec<u8>
, i.e. using a alignment ofstd::mem::align_of::<u8>()
, instead of enforcing a fixed minimal alignment suitable for all primitive types as e.g. amalloc
implementation on the host would. If the host runtime assumes alignment of returned memory to have suitable alignment for all primitive types like a libcmalloc
would, then this might even introduce crashes on architectures where unaligned memory access is not allowed.The text was updated successfully, but these errors were encountered: