Skip to content

Commit

Permalink
[common] Add explicit fsync callback to Protected Files
Browse files Browse the repository at this point in the history
Previously, Protected Files didn't have a way to ensure that the data is
physically stored on disk (aka fsync). This is in contrast to the "file
flush" operation, which only flushes user-space buffers to the OS kernel
buffers. (Gramine confusingly uses the term "flush" to mean flush and
fsync, depending on the context.)

This explicit fsync is important in e.g. persistent database workloads.
It was detected on a RocksDB workload.

Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
  • Loading branch information
dimakuv authored and mkow committed Feb 9, 2024
1 parent e740728 commit 4c6be0a
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 6 deletions.
19 changes: 17 additions & 2 deletions common/src/protected_files/protected_files.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
/* Host callbacks */
static pf_read_f g_cb_read = NULL;
static pf_write_f g_cb_write = NULL;
static pf_fsync_f g_cb_fsync = NULL;
static pf_truncate_f g_cb_truncate = NULL;
static pf_debug_f g_cb_debug = NULL;

Expand Down Expand Up @@ -1170,12 +1171,14 @@ static bool ipf_close(pf_context_t* pf) {

// public API

void pf_set_callbacks(pf_read_f read_f, pf_write_f write_f, pf_truncate_f truncate_f,
pf_aes_cmac_f aes_cmac_f, pf_aes_gcm_encrypt_f aes_gcm_encrypt_f,
void pf_set_callbacks(pf_read_f read_f, pf_write_f write_f, pf_fsync_f fsync_f,
pf_truncate_f truncate_f, pf_aes_cmac_f aes_cmac_f,
pf_aes_gcm_encrypt_f aes_gcm_encrypt_f,
pf_aes_gcm_decrypt_f aes_gcm_decrypt_f, pf_random_f random_f,
pf_debug_f debug_f) {
g_cb_read = read_f;
g_cb_write = write_f;
g_cb_fsync = fsync_f;
g_cb_truncate = truncate_f;
g_cb_aes_cmac = aes_cmac_f;
g_cb_aes_gcm_encrypt = aes_gcm_encrypt_f;
Expand Down Expand Up @@ -1350,6 +1353,18 @@ pf_status_t pf_flush(pf_context_t* pf) {

if (!ipf_internal_flush(pf))
return pf->last_error;

/* Perform a synchronous fsync in this "wrapper" function instead of `ipf_internal_flush()`.
* This is because we want to fsync only when explicitly asked for: `pf_flush()` is called when
* the app issues an actual fsync()/fdatasync() syscalls. The internal `ipf_internal_flush()` is
* more broadly used: it's called when file contents must be flushed to the host OS (e.g. during
* the rename operation). */
pf_status_t status = g_cb_fsync(pf->file);
if (PF_FAILURE(status)) {
pf->last_error = status;
return pf->last_error;
}

return PF_STATUS_SUCCESS;
}

Expand Down
15 changes: 13 additions & 2 deletions common/src/protected_files/protected_files.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ typedef pf_status_t (*pf_read_f)(pf_handle_t handle, void* buffer, uint64_t offs
typedef pf_status_t (*pf_write_f)(pf_handle_t handle, const void* buffer, uint64_t offset,
size_t size);

/*!
* \brief File sync callback.
*
* \param handle File handle.
*
* \returns PF status.
*/
typedef pf_status_t (*pf_fsync_f)(pf_handle_t handle);

/*!
* \brief File truncate callback.
*
Expand Down Expand Up @@ -172,6 +181,7 @@ typedef pf_status_t (*pf_random_f)(uint8_t* buffer, size_t size);
*
* \param read_f File read callback.
* \param write_f File write callback.
* \param fsync_f File sync callback.
* \param truncate_f File truncate callback.
* \param aes_cmac_f AES-CMAC callback.
* \param aes_gcm_encrypt_f AES-GCM encrypt callback.
Expand All @@ -181,8 +191,9 @@ typedef pf_status_t (*pf_random_f)(uint8_t* buffer, size_t size);
*
* Must be called before any actual APIs.
*/
void pf_set_callbacks(pf_read_f read_f, pf_write_f write_f, pf_truncate_f truncate_f,
pf_aes_cmac_f aes_cmac_f, pf_aes_gcm_encrypt_f aes_gcm_encrypt_f,
void pf_set_callbacks(pf_read_f read_f, pf_write_f write_f, pf_fsync_f fsync_f,
pf_truncate_f truncate_f, pf_aes_cmac_f aes_cmac_f,
pf_aes_gcm_encrypt_f aes_gcm_encrypt_f,
pf_aes_gcm_decrypt_f aes_gcm_decrypt_f, pf_random_f random_f,
pf_debug_f debug_f);

Expand Down
14 changes: 13 additions & 1 deletion libos/src/fs/libos_fs_encrypted.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ static pf_status_t cb_truncate(pf_handle_t handle, uint64_t size) {
return PF_STATUS_SUCCESS;
}

static pf_status_t cb_fsync(pf_handle_t handle) {
PAL_HANDLE pal_handle = (PAL_HANDLE)handle;

int ret = PalStreamFlush(pal_handle);
if (ret < 0) {
log_warning("PalStreamFlush failed: %s", pal_strerror(ret));
return PF_STATUS_CALLBACK_FAILED;
}

return PF_STATUS_SUCCESS;
}

static pf_status_t cb_aes_cmac(const pf_key_t* key, const void* input, size_t input_size,
pf_mac_t* mac) {
int ret = lib_AESCMAC((const uint8_t*)key, sizeof(*key), input, input_size, (uint8_t*)mac,
Expand Down Expand Up @@ -272,7 +284,7 @@ int init_encrypted_files(void) {
if (!create_lock(&g_keys_lock))
return -ENOMEM;

pf_set_callbacks(&cb_read, &cb_write, &cb_truncate,
pf_set_callbacks(&cb_read, &cb_write, &cb_fsync, &cb_truncate,
&cb_aes_cmac, &cb_aes_gcm_encrypt, &cb_aes_gcm_decrypt,
&cb_random, cb_debug_ptr);

Expand Down
14 changes: 13 additions & 1 deletion tools/sgx/common/pf_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,18 @@ static pf_status_t linux_write(pf_handle_t handle, const void* buffer, uint64_t
return PF_STATUS_SUCCESS;
}

static pf_status_t linux_fsync(pf_handle_t handle) {
int fd = *(int*)handle;
DBG("linux_fsync: fd %d\n", fd);
int ret = fsync(fd);
if (ret < 0) {
ERROR("fsync failed: %s\n", strerror(errno));
return PF_STATUS_CALLBACK_FAILED;
}

return PF_STATUS_SUCCESS;
}

static pf_status_t linux_truncate(pf_handle_t handle, uint64_t size) {
int fd = *(int*)handle;
DBG("linux_truncate: fd %d, size %zu\n", fd, size);
Expand Down Expand Up @@ -214,7 +226,7 @@ static int pf_set_linux_callbacks(pf_debug_f debug_f) {
return -1;
}

pf_set_callbacks(linux_read, linux_write, linux_truncate, mbedtls_aes_cmac,
pf_set_callbacks(linux_read, linux_write, linux_fsync, linux_truncate, mbedtls_aes_cmac,
mbedtls_aes_gcm_encrypt, mbedtls_aes_gcm_decrypt, mbedtls_random, debug_f);
return 0;
}
Expand Down

0 comments on commit 4c6be0a

Please sign in to comment.