From 60a360e329d63ba648d8a6e0485249b6c75aa670 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Fri, 7 Aug 2020 11:15:52 +0000 Subject: [PATCH 01/15] Add outside-enclave checks on shared memory Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_oe.c | 63 ++++++++++++++++++++++++----- src/include/enclave/enclave_state.h | 2 + src/include/shared/shared_memory.h | 8 ++-- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index 6d3622eff..e1412394c 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -12,6 +12,7 @@ #include "enclave/enclave_util.h" #include "enclave/lthread.h" #include "enclave/lthread_int.h" +#include "lkl/virtio.h" #include "shared/env.h" #include "shared/timer_dev.h" @@ -306,30 +307,55 @@ static void _read_eeid_config() sgxlkl_enclave_state.config = cfg; } +#define CHECK_INSIDE(X, S) \ + { \ + if ((X) != NULL && !oe_is_within_enclave((X), (S))) \ + oe_abort(); \ + } + +#define CHECK_OUTSIDE(X, S) \ + { \ + if ((X) != NULL && !oe_is_outside_enclave((X), (S))) \ + oe_abort(); \ + } + static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) { const sgxlkl_enclave_config_t* cfg = sgxlkl_enclave_state.config; - /* Deep copy where necessary */ + /* The host shared memory struct has been copied by OE (but not its + * contents). */ + CHECK_INSIDE(host, sizeof(sgxlkl_shared_memory_t)); - sgxlkl_shared_memory_t* enc = &sgxlkl_enclave_state.shared_memory; - memset(enc, 0, sizeof(sgxlkl_shared_memory_t)); + /* Temporary, volatile copy to make sure checks aren't reordered */ + volatile sgxlkl_shared_memory_t* enc = oe_calloc_or_die( + 1, + sizeof(sgxlkl_shared_memory_t), + "Could not allocate memory for shared memory\n"); if (cfg->io.network) + { enc->virtio_net_dev_mem = host->virtio_net_dev_mem; + CHECK_OUTSIDE(enc->virtio_net_dev_mem, sizeof(struct virtio_dev)); + } if (cfg->io.console) + { enc->virtio_console_mem = host->virtio_console_mem; + CHECK_OUTSIDE(enc->virtio_console_mem, sizeof(struct virtio_dev)); + } enc->evt_channel_num = host->evt_channel_num; - /* enc_dev_config is required to be outside the enclave */ enc->enc_dev_config = host->enc_dev_config; + CHECK_OUTSIDE( + enc->enc_dev_config, sizeof(enc_dev_config_t) * enc->evt_channel_num); enc->virtio_swiotlb = host->virtio_swiotlb; enc->virtio_swiotlb_size = host->virtio_swiotlb_size; + CHECK_OUTSIDE(enc->virtio_swiotlb, enc->virtio_swiotlb_size); - /* timer_dev_mem is required to be outside the enclave */ enc->timer_dev_mem = host->timer_dev_mem; + CHECK_OUTSIDE(enc->timer_dev_mem, sizeof(struct timer_dev)); if (cfg->io.block) { @@ -346,8 +372,12 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) for (size_t i = 0; i < enc->num_virtio_blk_dev; i++) { enc->virtio_blk_dev_mem[i] = host->virtio_blk_dev_mem[i]; + CHECK_OUTSIDE( + enc->virtio_blk_dev_mem[i], sizeof(struct virtio_dev)); + const char* name = host->virtio_blk_dev_names[i]; size_t name_len = oe_strlen(name) + 1; + CHECK_OUTSIDE(name, name_len); enc->virtio_blk_dev_names[i] = oe_calloc_or_die( name_len, sizeof(char), @@ -356,20 +386,32 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) } } - if (host->env) + char* const* henv = host->env; + if (henv) { size_t henvc = 0; - while (host->env[henvc++] != 0) - ; + while (henv[henvc] != 0) + henvc++; + CHECK_OUTSIDE(henv, sizeof(char*) * henvc); char** tmp = oe_calloc_or_die( henvc + 1, sizeof(char*), "Could not allocate memory for host import environment variable\n"); for (size_t i = 0; i < henvc; i++) - tmp[i] = oe_strdup(host->env[i]); + { + const char* env_i = henv[i]; + size_t n = strlen(env_i) + 1; + CHECK_OUTSIDE(env_i, n); + tmp[i] = oe_malloc(n); + memcpy(tmp[i], env_i, n); + } tmp[henvc] = NULL; enc->env = tmp; } + + /* Commit to the temporary copy */ + sgxlkl_enclave_state.shared_memory = *enc; + oe_free((sgxlkl_shared_memory_t*)enc); } static void _free_shared_memory() @@ -389,9 +431,10 @@ static void _free_shared_memory() int sgxlkl_enclave_init(const sgxlkl_shared_memory_t* shared_memory) { - SGXLKL_ASSERT(shared_memory); + SGXLKL_ASSERT(shared_memory && !sgxlkl_enclave_state.initialized); memset(&sgxlkl_enclave_state, 0, sizeof(sgxlkl_enclave_state)); + sgxlkl_enclave_state.initialized = true; sgxlkl_enclave_state.libc_state = libc_not_started; #ifdef DEBUG diff --git a/src/include/enclave/enclave_state.h b/src/include/enclave/enclave_state.h index f5c60103c..15eaf1a2f 100644 --- a/src/include/enclave/enclave_state.h +++ b/src/include/enclave/enclave_state.h @@ -33,6 +33,8 @@ typedef struct typedef struct sgxlkl_enclave_state { + _Atomic(bool) initialized; + const sgxlkl_enclave_config_t* config; /* Flattened ELF64 process stack */ diff --git a/src/include/shared/shared_memory.h b/src/include/shared/shared_memory.h index 41cbee9cc..93674fb8f 100644 --- a/src/include/shared/shared_memory.h +++ b/src/include/shared/shared_memory.h @@ -5,11 +5,13 @@ #include +struct virtio_dev; + typedef struct sgxlkl_shared_memory { /* Shared memory for virtio implementation */ - void* virtio_net_dev_mem; /* Virtio network device */ - void* virtio_console_mem; /* Virtio console device */ + struct virtio_dev* virtio_net_dev_mem; /* Virtio network device */ + struct virtio_dev* virtio_console_mem; /* Virtio console device */ size_t evt_channel_num; /* Number of event channels */ enc_dev_config_t* enc_dev_config; /* Device configuration */ @@ -22,7 +24,7 @@ typedef struct sgxlkl_shared_memory /* Shared memory for virtio block devices */ size_t num_virtio_blk_dev; - void** virtio_blk_dev_mem; + struct virtio_dev** virtio_blk_dev_mem; char** virtio_blk_dev_names; /* Host environment variables for optional import */ From 44cc896d865f95364ea0468c5675380b17b362c4 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Fri, 7 Aug 2020 12:04:47 +0000 Subject: [PATCH 02/15] Add memory checks to virtio devices Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_event_channel.c | 15 +++++++++++++++ src/enclave/enclave_oe.c | 1 + src/enclave/enclave_timer.c | 7 ++++++- src/lkl/setup.c | 9 ++++----- src/lkl/virtio_blkdev.c | 27 +++++++++++++++++++++------ src/lkl/virtio_console.c | 3 +++ src/lkl/virtio_netdev.c | 6 ++++++ 7 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/enclave/enclave_event_channel.c b/src/enclave/enclave_event_channel.c index 875138741..54df2ad51 100644 --- a/src/enclave/enclave_event_channel.c +++ b/src/enclave/enclave_event_channel.c @@ -137,6 +137,10 @@ void initialize_enclave_event_channel( uint8_t* dev_id = NULL; _evt_channel_num = evt_channel_num; + if (!oe_is_outside_enclave( + enc_dev_config, sizeof(enc_dev_config_t) * _evt_channel_num)) + oe_abort(); + evt_chn_lock = (struct ticketlock**)oe_calloc_or_die( evt_channel_num, sizeof(struct ticketlock*), @@ -150,6 +154,17 @@ void initialize_enclave_event_channel( _enc_dev_config = enc_dev_config; for (int i = 0; i < evt_channel_num; i++) { + if (!oe_is_outside_enclave( + &enc_dev_config[i], sizeof(enc_dev_config_t)) || + !oe_is_outside_enclave( + &enc_dev_config[i].enc_evt_chn, sizeof(enc_evt_channel_t)) || + !oe_is_outside_enclave( + &enc_dev_config[i].enc_evt_chn->host_evt_channel, + sizeof(evt_t)) || + !oe_is_outside_enclave( + &enc_dev_config[i].enc_evt_chn->qidx_p, sizeof(uint32_t))) + oe_abort(); + evt_chn_lock[i] = (struct ticketlock*)oe_calloc_or_die( 1, sizeof(struct ticketlock), diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index e1412394c..393eb0f8a 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -386,6 +386,7 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) } } + /* Copy the host's environment variables to enclave memory */ char* const* henv = host->env; if (henv) { diff --git a/src/enclave/enclave_timer.c b/src/enclave/enclave_timer.c index 583e57365..cb9bcb2b9 100644 --- a/src/enclave/enclave_timer.c +++ b/src/enclave/enclave_timer.c @@ -28,7 +28,12 @@ _Atomic(uint64_t) internal_counter = 0; */ uint64_t enclave_nanos() { - uint64_t e = sgxlkl_enclave_state.shared_memory.timer_dev_mem->nanos; + struct timer_dev* t = sgxlkl_enclave_state.shared_memory.timer_dev_mem; + + if (!oe_is_outside_enclave(t, sizeof(struct timer_dev))) + oe_abort(); + + uint64_t e = t->nanos; uint64_t i = internal_counter; if (e > i) { diff --git a/src/lkl/setup.c b/src/lkl/setup.c index 1a77bc25c..d739bbdc0 100644 --- a/src/lkl/setup.c +++ b/src/lkl/setup.c @@ -1315,13 +1315,14 @@ static void init_enclave_clock() SGXLKL_VERBOSE("Setting enclave realtime clock\n"); - if (oe_is_within_enclave(shm->timer_dev_mem, sizeof(struct timer_dev))) + struct timer_dev* t = shm->timer_dev_mem; + + if (!oe_is_outside_enclave(t, sizeof(struct timer_dev))) { sgxlkl_fail( "timer_dev memory isn't outside of the enclave. Aborting.\n"); } - struct timer_dev* t = shm->timer_dev_mem; struct lkl_timespec start_time; start_time.tv_sec = t->init_walltime_sec; start_time.tv_nsec = t->init_walltime_nsec; @@ -1396,9 +1397,7 @@ void lkl_start_init() lkl_virtio_console_add(shm->virtio_console_mem); // Register network tap if given one - int net_dev_id = -1; - if (shm->virtio_net_dev_mem) - net_dev_id = lkl_virtio_netdev_add(shm->virtio_net_dev_mem); + int net_dev_id = lkl_virtio_netdev_add(shm->virtio_net_dev_mem); /* Prepare bootargs to boot lkl kernel */ char bootargs[BOOTARGS_LEN] = {0}; diff --git a/src/lkl/virtio_blkdev.c b/src/lkl/virtio_blkdev.c index c98ed0517..b3e2f9bb9 100644 --- a/src/lkl/virtio_blkdev.c +++ b/src/lkl/virtio_blkdev.c @@ -13,8 +13,14 @@ */ static void lkl_deliver_irq(uint8_t dev_id) { - struct virtio_dev* dev = - sgxlkl_enclave_state.shared_memory.virtio_blk_dev_mem[dev_id]; + struct virtio_dev** mems = + sgxlkl_enclave_state.shared_memory.virtio_blk_dev_mem; + + sgxlkl_ensure_outside(mems, sizeof(struct virtio_dev*)); + + struct virtio_dev* dev = mems[dev_id]; + + sgxlkl_ensure_outside(dev, sizeof(struct virtio_dev)); dev->int_status |= VIRTIO_MMIO_INT_VRING; @@ -29,9 +35,17 @@ int lkl_add_disks( const sgxlkl_enclave_mount_config_t* mounts, size_t num_mounts) { + struct virtio_dev** mems = + sgxlkl_enclave_state.shared_memory.virtio_blk_dev_mem; + + sgxlkl_ensure_outside(mems, sizeof(struct virtio_dev*)); + struct virtio_dev* root_dev = - sgxlkl_enclave_state.shared_memory.virtio_blk_dev_mem - [sgxlkl_enclave_state.disk_state[0].host_disk_index]; + mems[sgxlkl_enclave_state.disk_state[0].host_disk_index]; + + if (!oe_is_outside_enclave(root_dev, sizeof(struct virtio_dev))) + oe_abort(); + int mmio_size = VIRTIO_MMIO_CONFIG + root_dev->config_len; if (lkl_virtio_dev_setup(root_dev, mmio_size, lkl_deliver_irq) != 0) return -1; @@ -39,8 +53,9 @@ int lkl_add_disks( for (size_t i = 0; i < num_mounts; ++i) { struct virtio_dev* dev = - sgxlkl_enclave_state.shared_memory.virtio_blk_dev_mem - [sgxlkl_enclave_state.disk_state[i + 1].host_disk_index]; + mems[sgxlkl_enclave_state.disk_state[i + 1].host_disk_index]; + if (!oe_is_outside_enclave(dev, sizeof(struct virtio_dev))) + oe_abort(); int mmio_size = VIRTIO_MMIO_CONFIG + dev->config_len; if (lkl_virtio_dev_setup(dev, mmio_size, lkl_deliver_irq) != 0) return -1; diff --git a/src/lkl/virtio_console.c b/src/lkl/virtio_console.c index 92c2971f1..8d8269ed9 100644 --- a/src/lkl/virtio_console.c +++ b/src/lkl/virtio_console.c @@ -17,6 +17,7 @@ static void lkl_deliver_irq(uint64_t dev_id) struct virtio_dev* dev = sgxlkl_enclave_state.shared_memory.virtio_console_mem; + sgxlkl_ensure_outside(dev, sizeof(struct virtio_dev)); dev->int_status |= VIRTIO_MMIO_INT_VRING; lkl_trigger_irq(dev->irq); @@ -29,6 +30,8 @@ int lkl_virtio_console_add(struct virtio_dev* console) { int ret = -1; + sgxlkl_ensure_outside(console, sizeof(struct virtio_dev)); + int mmio_size = VIRTIO_MMIO_CONFIG + console->config_len; ret = lkl_virtio_dev_setup(console, mmio_size, &lkl_deliver_irq); diff --git a/src/lkl/virtio_netdev.c b/src/lkl/virtio_netdev.c index 1222300fa..92a7d91ea 100644 --- a/src/lkl/virtio_netdev.c +++ b/src/lkl/virtio_netdev.c @@ -65,6 +65,12 @@ static void lkl_deliver_irq(uint64_t dev_id) int lkl_virtio_netdev_add(struct virtio_dev* netdev) { int ret = -1; + + if (!netdev) + return -1; + + sgxlkl_ensure_outside(netdev, sizeof(struct virtio_dev)); + int mmio_size = VIRTIO_MMIO_CONFIG + netdev->config_len; registered_devs[registered_dev_idx] = netdev; From 8841fa8715fab1dccb2a5097872197ed6af711cf Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Fri, 7 Aug 2020 12:15:40 +0000 Subject: [PATCH 03/15] Fix memory check in setup.c Signed-off-by: Christoph M. Wintersteiger --- src/lkl/setup.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lkl/setup.c b/src/lkl/setup.c index d739bbdc0..186a32896 100644 --- a/src/lkl/setup.c +++ b/src/lkl/setup.c @@ -1,3 +1,5 @@ +/* clang-format off */ + #include #include #include @@ -1461,7 +1463,7 @@ void lkl_start_init() if (sgxlkl_enclave_state.config->swiotlb) { /* validate bounce buffer memory before setting it up */ - if (!oe_is_within_enclave( + if (oe_is_outside_enclave( shm->virtio_swiotlb, shm->virtio_swiotlb_size)) { lkl_initialize_swiotlb( From cd458c501816f314c44c323490bae5b7a869e2e2 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Fri, 7 Aug 2020 14:28:20 +0000 Subject: [PATCH 04/15] Fix memory location checks Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_oe.c | 2 +- src/lkl/virtio_blkdev.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index 393eb0f8a..89ae0f7c0 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -363,7 +363,7 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) enc->virtio_blk_dev_mem = oe_calloc_or_die( enc->num_virtio_blk_dev, - sizeof(void*), + sizeof(struct virtio_dev*), "Could not allocate memory for virtio block devices\n"); enc->virtio_blk_dev_names = oe_calloc_or_die( enc->num_virtio_blk_dev, diff --git a/src/lkl/virtio_blkdev.c b/src/lkl/virtio_blkdev.c index b3e2f9bb9..1d370377d 100644 --- a/src/lkl/virtio_blkdev.c +++ b/src/lkl/virtio_blkdev.c @@ -16,7 +16,7 @@ static void lkl_deliver_irq(uint8_t dev_id) struct virtio_dev** mems = sgxlkl_enclave_state.shared_memory.virtio_blk_dev_mem; - sgxlkl_ensure_outside(mems, sizeof(struct virtio_dev*)); + sgxlkl_ensure_inside(mems, sizeof(struct virtio_dev*)); struct virtio_dev* dev = mems[dev_id]; @@ -38,7 +38,7 @@ int lkl_add_disks( struct virtio_dev** mems = sgxlkl_enclave_state.shared_memory.virtio_blk_dev_mem; - sgxlkl_ensure_outside(mems, sizeof(struct virtio_dev*)); + sgxlkl_ensure_inside(mems, sizeof(struct virtio_dev*)); struct virtio_dev* root_dev = mems[sgxlkl_enclave_state.disk_state[0].host_disk_index]; From bbeb7dee6c46355d7a92d2fe15ff5887d01117a9 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Fri, 7 Aug 2020 17:26:10 +0000 Subject: [PATCH 05/15] Fix host env import Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_oe.c | 33 +++++++++++++--------------- src/include/shared/shared_memory.h | 1 + src/main-oe/sgxlkl_run_oe.c | 3 +++ tests/basic/eeid-config/Makefile | 2 +- tests/basic/eeid-config/hello-eeid.c | 11 ++++++++++ 5 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index 89ae0f7c0..b7682efb9 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -108,7 +108,8 @@ static void _prepare_elf_stack() size_t num_imported_env = 0; const char** imported_env = NULL; - if (sgxlkl_enclave_state.shared_memory.env && cfg->num_host_import_env > 0) + if (sgxlkl_enclave_state.shared_memory.env && + sgxlkl_enclave_state.shared_memory.envc && cfg->num_host_import_env > 0) { imported_env = oe_calloc_or_die( cfg->num_host_import_env, @@ -118,16 +119,12 @@ static void _prepare_elf_stack() for (size_t i = 0; i < cfg->num_host_import_env; i++) { const char* name = cfg->host_import_env[i]; - for (char* const* p = sgxlkl_enclave_state.shared_memory.env; - p && *p != NULL; - p++) + size_t n = oe_strlen(name); + for (size_t i = 0; i < sgxlkl_enclave_state.shared_memory.envc; i++) { - size_t n = oe_strlen(name); - if (_strncmp(name, *p, n) == 0 && (*p)[n] == '=') - { - const char* str = *p; - imported_env[num_imported_env++] = str; - } + const char* henv_i = sgxlkl_enclave_state.shared_memory.env[i]; + if (_strncmp(name, henv_i, n) == 0 && henv_i[n] == '=') + imported_env[num_imported_env++] = henv_i; } } } @@ -388,14 +385,12 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) /* Copy the host's environment variables to enclave memory */ char* const* henv = host->env; - if (henv) + size_t henvc = host->envc; + if (henv && henvc) { - size_t henvc = 0; - while (henv[henvc] != 0) - henvc++; CHECK_OUTSIDE(henv, sizeof(char*) * henvc); char** tmp = oe_calloc_or_die( - henvc + 1, + henvc, sizeof(char*), "Could not allocate memory for host import environment variable\n"); for (size_t i = 0; i < henvc; i++) @@ -406,8 +401,9 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) tmp[i] = oe_malloc(n); memcpy(tmp[i], env_i, n); } - tmp[henvc] = NULL; enc->env = tmp; + enc->envc = henvc; + CHECK_INSIDE(enc->env, sizeof(char*) * enc->envc); } /* Commit to the temporary copy */ @@ -425,8 +421,9 @@ static void _free_shared_memory() oe_free(shm->virtio_blk_dev_mem); oe_free(shm->virtio_blk_dev_names); - for (size_t i = 0; shm->env[i] != 0; i++) - oe_free(shm->env[i]); + if (shm->env && shm->envc) + for (size_t i = 0; i < shm->envc; i++) + oe_free(shm->env[i]); oe_free((char**)shm->env); } diff --git a/src/include/shared/shared_memory.h b/src/include/shared/shared_memory.h index 93674fb8f..a9ac58ad9 100644 --- a/src/include/shared/shared_memory.h +++ b/src/include/shared/shared_memory.h @@ -29,6 +29,7 @@ typedef struct sgxlkl_shared_memory /* Host environment variables for optional import */ char* const* env; + size_t envc; } sgxlkl_shared_memory_t; #endif /* SGXLKL_SHARED_MEMORY_H */ \ No newline at end of file diff --git a/src/main-oe/sgxlkl_run_oe.c b/src/main-oe/sgxlkl_run_oe.c index e30cb84ad..510c66801 100644 --- a/src/main-oe/sgxlkl_run_oe.c +++ b/src/main-oe/sgxlkl_run_oe.c @@ -1884,6 +1884,9 @@ int main(int argc, char* argv[], char* envp[]) bool have_enclave_config_file = enclave_config_path != NULL; set_clock_res(have_enclave_config_file); + sgxlkl_host_state.shared_memory.envc = 0; + for (char** env = envp; *env != 0; env++) + sgxlkl_host_state.shared_memory.envc++; sgxlkl_host_state.shared_memory.env = envp; set_tls(have_enclave_config_file); register_hds(root_hd); diff --git a/tests/basic/eeid-config/Makefile b/tests/basic/eeid-config/Makefile index 7efc8772f..afab966fd 100644 --- a/tests/basic/eeid-config/Makefile +++ b/tests/basic/eeid-config/Makefile @@ -6,7 +6,7 @@ IMAGE_SIZE=5M EXECUTION_TIMEOUT=60 -SGXLKL_ENV=SGXLKL_VERBOSE=1 SGXLKL_KERNEL_VERBOSE=1 +SGXLKL_ENV=SGXLKL_VERBOSE=1 SGXLKL_KERNEL_VERBOSE=1 HOSTNAME=EEIDHOST SGXLKL_HW_PARAMS=--hw-debug SGXLKL_SW_PARAMS=--sw-debug diff --git a/tests/basic/eeid-config/hello-eeid.c b/tests/basic/eeid-config/hello-eeid.c index 693dfe519..6c66984bd 100644 --- a/tests/basic/eeid-config/hello-eeid.c +++ b/tests/basic/eeid-config/hello-eeid.c @@ -31,5 +31,16 @@ int main(int argc, char** argv) exit(1); } + // Application environment variable + const char* abc = getenv("ABC"); + if (strcmp(abc, "DEF") != 0) + exit(1); + + // Environment variable imported from host + const char* hostname = getenv("HOSTNAME"); + printf("HOSTNAME=%s\n", hostname); + if (!hostname || strcmp(hostname, "EEIDHOST") != 0) + exit(1); + return 0; } From b3b466f08fb3e70e0b3e1df6caac83bd0ef4ed06 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Mon, 17 Aug 2020 15:37:45 +0000 Subject: [PATCH 06/15] Deep-copy sgxlkl_enclave_config->enc_dev_config. Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_event_channel.c | 21 ++---- src/enclave/enclave_oe.c | 111 ++++++++++++++++++++++------ src/enclave/enclave_timer.c | 3 +- src/include/enclave/enclave_util.h | 6 ++ 4 files changed, 102 insertions(+), 39 deletions(-) diff --git a/src/enclave/enclave_event_channel.c b/src/enclave/enclave_event_channel.c index 54df2ad51..9d384abbd 100644 --- a/src/enclave/enclave_event_channel.c +++ b/src/enclave/enclave_event_channel.c @@ -137,9 +137,8 @@ void initialize_enclave_event_channel( uint8_t* dev_id = NULL; _evt_channel_num = evt_channel_num; - if (!oe_is_outside_enclave( - enc_dev_config, sizeof(enc_dev_config_t) * _evt_channel_num)) - oe_abort(); + sgxlkl_ensure_inside( + enc_dev_config, sizeof(enc_dev_config_t) * _evt_channel_num); evt_chn_lock = (struct ticketlock**)oe_calloc_or_die( evt_channel_num, @@ -154,16 +153,10 @@ void initialize_enclave_event_channel( _enc_dev_config = enc_dev_config; for (int i = 0; i < evt_channel_num; i++) { - if (!oe_is_outside_enclave( - &enc_dev_config[i], sizeof(enc_dev_config_t)) || - !oe_is_outside_enclave( - &enc_dev_config[i].enc_evt_chn, sizeof(enc_evt_channel_t)) || - !oe_is_outside_enclave( - &enc_dev_config[i].enc_evt_chn->host_evt_channel, - sizeof(evt_t)) || - !oe_is_outside_enclave( - &enc_dev_config[i].enc_evt_chn->qidx_p, sizeof(uint32_t))) - oe_abort(); + const enc_dev_config_t* ed_conf_i = &enc_dev_config[i]; + // const enc_evt_channel_t* ee_chan_i = ed_conf_i->enc_evt_chn; + // sgxlkl_ensure_outside(ee_chan_i->host_evt_channel, sizeof(evt_t)); + // sgxlkl_ensure_outside(ee_chan_i->qidx_p, sizeof(uint32_t)); evt_chn_lock[i] = (struct ticketlock*)oe_calloc_or_die( 1, @@ -174,7 +167,7 @@ void initialize_enclave_event_channel( dev_id = (uint8_t*)oe_calloc_or_die( 1, sizeof(uint8_t), "Could not allocate memory for dev_id\n"); - *dev_id = enc_dev_config[i].dev_id; + *dev_id = ed_conf_i->dev_id; if (lthread_create( &vio_tasks[i], diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index b7682efb9..497bac4a5 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -304,17 +304,17 @@ static void _read_eeid_config() sgxlkl_enclave_state.config = cfg; } -#define CHECK_INSIDE(X, S) \ - { \ - if ((X) != NULL && !oe_is_within_enclave((X), (S))) \ - oe_abort(); \ - } +void sgxlkl_ensure_inside(const void* ptr, size_t sz) +{ + if (ptr != NULL && !oe_is_within_enclave(ptr, sz)) + sgxlkl_fail("Unexpected: buffer is not in enclave memory.\n"); +} -#define CHECK_OUTSIDE(X, S) \ - { \ - if ((X) != NULL && !oe_is_outside_enclave((X), (S))) \ - oe_abort(); \ - } +void sgxlkl_ensure_outside(const void* ptr, size_t sz) +{ + if (ptr != NULL && !oe_is_outside_enclave(ptr, sz)) + sgxlkl_fail("Unexpected: buffer is in enclave memory.\n"); +} static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) { @@ -322,7 +322,7 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) /* The host shared memory struct has been copied by OE (but not its * contents). */ - CHECK_INSIDE(host, sizeof(sgxlkl_shared_memory_t)); + sgxlkl_ensure_inside(host, sizeof(sgxlkl_shared_memory_t)); /* Temporary, volatile copy to make sure checks aren't reordered */ volatile sgxlkl_shared_memory_t* enc = oe_calloc_or_die( @@ -333,26 +333,80 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) if (cfg->io.network) { enc->virtio_net_dev_mem = host->virtio_net_dev_mem; - CHECK_OUTSIDE(enc->virtio_net_dev_mem, sizeof(struct virtio_dev)); + sgxlkl_ensure_outside( + enc->virtio_net_dev_mem, sizeof(struct virtio_dev)); } if (cfg->io.console) { enc->virtio_console_mem = host->virtio_console_mem; - CHECK_OUTSIDE(enc->virtio_console_mem, sizeof(struct virtio_dev)); + sgxlkl_ensure_outside( + enc->virtio_console_mem, sizeof(struct virtio_dev)); } enc->evt_channel_num = host->evt_channel_num; - enc->enc_dev_config = host->enc_dev_config; - CHECK_OUTSIDE( - enc->enc_dev_config, sizeof(enc_dev_config_t) * enc->evt_channel_num); + + if (enc->evt_channel_num > 0) + { + _Static_assert( + sizeof(enc_dev_config_t) == 24UL, + "enc_dev_config_t size has changed"); + + enc->enc_dev_config = oe_calloc_or_die( + enc->evt_channel_num, + sizeof(enc_dev_config_t), + "Could not allocate memory for event channel device config\n"); + + for (size_t i = 0; i < enc->evt_channel_num; i++) + { + const enc_dev_config_t* host_conf_i = &host->enc_dev_config[i]; + enc_dev_config_t* enc_conf_i = &enc->enc_dev_config[i]; + + enc_conf_i->dev_id = host_conf_i->dev_id; + + enc_evt_channel_t* host_chn = host_conf_i->enc_evt_chn; + if (host_chn) + { + enc_conf_i->enc_evt_chn = host_chn; + + // enc_conf_i->enc_evt_chn = oe_calloc_or_die( + // 1, + // sizeof(enc_evt_channel_t), + // "Could not allocate memory for event channel config\n "); + + // enc_conf_i->enc_evt_chn->host_evt_channel = oe_calloc_or_die( + // 1, + // sizeof(evt_t), + // "Could not allocate memory for host event channel + // evt_t\n"); + + // enc_conf_i->enc_evt_chn->qidx_p = oe_calloc_or_die( + // 1, + // sizeof(uint32_t), + // "Could not allocate memory for event channel qidx_p\n "); + + // enc_evt_channel_t* enc_chn = enc_conf_i->enc_evt_chn; + + // enc_chn->enclave_evt_channel = host_chn->enclave_evt_channel; + + // evt_t* hechan = host_chn->host_evt_channel; + // sgxlkl_ensure_outside(hechan, sizeof(evt_t)); + //*enc_chn->host_evt_channel = *hechan; + + // uint32_t* qidx_p = host_chn->qidx_p; + // sgxlkl_ensure_outside(qidx_p, sizeof(uint32_t)); + //*enc_chn->qidx_p = *qidx_p; + } + enc_conf_i->evt_processed = host_conf_i->evt_processed; + } + } enc->virtio_swiotlb = host->virtio_swiotlb; enc->virtio_swiotlb_size = host->virtio_swiotlb_size; - CHECK_OUTSIDE(enc->virtio_swiotlb, enc->virtio_swiotlb_size); + sgxlkl_ensure_outside(enc->virtio_swiotlb, enc->virtio_swiotlb_size); enc->timer_dev_mem = host->timer_dev_mem; - CHECK_OUTSIDE(enc->timer_dev_mem, sizeof(struct timer_dev)); + sgxlkl_ensure_outside(enc->timer_dev_mem, sizeof(struct timer_dev)); if (cfg->io.block) { @@ -369,12 +423,12 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) for (size_t i = 0; i < enc->num_virtio_blk_dev; i++) { enc->virtio_blk_dev_mem[i] = host->virtio_blk_dev_mem[i]; - CHECK_OUTSIDE( + sgxlkl_ensure_outside( enc->virtio_blk_dev_mem[i], sizeof(struct virtio_dev)); const char* name = host->virtio_blk_dev_names[i]; size_t name_len = oe_strlen(name) + 1; - CHECK_OUTSIDE(name, name_len); + sgxlkl_ensure_outside(name, name_len); enc->virtio_blk_dev_names[i] = oe_calloc_or_die( name_len, sizeof(char), @@ -388,7 +442,7 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) size_t henvc = host->envc; if (henv && henvc) { - CHECK_OUTSIDE(henv, sizeof(char*) * henvc); + sgxlkl_ensure_outside(henv, sizeof(char*) * henvc); char** tmp = oe_calloc_or_die( henvc, sizeof(char*), @@ -397,13 +451,13 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) { const char* env_i = henv[i]; size_t n = strlen(env_i) + 1; - CHECK_OUTSIDE(env_i, n); + sgxlkl_ensure_outside(env_i, n); tmp[i] = oe_malloc(n); memcpy(tmp[i], env_i, n); } enc->env = tmp; enc->envc = henvc; - CHECK_INSIDE(enc->env, sizeof(char*) * enc->envc); + sgxlkl_ensure_inside(enc->env, sizeof(char*) * enc->envc); } /* Commit to the temporary copy */ @@ -415,6 +469,17 @@ static void _free_shared_memory() { sgxlkl_shared_memory_t* shm = &sgxlkl_enclave_state.shared_memory; + for (size_t i = 0; i < shm->evt_channel_num; i++) + { + if (shm->enc_dev_config[i].enc_evt_chn) + { + oe_free(shm->enc_dev_config[i].enc_evt_chn->host_evt_channel); + oe_free(shm->enc_dev_config[i].enc_evt_chn->qidx_p); + oe_free(shm->enc_dev_config[i].enc_evt_chn); + } + } + oe_free(shm->enc_dev_config); + for (size_t i = 0; i < shm->num_virtio_blk_dev; i++) oe_free(shm->virtio_blk_dev_names[i]); diff --git a/src/enclave/enclave_timer.c b/src/enclave/enclave_timer.c index cb9bcb2b9..c42d8bd6b 100644 --- a/src/enclave/enclave_timer.c +++ b/src/enclave/enclave_timer.c @@ -30,8 +30,7 @@ uint64_t enclave_nanos() { struct timer_dev* t = sgxlkl_enclave_state.shared_memory.timer_dev_mem; - if (!oe_is_outside_enclave(t, sizeof(struct timer_dev))) - oe_abort(); + sgxlkl_ensure_outside(t, sizeof(struct timer_dev)); uint64_t e = t->nanos; uint64_t i = internal_counter; diff --git a/src/include/enclave/enclave_util.h b/src/include/enclave/enclave_util.h index 586ec6de8..639f0508b 100644 --- a/src/include/enclave/enclave_util.h +++ b/src/include/enclave/enclave_util.h @@ -91,6 +91,12 @@ void* oe_malloc_or_die(size_t size, const char* fail_msg, ...); */ void* oe_calloc_or_die(size_t nmemb, size_t size, const char* fail_msg, ...); +/** + * Ensure buffers are held entirely inside or outside the enclave memory. + */ +void sgxlkl_ensure_inside(const void* ptr, size_t sz); +void sgxlkl_ensure_outside(const void* ptr, size_t sz); + /** * * Note that generating a stack trace by unwinding stack frames could be exploited From 51a88c27384c7a265f13af0cd691983eda3f840e Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Tue, 18 Aug 2020 15:44:59 +0000 Subject: [PATCH 07/15] Clean up event channel shared memory Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_event_channel.c | 73 +++++++++++++---------- src/enclave/enclave_oe.c | 81 ++++++++++++-------------- src/include/enclave/enclave_state.h | 10 ++++ src/include/shared/shared_memory.h | 2 +- src/include/shared/vio_event_channel.h | 1 - src/lkl/setup.c | 6 +- src/main-oe/sgxlkl_evt_chn_cfg.c | 1 - src/main-oe/sgxlkl_run_oe.c | 6 +- 8 files changed, 94 insertions(+), 86 deletions(-) diff --git a/src/enclave/enclave_event_channel.c b/src/enclave/enclave_event_channel.c index 9d384abbd..01da0c9e2 100644 --- a/src/enclave/enclave_event_channel.c +++ b/src/enclave/enclave_event_channel.c @@ -19,9 +19,7 @@ static struct ticketlock** evt_chn_lock; struct lthread** vio_tasks = NULL; /* Event channel configuration */ -static enc_dev_config_t* _enc_dev_config = NULL; static bool _event_channel_initialized = false; -static uint8_t _evt_channel_num; static _Atomic(bool) _vio_terminate = false; @@ -55,10 +53,16 @@ static inline void vio_wait_for_host_event( */ static inline int vio_signal_evt_channel(uint8_t dev_id) { - enc_evt_channel_t* evt_channel = _enc_dev_config[dev_id].enc_evt_chn; - evt_t* processed = &_enc_dev_config[dev_id].evt_processed; + enc_dev_config_t* enc_dev_config = + sgxlkl_enclave_state.shared_memory.enc_dev_config; + enc_evt_channel_t* evt_channel = enc_dev_config[dev_id].enc_evt_chn; + evt_t* processed = + &sgxlkl_enclave_state.event_channel_state[dev_id].evt_processed; evt_t* evt_chn = &evt_channel->enclave_evt_channel; + sgxlkl_ensure_outside(evt_channel, sizeof(enc_evt_channel_t)); + sgxlkl_ensure_outside(evt_chn, sizeof(evt_t)); + evt_t cur = __atomic_load_n(evt_chn, __ATOMIC_SEQ_CST); struct lthread* lt = vio_tasks[dev_id]; @@ -83,14 +87,17 @@ static void vio_enclave_process_host_event(uint8_t* param) lthread_set_funcname(lthread_self(), thread_name); lthread_detach(); - /* release memory after extracting dev_id */ - oe_free(param); - - enc_evt_channel_t* evt_channel = _enc_dev_config[dev_id].enc_evt_chn; - evt_t* evt_processed = &_enc_dev_config[dev_id].evt_processed; + enc_dev_config_t* enc_dev_config = + sgxlkl_enclave_state.shared_memory.enc_dev_config; + enc_evt_channel_t* evt_channel = enc_dev_config[dev_id].enc_evt_chn; + evt_t* evt_processed = + &sgxlkl_enclave_state.event_channel_state[dev_id].evt_processed; evt_t* evt_chn = &evt_channel->enclave_evt_channel; evt_t cur = 0, new = 0; + sgxlkl_ensure_outside(evt_channel, sizeof(enc_evt_channel_t)); + sgxlkl_ensure_outside(evt_chn, sizeof(evt_t)); + for (;;) { if (_vio_terminate) @@ -130,33 +137,33 @@ static void vio_enclave_process_host_event(uint8_t* param) /* * Function to initialize enclave event handler */ -void initialize_enclave_event_channel( - enc_dev_config_t* enc_dev_config, - size_t evt_channel_num) +void initialize_enclave_event_channels(void) { - uint8_t* dev_id = NULL; - _evt_channel_num = evt_channel_num; + enc_dev_config_t* enc_dev_config = + sgxlkl_enclave_state.shared_memory.enc_dev_config; + const size_t _num_channels = sgxlkl_enclave_state.num_event_channel_state; sgxlkl_ensure_inside( - enc_dev_config, sizeof(enc_dev_config_t) * _evt_channel_num); + enc_dev_config, sizeof(enc_dev_config_t) * _num_channels); evt_chn_lock = (struct ticketlock**)oe_calloc_or_die( - evt_channel_num, + _num_channels, sizeof(struct ticketlock*), "Could not allocate memory for evt_chn_lock\n"); vio_tasks = (struct lthread**)oe_calloc_or_die( - evt_channel_num, + _num_channels, sizeof(struct lthread*), "Could not allocate memory for vio_tasks\n"); - _enc_dev_config = enc_dev_config; - for (int i = 0; i < evt_channel_num; i++) + for (int i = 0; i < _num_channels; i++) { const enc_dev_config_t* ed_conf_i = &enc_dev_config[i]; - // const enc_evt_channel_t* ee_chan_i = ed_conf_i->enc_evt_chn; - // sgxlkl_ensure_outside(ee_chan_i->host_evt_channel, sizeof(evt_t)); - // sgxlkl_ensure_outside(ee_chan_i->qidx_p, sizeof(uint32_t)); + const enc_evt_channel_t* ee_chan_i = ed_conf_i->enc_evt_chn; + sgxlkl_ensure_inside(ed_conf_i, sizeof(enc_dev_config_t)); + sgxlkl_ensure_outside(ee_chan_i, sizeof(enc_evt_channel_t)); + sgxlkl_ensure_outside(ee_chan_i->host_evt_channel, sizeof(evt_t)); + sgxlkl_ensure_outside(ee_chan_i->qidx_p, sizeof(uint32_t)); evt_chn_lock[i] = (struct ticketlock*)oe_calloc_or_die( 1, @@ -164,16 +171,11 @@ void initialize_enclave_event_channel( "Could not allocate memory for evt_chn_lock[%i]\n", i); - dev_id = (uint8_t*)oe_calloc_or_die( - 1, sizeof(uint8_t), "Could not allocate memory for dev_id\n"); - - *dev_id = ed_conf_i->dev_id; - if (lthread_create( &vio_tasks[i], NULL, vio_enclave_process_host_event, - (void*)dev_id) != 0) + (void*)&ed_conf_i->dev_id) != 0) { oe_free(vio_tasks); sgxlkl_fail("Failed to create lthread for event channel\n"); @@ -193,11 +195,17 @@ void vio_terminate() */ void vio_enclave_notify_enclave_event(uint8_t dev_id, uint32_t qidx) { - enc_evt_channel_t* evt_chn = _enc_dev_config[dev_id].enc_evt_chn; + enc_dev_config_t* enc_dev_config = + sgxlkl_enclave_state.shared_memory.enc_dev_config; + enc_evt_channel_t* evt_chn = enc_dev_config[dev_id].enc_evt_chn; uint32_t* qidx_p = evt_chn->qidx_p; + evt_t* host_evt_chn = evt_chn->host_evt_channel; + + sgxlkl_ensure_outside(evt_chn, sizeof(enc_evt_channel_t)); + sgxlkl_ensure_outside(qidx_p, sizeof(uint32_t)); + sgxlkl_ensure_outside(host_evt_chn, sizeof(evt_t)); - evt_t cur = - __atomic_fetch_add(evt_chn->host_evt_channel, 2, __ATOMIC_SEQ_CST); + evt_t cur = __atomic_fetch_add(host_evt_chn, 2, __ATOMIC_SEQ_CST); *qidx_p = qidx; @@ -212,13 +220,14 @@ void vio_enclave_notify_enclave_event(uint8_t dev_id, uint32_t qidx) int vio_enclave_wakeup_event_channel(void) { int ret = 0; + const size_t _num_channels = sgxlkl_enclave_state.num_event_channel_state; /* Event channel processing not available or terminating */ if (!_event_channel_initialized || _vio_terminate) return 0; /* Schedule picks up the available event channel processing */ - for (uint8_t dev_id = 0; dev_id < _evt_channel_num; dev_id++) + for (uint8_t dev_id = 0; dev_id < _num_channels; dev_id++) { if (ticket_trylock(evt_chn_lock[dev_id])) continue; diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index 497bac4a5..39a1a66ee 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -344,60 +344,30 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) enc->virtio_console_mem, sizeof(struct virtio_dev)); } - enc->evt_channel_num = host->evt_channel_num; - - if (enc->evt_channel_num > 0) + size_t num_channels = host->num_evt_channel; + if (num_channels > 0) { _Static_assert( - sizeof(enc_dev_config_t) == 24UL, + sizeof(enc_dev_config_t) == 16UL, "enc_dev_config_t size has changed"); + _Static_assert( + sizeof(enc_dev_state_t) == 8UL, "enc_dev_state_t size has changed"); + + enc->num_evt_channel = num_channels; enc->enc_dev_config = oe_calloc_or_die( - enc->evt_channel_num, + num_channels, sizeof(enc_dev_config_t), - "Could not allocate memory for event channel device config\n"); + "Could not allocate memory for event channel config\n"); - for (size_t i = 0; i < enc->evt_channel_num; i++) + for (size_t i = 0; i < num_channels; i++) { const enc_dev_config_t* host_conf_i = &host->enc_dev_config[i]; enc_dev_config_t* enc_conf_i = &enc->enc_dev_config[i]; - enc_conf_i->dev_id = host_conf_i->dev_id; - - enc_evt_channel_t* host_chn = host_conf_i->enc_evt_chn; - if (host_chn) - { - enc_conf_i->enc_evt_chn = host_chn; - - // enc_conf_i->enc_evt_chn = oe_calloc_or_die( - // 1, - // sizeof(enc_evt_channel_t), - // "Could not allocate memory for event channel config\n "); - - // enc_conf_i->enc_evt_chn->host_evt_channel = oe_calloc_or_die( - // 1, - // sizeof(evt_t), - // "Could not allocate memory for host event channel - // evt_t\n"); - - // enc_conf_i->enc_evt_chn->qidx_p = oe_calloc_or_die( - // 1, - // sizeof(uint32_t), - // "Could not allocate memory for event channel qidx_p\n "); - - // enc_evt_channel_t* enc_chn = enc_conf_i->enc_evt_chn; - - // enc_chn->enclave_evt_channel = host_chn->enclave_evt_channel; - - // evt_t* hechan = host_chn->host_evt_channel; - // sgxlkl_ensure_outside(hechan, sizeof(evt_t)); - //*enc_chn->host_evt_channel = *hechan; - - // uint32_t* qidx_p = host_chn->qidx_p; - // sgxlkl_ensure_outside(qidx_p, sizeof(uint32_t)); - //*enc_chn->qidx_p = *qidx_p; - } - enc_conf_i->evt_processed = host_conf_i->evt_processed; + enc_conf_i->enc_evt_chn = host_conf_i->enc_evt_chn; + sgxlkl_ensure_outside( + enc_conf_i->enc_evt_chn, sizeof(enc_evt_channel_t)); } } @@ -465,11 +435,30 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) oe_free((sgxlkl_shared_memory_t*)enc); } +static void _init_event_channel_state(void) +{ + const sgxlkl_shared_memory_t* shm = &sgxlkl_enclave_state.shared_memory; + size_t num_channels = shm->num_evt_channel; + + sgxlkl_enclave_state.event_channel_state = oe_calloc_or_die( + num_channels, + sizeof(enc_dev_state_t), + "Could not allocate memory for event channel state\n"); + + sgxlkl_enclave_state.num_event_channel_state = num_channels; + for (size_t i = 0; i < num_channels; i++) + { + enc_dev_state_t* enc_state_i = + &sgxlkl_enclave_state.event_channel_state[i]; + enc_state_i->evt_processed = 0; + } +} + static void _free_shared_memory() { sgxlkl_shared_memory_t* shm = &sgxlkl_enclave_state.shared_memory; - for (size_t i = 0; i < shm->evt_channel_num; i++) + for (size_t i = 0; i < shm->num_evt_channel; i++) { if (shm->enc_dev_config[i].enc_evt_chn) { @@ -508,6 +497,7 @@ int sgxlkl_enclave_init(const sgxlkl_shared_memory_t* shared_memory) _read_eeid_config(); _copy_shared_memory(shared_memory); + _init_event_channel_state(); #ifdef DEBUG // Initialise verbosity setting, so SGXLKL_VERBOSE can be used from this @@ -554,6 +544,9 @@ void sgxlkl_free_enclave_state() state->libc_state = libc_not_started; _free_shared_memory(); + + state->num_event_channel_state = 0; + oe_free(state->event_channel_state); } void sgxlkl_debug_dump_stack_traces(void) diff --git a/src/include/enclave/enclave_state.h b/src/include/enclave/enclave_state.h index 15eaf1a2f..8e1ac7a5b 100644 --- a/src/include/enclave/enclave_state.h +++ b/src/include/enclave/enclave_state.h @@ -31,6 +31,12 @@ typedef struct char* data; /* Buffer that holds all strings on the stack */ } elf64_stack_t; +/* State of event channels */ +typedef struct enc_dev_state +{ + evt_t evt_processed; +} enc_dev_state_t; + typedef struct sgxlkl_enclave_state { _Atomic(bool) initialized; @@ -55,6 +61,10 @@ typedef struct sgxlkl_enclave_state /* This flag is used by the tracing macros */ bool verbose; + + /* Event channel state */ + size_t num_event_channel_state; + enc_dev_state_t* event_channel_state; } sgxlkl_enclave_state_t; extern sgxlkl_enclave_state_t sgxlkl_enclave_state; diff --git a/src/include/shared/shared_memory.h b/src/include/shared/shared_memory.h index a9ac58ad9..4eb8d0f6b 100644 --- a/src/include/shared/shared_memory.h +++ b/src/include/shared/shared_memory.h @@ -13,7 +13,7 @@ typedef struct sgxlkl_shared_memory struct virtio_dev* virtio_net_dev_mem; /* Virtio network device */ struct virtio_dev* virtio_console_mem; /* Virtio console device */ - size_t evt_channel_num; /* Number of event channels */ + size_t num_evt_channel; /* Number of event channels */ enc_dev_config_t* enc_dev_config; /* Device configuration */ void* virtio_swiotlb; /* Memory for setting up bounce buffer */ diff --git a/src/include/shared/vio_event_channel.h b/src/include/shared/vio_event_channel.h index 5c93253cb..360498eae 100644 --- a/src/include/shared/vio_event_channel.h +++ b/src/include/shared/vio_event_channel.h @@ -18,7 +18,6 @@ typedef struct enc_dev_config { uint8_t dev_id; enc_evt_channel_t* enc_evt_chn; - evt_t evt_processed; } enc_dev_config_t; #endif //_VIO_EVENT_CHANNEL_H diff --git a/src/lkl/setup.c b/src/lkl/setup.c index 186a32896..48d6bd499 100644 --- a/src/lkl/setup.c +++ b/src/lkl/setup.c @@ -96,9 +96,7 @@ int sgxlkl_mtu = 0; extern struct timespec sgxlkl_app_starttime; /* Function to setup bounce buffer in LKL */ -extern void initialize_enclave_event_channel( - enc_dev_config_t* enc_dev_config, - size_t evt_channel_num); +extern void initialize_enclave_event_channels(void); extern void lkl_virtio_netdev_remove(void); extern void vio_terminate(void); @@ -1393,7 +1391,7 @@ void lkl_start_init() sgxlkl_mtu = cfg->tap_mtu; SGXLKL_VERBOSE("calling initialize_enclave_event_channel()\n"); - initialize_enclave_event_channel(shm->enc_dev_config, shm->evt_channel_num); + initialize_enclave_event_channels(); // Register console device lkl_virtio_console_add(shm->virtio_console_mem); diff --git a/src/main-oe/sgxlkl_evt_chn_cfg.c b/src/main-oe/sgxlkl_evt_chn_cfg.c index 5d946d624..7bf6d5190 100644 --- a/src/main-oe/sgxlkl_evt_chn_cfg.c +++ b/src/main-oe/sgxlkl_evt_chn_cfg.c @@ -119,7 +119,6 @@ static void host_dev_cfg_init( enc_dev_config_t* edev_cfg = &enc_dev_cfg[index]; edev_cfg->dev_id = (uint8_t)index; edev_cfg->enc_evt_chn = &enc_evt_channel[index]; - edev_cfg->evt_processed = 0; } *h_dev_config = host_dev_cfg; *e_dev_config = enc_dev_cfg; diff --git a/src/main-oe/sgxlkl_run_oe.c b/src/main-oe/sgxlkl_run_oe.c index 510c66801..2d2e3824e 100644 --- a/src/main-oe/sgxlkl_run_oe.c +++ b/src/main-oe/sgxlkl_run_oe.c @@ -1963,7 +1963,7 @@ int main(int argc, char* argv[], char* envp[]) * console device. Each block disk is treated as a seperate block * device and have an event channel associated with it. */ - sgxlkl_host_state.shared_memory.evt_channel_num = + sgxlkl_host_state.shared_memory.num_evt_channel = sgxlkl_host_state.num_disks + HOST_NETWORK_DEV_COUNT + HOST_CONSOLE_DEV_COUNT; @@ -1976,11 +1976,11 @@ int main(int argc, char* argv[], char* envp[]) &sgxlkl_host_state.shared_memory, &host_dev_cfg, &enc_dev_config, - sgxlkl_host_state.shared_memory.evt_channel_num); + sgxlkl_host_state.shared_memory.num_evt_channel); /* Initialize the host dev configuration in host event handler */ vio_host_initialize_device_cfg( - host_dev_cfg, sgxlkl_host_state.shared_memory.evt_channel_num); + host_dev_cfg, sgxlkl_host_state.shared_memory.num_evt_channel); int dev_index = 0; From aaec85609732f28014fb0bfab1a952a0ac5dde06 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Tue, 18 Aug 2020 18:49:13 +0000 Subject: [PATCH 08/15] Fix host environment variable import Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_oe.c | 99 +++++++++++++++-------------- src/include/enclave/enclave_state.h | 4 ++ src/include/shared/shared_memory.h | 4 +- src/main-oe/sgxlkl_run_oe.c | 17 +++-- 4 files changed, 72 insertions(+), 52 deletions(-) diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index 39a1a66ee..05eacd04b 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -105,29 +105,8 @@ static void _prepare_elf_stack() sgxlkl_enclave_state_t* state = &sgxlkl_enclave_state; const sgxlkl_enclave_config_t* cfg = state->config; - size_t num_imported_env = 0; - const char** imported_env = NULL; - - if (sgxlkl_enclave_state.shared_memory.env && - sgxlkl_enclave_state.shared_memory.envc && cfg->num_host_import_env > 0) - { - imported_env = oe_calloc_or_die( - cfg->num_host_import_env, - sizeof(char*), - "Could not allocate memory for imported host environment\n"); - - for (size_t i = 0; i < cfg->num_host_import_env; i++) - { - const char* name = cfg->host_import_env[i]; - size_t n = oe_strlen(name); - for (size_t i = 0; i < sgxlkl_enclave_state.shared_memory.envc; i++) - { - const char* henv_i = sgxlkl_enclave_state.shared_memory.env[i]; - if (_strncmp(name, henv_i, n) == 0 && henv_i[n] == '=') - imported_env[num_imported_env++] = henv_i; - } - } - } + size_t num_imported_env = state->num_env_imported; + char* const* imported_env = state->env_imported; size_t num_bytes = 0; size_t num_ptrs = 0; @@ -189,8 +168,6 @@ static void _prepare_elf_stack() SGXLKL_ASSERT(j + 1 == num_ptrs); SGXLKL_ASSERT(out[j] == NULL); SGXLKL_ASSERT(out[j - 4] == (char*)AT_HW_MODE); - - oe_free(imported_env); } static void _free_elf_stack() @@ -407,27 +384,58 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) } } - /* Copy the host's environment variables to enclave memory */ - char* const* henv = host->env; - size_t henvc = host->envc; - if (henv && henvc) + /* Import environment variables from the host */ + const char* henv = host->env; + size_t henv_len = host->env_length; + if (cfg->num_host_import_env && cfg->host_import_env && henv && henv_len) { - sgxlkl_ensure_outside(henv, sizeof(char*) * henvc); + sgxlkl_ensure_outside(henv, sizeof(char) * henv_len); + + size_t num_vars = 0; + for (size_t i = 0; i < henv_len; i++) + if (henv[i] == '\0') + num_vars++; + char** tmp = oe_calloc_or_die( - henvc, + num_vars, sizeof(char*), - "Could not allocate memory for host import environment variable\n"); - for (size_t i = 0; i < henvc; i++) + "Could not allocate memory for host-imported environment " + "variables\n"); + + size_t begin = 0; + sgxlkl_enclave_state.num_env_imported = 0; + for (size_t i = 0; i < henv_len; i++) { - const char* env_i = henv[i]; - size_t n = strlen(env_i) + 1; - sgxlkl_ensure_outside(env_i, n); - tmp[i] = oe_malloc(n); - memcpy(tmp[i], env_i, n); + if (henv[i] == '\0') + { + const char* henv_i = henv + begin; + size_t henv_i_len = i - begin; + + for (size_t j = 0; j < cfg->num_host_import_env; j++) + { + const char* name = cfg->host_import_env[j]; + size_t name_len = oe_strlen(name); + if (name_len + 1 < henv_i_len && + _strncmp(name, henv_i, name_len) == 0 && + henv_i[name_len] == '=') + { + tmp[sgxlkl_enclave_state.num_env_imported++] = + oe_strdup(henv_i); + } + } + + begin = i + 1; + } } - enc->env = tmp; - enc->envc = henvc; - sgxlkl_ensure_inside(enc->env, sizeof(char*) * enc->envc); + + if (sgxlkl_enclave_state.num_env_imported == 0) + oe_free(tmp); + else + sgxlkl_enclave_state.env_imported = tmp; + + /* Shared memory not needed anymore. */ + sgxlkl_enclave_state.shared_memory.env = NULL; + sgxlkl_enclave_state.shared_memory.env_length = 0; } /* Commit to the temporary copy */ @@ -474,11 +482,6 @@ static void _free_shared_memory() oe_free(shm->virtio_blk_dev_mem); oe_free(shm->virtio_blk_dev_names); - - if (shm->env && shm->envc) - for (size_t i = 0; i < shm->envc; i++) - oe_free(shm->env[i]); - oe_free((char**)shm->env); } int sgxlkl_enclave_init(const sgxlkl_shared_memory_t* shared_memory) @@ -547,6 +550,10 @@ void sgxlkl_free_enclave_state() state->num_event_channel_state = 0; oe_free(state->event_channel_state); + + for (size_t i = 0; i < state->num_env_imported; i++) + oe_free(state->env_imported[i]); + oe_free((void*)state->env_imported); } void sgxlkl_debug_dump_stack_traces(void) diff --git a/src/include/enclave/enclave_state.h b/src/include/enclave/enclave_state.h index 8e1ac7a5b..49c3781e2 100644 --- a/src/include/enclave/enclave_state.h +++ b/src/include/enclave/enclave_state.h @@ -65,6 +65,10 @@ typedef struct sgxlkl_enclave_state /* Event channel state */ size_t num_event_channel_state; enc_dev_state_t* event_channel_state; + + /* Environment variables imported from the host */ + size_t num_env_imported; + char* const* env_imported; } sgxlkl_enclave_state_t; extern sgxlkl_enclave_state_t sgxlkl_enclave_state; diff --git a/src/include/shared/shared_memory.h b/src/include/shared/shared_memory.h index 4eb8d0f6b..9e6189282 100644 --- a/src/include/shared/shared_memory.h +++ b/src/include/shared/shared_memory.h @@ -28,8 +28,8 @@ typedef struct sgxlkl_shared_memory char** virtio_blk_dev_names; /* Host environment variables for optional import */ - char* const* env; - size_t envc; + char const* env; + size_t env_length; } sgxlkl_shared_memory_t; #endif /* SGXLKL_SHARED_MEMORY_H */ \ No newline at end of file diff --git a/src/main-oe/sgxlkl_run_oe.c b/src/main-oe/sgxlkl_run_oe.c index 2d2e3824e..16817ff2d 100644 --- a/src/main-oe/sgxlkl_run_oe.c +++ b/src/main-oe/sgxlkl_run_oe.c @@ -820,6 +820,18 @@ void set_clock_res(bool have_enclave_config) } } +static void set_host_env(char* const* envp) +{ + sgxlkl_host_state.shared_memory.env_length = 0; + for (char* const* env = envp; *env != 0; env++) + sgxlkl_host_state.shared_memory.env_length += strlen(*env) + 1; + + char* tmp = malloc(sgxlkl_host_state.shared_memory.env_length); + sgxlkl_host_state.shared_memory.env = tmp; + for (char* const* env = envp; *env != 0; env++) + tmp += sprintf(tmp, "%s", *env) + 1; +} + static void rdfsbase_sigill_handler(int sig, siginfo_t* si, void* data) { rdfsbase_caused_sigill = 1; @@ -1884,10 +1896,7 @@ int main(int argc, char* argv[], char* envp[]) bool have_enclave_config_file = enclave_config_path != NULL; set_clock_res(have_enclave_config_file); - sgxlkl_host_state.shared_memory.envc = 0; - for (char** env = envp; *env != 0; env++) - sgxlkl_host_state.shared_memory.envc++; - sgxlkl_host_state.shared_memory.env = envp; + set_host_env(envp); set_tls(have_enclave_config_file); register_hds(root_hd); register_net(); From 3e667b54d73ebec799fdf1c2f3a266b2cacabb9e Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Fri, 21 Aug 2020 14:12:53 +0000 Subject: [PATCH 09/15] Add overflow check Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_event_channel.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/enclave/enclave_event_channel.c b/src/enclave/enclave_event_channel.c index 01da0c9e2..0ae66e9bb 100644 --- a/src/enclave/enclave_event_channel.c +++ b/src/enclave/enclave_event_channel.c @@ -143,6 +143,7 @@ void initialize_enclave_event_channels(void) sgxlkl_enclave_state.shared_memory.enc_dev_config; const size_t _num_channels = sgxlkl_enclave_state.num_event_channel_state; + SGXLKL_ASSERT(SIZE_MAX / sizeof(enc_dev_config_t) >= _num_channels); sgxlkl_ensure_inside( enc_dev_config, sizeof(enc_dev_config_t) * _num_channels); From 0a32248773f559bd99aa3a87a6c58e220e89c889 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Tue, 15 Sep 2020 12:09:05 +0000 Subject: [PATCH 10/15] Remove unnecessary memory checks Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_timer.c | 2 -- src/lkl/setup.c | 7 ------- 2 files changed, 9 deletions(-) diff --git a/src/enclave/enclave_timer.c b/src/enclave/enclave_timer.c index c42d8bd6b..af6ed2e7d 100644 --- a/src/enclave/enclave_timer.c +++ b/src/enclave/enclave_timer.c @@ -30,8 +30,6 @@ uint64_t enclave_nanos() { struct timer_dev* t = sgxlkl_enclave_state.shared_memory.timer_dev_mem; - sgxlkl_ensure_outside(t, sizeof(struct timer_dev)); - uint64_t e = t->nanos; uint64_t i = internal_counter; if (e > i) diff --git a/src/lkl/setup.c b/src/lkl/setup.c index 48d6bd499..9fb30a255 100644 --- a/src/lkl/setup.c +++ b/src/lkl/setup.c @@ -1316,13 +1316,6 @@ static void init_enclave_clock() SGXLKL_VERBOSE("Setting enclave realtime clock\n"); struct timer_dev* t = shm->timer_dev_mem; - - if (!oe_is_outside_enclave(t, sizeof(struct timer_dev))) - { - sgxlkl_fail( - "timer_dev memory isn't outside of the enclave. Aborting.\n"); - } - struct lkl_timespec start_time; start_time.tv_sec = t->init_walltime_sec; start_time.tv_nsec = t->init_walltime_nsec; From 8fb5579b07d0c0fa26a9dbd34b9d0c66a55f4a68 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Tue, 15 Sep 2020 12:21:03 +0000 Subject: [PATCH 11/15] Add oe_lfence()s to memory boundary checks Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_oe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index 05eacd04b..6bf2af74f 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "openenclave/corelibc/oestring.h" @@ -283,12 +284,14 @@ static void _read_eeid_config() void sgxlkl_ensure_inside(const void* ptr, size_t sz) { + oe_lfence(); if (ptr != NULL && !oe_is_within_enclave(ptr, sz)) sgxlkl_fail("Unexpected: buffer is not in enclave memory.\n"); } void sgxlkl_ensure_outside(const void* ptr, size_t sz) { + oe_lfence(); if (ptr != NULL && !oe_is_outside_enclave(ptr, sz)) sgxlkl_fail("Unexpected: buffer is in enclave memory.\n"); } From 20cc35dc076c0ebafddea0dcc906cce6d00d149d Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Tue, 15 Sep 2020 12:28:28 +0000 Subject: [PATCH 12/15] Add local copy of host environment variable strings Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_oe.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index 6bf2af74f..4c194a0e4 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -394,6 +394,12 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) { sgxlkl_ensure_outside(henv, sizeof(char) * henv_len); + henv = oe_calloc_or_die( + henv_len, + sizeof(char), + "Could not allocate memory for host-imported environment variable " + "copy."); + size_t num_vars = 0; for (size_t i = 0; i < henv_len; i++) if (henv[i] == '\0') @@ -439,6 +445,7 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) /* Shared memory not needed anymore. */ sgxlkl_enclave_state.shared_memory.env = NULL; sgxlkl_enclave_state.shared_memory.env_length = 0; + oe_free((char*)henv); } /* Commit to the temporary copy */ From 1e58739cec8763d41604a276de283b53a768f7b5 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Tue, 15 Sep 2020 12:31:49 +0000 Subject: [PATCH 13/15] Add comments to host env import code Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_oe.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index 4c194a0e4..5ee582d2c 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -411,6 +411,8 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) "Could not allocate memory for host-imported environment " "variables\n"); + /* Import only those variable settings that are specified to be imported + * in cfg->host_import_env and ignore the others. */ size_t begin = 0; sgxlkl_enclave_state.num_env_imported = 0; for (size_t i = 0; i < henv_len; i++) @@ -420,6 +422,7 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) const char* henv_i = henv + begin; size_t henv_i_len = i - begin; + /* Find setting in cfg->host_import_env */ for (size_t j = 0; j < cfg->num_host_import_env; j++) { const char* name = cfg->host_import_env[j]; From 11ed88e1bfdbaa873fba900bedb4f51c5a4c23e5 Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Tue, 15 Sep 2020 15:49:07 +0000 Subject: [PATCH 14/15] Fix import of block device names Signed-off-by: Christoph M. Wintersteiger --- src/enclave/enclave_init.c | 3 +- src/enclave/enclave_oe.c | 62 +++++++++++++++++++---------- src/host_interface/virtio_blkdev.c | 2 - src/include/enclave/enclave_state.h | 3 ++ src/include/shared/shared_memory.h | 3 +- src/main-oe/sgxlkl_run_oe.c | 17 +++++++- 6 files changed, 63 insertions(+), 27 deletions(-) diff --git a/src/enclave/enclave_init.c b/src/enclave/enclave_init.c index 4c1df4805..f4e671ec9 100644 --- a/src/enclave/enclave_init.c +++ b/src/enclave/enclave_init.c @@ -55,7 +55,8 @@ static void find_and_mount_disks() for (int j = 0; j < shm->num_virtio_blk_dev && !found; j++) { if (oe_strcmp( - cfg_disk->destination, shm->virtio_blk_dev_names[j]) == 0) + cfg_disk->destination, estate->virtio_blk_dev_names[j]) == + 0) { estate->disk_state[i + 1].host_disk_index = j; found = true; diff --git a/src/enclave/enclave_oe.c b/src/enclave/enclave_oe.c index 5ee582d2c..5989f9d54 100644 --- a/src/enclave/enclave_oe.c +++ b/src/enclave/enclave_oe.c @@ -366,25 +366,43 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) enc->num_virtio_blk_dev, sizeof(struct virtio_dev*), "Could not allocate memory for virtio block devices\n"); - enc->virtio_blk_dev_names = oe_calloc_or_die( - enc->num_virtio_blk_dev, - sizeof(char*), - "Could not allocate memory for virtio block devices\n"); for (size_t i = 0; i < enc->num_virtio_blk_dev; i++) { enc->virtio_blk_dev_mem[i] = host->virtio_blk_dev_mem[i]; sgxlkl_ensure_outside( enc->virtio_blk_dev_mem[i], sizeof(struct virtio_dev)); + } + + size_t names_length = host->virtio_blk_dev_names_length; + sgxlkl_ensure_outside(host->virtio_blk_dev_names, names_length); + char* names_tmp = oe_calloc_or_die( + names_length, + sizeof(char), + "Could not allocate temporary memory for block device names\n"); + oe_memcpy_s( + names_tmp, names_length, host->virtio_blk_dev_names, names_length); + + if (names_tmp[names_length - 1] != '\0') + sgxlkl_fail("unterminated virtio block device name\n"); + + char** name_array_tmp = oe_calloc_or_die( + enc->num_virtio_blk_dev, + sizeof(char*), + "Could not allocate temporary memory for block device name " + "array\n"); - const char* name = host->virtio_blk_dev_names[i]; - size_t name_len = oe_strlen(name) + 1; - sgxlkl_ensure_outside(name, name_len); - enc->virtio_blk_dev_names[i] = oe_calloc_or_die( - name_len, - sizeof(char), - "Could not allocate memory for virtio block device name\n"); - memcpy(enc->virtio_blk_dev_names[i], name, name_len); + sgxlkl_enclave_state.virtio_blk_dev_names = oe_calloc_or_die( + enc->num_virtio_blk_dev, + sizeof(char*), + "Could not allocate memory for virtio block device names\n"); + const char* name_p = names_tmp; + for (size_t i = 0; i < enc->num_virtio_blk_dev; i++) + { + name_array_tmp[i] = oe_strdup(name_p); + name_p += oe_strlen(name_p) + 1; } + free(names_tmp); + sgxlkl_enclave_state.virtio_blk_dev_names = name_array_tmp; } /* Import environment variables from the host */ @@ -394,15 +412,16 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) { sgxlkl_ensure_outside(henv, sizeof(char) * henv_len); - henv = oe_calloc_or_die( + char* henv_copy = oe_calloc_or_die( henv_len, sizeof(char), "Could not allocate memory for host-imported environment variable " "copy."); + oe_memcpy_s(henv_copy, henv_len, henv, henv_len); size_t num_vars = 0; for (size_t i = 0; i < henv_len; i++) - if (henv[i] == '\0') + if (henv_copy[i] == '\0') num_vars++; char** tmp = oe_calloc_or_die( @@ -417,9 +436,9 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) sgxlkl_enclave_state.num_env_imported = 0; for (size_t i = 0; i < henv_len; i++) { - if (henv[i] == '\0') + if (henv_copy[i] == '\0') { - const char* henv_i = henv + begin; + const char* henv_i = henv_copy + begin; size_t henv_i_len = i - begin; /* Find setting in cfg->host_import_env */ @@ -448,7 +467,7 @@ static void _copy_shared_memory(const sgxlkl_shared_memory_t* host) /* Shared memory not needed anymore. */ sgxlkl_enclave_state.shared_memory.env = NULL; sgxlkl_enclave_state.shared_memory.env_length = 0; - oe_free((char*)henv); + oe_free((char*)henv_copy); } /* Commit to the temporary copy */ @@ -490,11 +509,7 @@ static void _free_shared_memory() } oe_free(shm->enc_dev_config); - for (size_t i = 0; i < shm->num_virtio_blk_dev; i++) - oe_free(shm->virtio_blk_dev_names[i]); - oe_free(shm->virtio_blk_dev_mem); - oe_free(shm->virtio_blk_dev_names); } int sgxlkl_enclave_init(const sgxlkl_shared_memory_t* shared_memory) @@ -550,6 +565,7 @@ int sgxlkl_enclave_init(const sgxlkl_shared_memory_t* shared_memory) void sgxlkl_free_enclave_state() { sgxlkl_enclave_state_t* state = &sgxlkl_enclave_state; + size_t num_blk_devs = state->shared_memory.num_virtio_blk_dev; state->elf64_stack.argc = 0; oe_free(state->elf64_stack.argv); /* includes envp/auxv */ @@ -564,6 +580,10 @@ void sgxlkl_free_enclave_state() state->num_event_channel_state = 0; oe_free(state->event_channel_state); + for (size_t i = 0; i < num_blk_devs; i++) + oe_free(state->virtio_blk_dev_names[i]); + oe_free((char**)state->virtio_blk_dev_names); + for (size_t i = 0; i < state->num_env_imported; i++) oe_free(state->env_imported[i]); oe_free((void*)state->env_imported); diff --git a/src/host_interface/virtio_blkdev.c b/src/host_interface/virtio_blkdev.c index e62c15db2..c0f32c7bd 100644 --- a/src/host_interface/virtio_blkdev.c +++ b/src/host_interface/virtio_blkdev.c @@ -163,8 +163,6 @@ int blk_device_init( sgxlkl_host_state.shared_memory.virtio_blk_dev_mem[disk_index] = &host_blk_device->dev; - sgxlkl_host_state.shared_memory.virtio_blk_dev_names[disk_index] = - strdup(disk->root_config ? "/" : disk->mount_config->destination); return 0; } diff --git a/src/include/enclave/enclave_state.h b/src/include/enclave/enclave_state.h index 49c3781e2..b6eea5407 100644 --- a/src/include/enclave/enclave_state.h +++ b/src/include/enclave/enclave_state.h @@ -66,6 +66,9 @@ typedef struct sgxlkl_enclave_state size_t num_event_channel_state; enc_dev_state_t* event_channel_state; + /* Block device names */ + char* const* virtio_blk_dev_names; + /* Environment variables imported from the host */ size_t num_env_imported; char* const* env_imported; diff --git a/src/include/shared/shared_memory.h b/src/include/shared/shared_memory.h index 9e6189282..0c77ea85d 100644 --- a/src/include/shared/shared_memory.h +++ b/src/include/shared/shared_memory.h @@ -25,7 +25,8 @@ typedef struct sgxlkl_shared_memory /* Shared memory for virtio block devices */ size_t num_virtio_blk_dev; struct virtio_dev** virtio_blk_dev_mem; - char** virtio_blk_dev_names; + char const* virtio_blk_dev_names; + size_t virtio_blk_dev_names_length; /* Host environment variables for optional import */ char const* env; diff --git a/src/main-oe/sgxlkl_run_oe.c b/src/main-oe/sgxlkl_run_oe.c index 16817ff2d..43861be6a 100644 --- a/src/main-oe/sgxlkl_run_oe.c +++ b/src/main-oe/sgxlkl_run_oe.c @@ -1053,6 +1053,7 @@ static void register_hds(char* root_hd) if (!shm->virtio_blk_dev_mem || !shm->virtio_blk_dev_names) sgxlkl_host_fail("out of memory\n"); + size_t names_length = 0; for (size_t i = 0; i < num_disks; i++) { sgxlkl_host_disk_state_t* disk = &sgxlkl_host_state.disks[i]; @@ -1076,9 +1077,21 @@ static void register_hds(char* root_hd) disk->mount_config->destination, disk->mount_config->readonly, i); - strcpy(shm->virtio_blk_dev_names[i], dest); } + names_length += strlen(dest) + 1; + } + + char* tmp = malloc(names_length); + if (!tmp) + sgxlkl_host_fail("out of memory\n"); + char* p = tmp; + for (size_t i = 0; i < num_disks; i++) + { + sgxlkl_host_disk_state_t* disk = &sgxlkl_host_state.disks[i]; + char* dest = disk->root_config ? "/" : disk->mount_config->destination; + p += sprintf(p, "%s", dest) + 1; } + shm->virtio_blk_dev_names = tmp; } static void register_net() @@ -1139,7 +1152,7 @@ static void sgxlkl_cleanup(void) close(sgxlkl_host_state.disks[--sgxlkl_host_state.num_disks].fd); } free(sgxlkl_host_state.shared_memory.virtio_blk_dev_mem); - free(sgxlkl_host_state.shared_memory.virtio_blk_dev_names); + free((char*)sgxlkl_host_state.shared_memory.virtio_blk_dev_names); } static void serialize_ucontext( From e33f1c30d3b824c5ebdfa0e08ca560d14943fb0b Mon Sep 17 00:00:00 2001 From: "Christoph M. Wintersteiger" Date: Fri, 25 Sep 2020 12:34:05 +0000 Subject: [PATCH 15/15] Fix block device name import string length Signed-off-by: Christoph M. Wintersteiger --- src/main-oe/sgxlkl_run_oe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main-oe/sgxlkl_run_oe.c b/src/main-oe/sgxlkl_run_oe.c index 43861be6a..c07c190e5 100644 --- a/src/main-oe/sgxlkl_run_oe.c +++ b/src/main-oe/sgxlkl_run_oe.c @@ -1079,7 +1079,7 @@ static void register_hds(char* root_hd) i); } names_length += strlen(dest) + 1; - } + } char* tmp = malloc(names_length); if (!tmp) @@ -1092,6 +1092,7 @@ static void register_hds(char* root_hd) p += sprintf(p, "%s", dest) + 1; } shm->virtio_blk_dev_names = tmp; + shm->virtio_blk_dev_names_length = names_length; } static void register_net()