Openenclave: Update internal EDL to use deep copy

Created on 19 Nov 2019  路  3Comments  路  Source: openenclave/openenclave

We're considering internal use of deep copy as the quality bar for which to make it non-experimental.

Here is the list of APIs that could benefit:

These could be collapsed into a single oe_syscall_getaddrinfo() if deep copy were used.

  • oe_syscall_getaddrinfo_open_ocall()
  • oe_syscall_getaddrinfo_read_ocall()
  • oe_syscall_getaddrinfo_close_ocall()

These currently flatten the iovec struct today and could benefit.

  • oe_syscall_readv_ocall()
  • oe_syscall_writev_ocall()
  • oe_syscall_recvv_ocall()
  • oe_syscall_sendv_ocall()

These unwrap the msghdr struct today and could benefit.

  • oe_syscall_sendmsg_ocall()
  • oe_syscall_recvmsg_ocall()

/cc @radhikaj @anakrish @mikbras

core triaged

All 3 comments

Discussed with @andschwa a little offline, capturing the essence of that discussion here. To me deep copy means a few things, some use cases are simpler to implement than others.

  1. A struct with a buffer that we would like deep copied into the enclave where we know the size of the buffer. The buffer can be annotate with the known length, like the following:
struct local_iovec
{
    [size=iov_len]
    void* iov_base;
    size_t iov_len;
};

In this case, edger8r can properly copy all of iov_base into enclave memory.

  1. A linked list with a known length. This would benefit something like the readv syscall:

ssize_t readv(int fd, const struct iovec *iov, int iovcnt);

iov is a linked list, and iovcnt tells the number of elements in that list. iov can be annotated with iovcnt
and edger8r can copy each element into enclave memory. In this case, edger8r knows the total memory needed to do the copy. This would benefit readv, writev, recv, sendv, and (with some extra annotation) sendmsg and recmsg.

Note, this would likely require a new annotation added to EDL.

  1. A linked list with an unknown length. This is the hard one and would benefit calls like getaddrinfo. getaddrinfo gives back an addrinfo struct:
struct addrinfo {
   int              ai_flags;
   int              ai_family;
   int              ai_socktype;
   int              ai_protocol;
   socklen_t        ai_addrlen;
   struct sockaddr *ai_addr;
   char            *ai_canonname;
   struct addrinfo *ai_next;
};

This is also a linked list, where ai_next is the head of the list. In this case, the length of the list is not known. It is just null-terminated, so you need to enumerate the list to know how many elements there are. This means edger8r can't know how large the list is. It is possible for the marshalling code to just allocate a large buffer and lay the list into that memory, but there are issues with that approach.

The addrinfo struct also highlights another case with ai_cannonname. Deep copying null-terminated strings gives us the same problem as "linked list with unknown length.

We should decide what we want to support and what we don't.

cc @radhikaj and @mikbras to discuss what exactly we want to support here. For the most part I would say this issue is a much bigger item than initially thought.

At this time, we cannot act on this issue since deep copy does not support linked lists.
At this time, find any APIs that would benefit from deep copy and fix those.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

andschwa picture andschwa  路  9Comments

dthaler picture dthaler  路  5Comments

wintersteiger picture wintersteiger  路  7Comments

Lusitanian picture Lusitanian  路  8Comments

radhikaj picture radhikaj  路  7Comments