enarx-archive / sallyport

API for the hypervisor-microkernel boundary
Apache License 2.0
7 stars 6 forks source link

Move constants and required types from 0.1, make `NULL` public #83

Closed rvolosatovs closed 2 years ago

rvolosatovs commented 2 years ago

These are (or should be) used by enarx. Haven't checked if everything is actually used. https://github.com/enarx/enarx is already using 2021 Required for https://github.com/enarx/enarx/issues/1062

haraldh commented 2 years ago

Regarding the removal of MAX_UDP_PACKET_SIZE.

Don't we need the wanted size of the block somehow?

impl Block {
    /// Returns the capacity of `Block.buf`
    pub const fn buf_capacity() -> usize {
        // At least MAX_UDP_PACKET_SIZE rounded up Page::size() alignment
        (MAX_UDP_PACKET_SIZE + size_of::<Message>() + Page::SIZE - 1) / Page::SIZE * Page::SIZE
            - size_of::<Message>()
    }
}

sgx src/backend/sgx/thread.rs

pub struct Thread {
[…]
    block: Block,
[…]
}

sev is somehow hardcoded in internal/shim-sev/layout.ld

_ENARX_SALLYPORT_BLOCK_SIZE = 69632;

but later dynamically used with kvm_builder_map() in src/backend/kvm/builder.rs using size_of::<Block>()

rvolosatovs commented 2 years ago

I didn't find a usage in https://github.com/enarx/enarx https://github.com/enarx/enarx/search?q=MAX_UDP_PACKET_SIZE, and hence removed it. The sallyport crate is designed to work with blocks of arbitrary size, and I considered this to be a configuration parameter, value of which is specific to the use case of the consumer of the crate.

If we were to include this constant in the crate, I feel like it would make the most sense to then also provide a Block type, which would have a From implemented for an arbitrary byte buffer and would validate that the size of that buffer is at least MAX_UDP_PACKET_SIZE, although there is nothing within the sallyport crate implementation that actually requires this. Note, that this would also slightly complicate testing, because now we would have to first fill the start of the block with something to test for exceeding the block size.

So yeah, I feel like block size is something that should be tightly coupled to the shim implementations, and hence, reside there, but I don't mind including it here either, even if just an exposed constant not actually used by the crate.

Let me know, which approach you think is the most applicable here.

rvolosatovs commented 2 years ago

How about we provide an API to calculate a "size hint" of the block, given the size of contents you'd want to send? Then we could also provide the UDP constant and consumers of the crate could do something like block::size_hint(MAX_UDP_PACKET_SIZE), which would return the length of the block required to fit at least one call of biggest size (i.e. a syscall at the moment of writing, which has 7 args) with an allocated MAX_UDP_PACKET_SIZE bytes of payload. It would also account for the gettime smuggling once we implement it enarx/enarx#1727 It could also even take the amount of items as a parameter.

It's a "hint", because it does not account for alignment of the payload.

e.g.

const fn size_hint(items: usize, alloc_bytes: usize) -> usize