From 11858ac8343f4e658dbe6179afcb083d140d0456 Mon Sep 17 00:00:00 2001 From: g2flyer Date: Thu, 2 May 2024 09:38:07 -0700 Subject: [PATCH] fixup! Single-process-lifetime rollback protection for protected files (WIP) Signed-off-by: g2flyer --- libos/include/libos_fs_encrypted.h | 2 +- libos/src/fs/chroot/encrypted.c | 6 ++-- libos/src/fs/libos_fs_encrypted.c | 13 ++++---- libos/test/regression/rename_unlink.c | 44 +++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/libos/include/libos_fs_encrypted.h b/libos/include/libos_fs_encrypted.h index f1728035f3..ef180fd30e 100644 --- a/libos/include/libos_fs_encrypted.h +++ b/libos/include/libos_fs_encrypted.h @@ -190,7 +190,7 @@ int encrypted_file_get(struct libos_encrypted_file* enc); * * This decreases `use_count`, and closes the file if it reaches 0. */ -void encrypted_file_put(struct libos_encrypted_file* enc); +void encrypted_file_put(struct libos_encrypted_file* enc, bool fs_reachable); /* * \brief Flush pending writes to an encrypted file. diff --git a/libos/src/fs/chroot/encrypted.c b/libos/src/fs/chroot/encrypted.c index 7b771aee43..de7d0f5f06 100644 --- a/libos/src/fs/chroot/encrypted.c +++ b/libos/src/fs/chroot/encrypted.c @@ -200,7 +200,7 @@ static int chroot_encrypted_lookup(struct libos_dentry* dent) { } } else { ret = encrypted_file_get_size(enc, &size); - encrypted_file_put(enc); + encrypted_file_put(enc, true); if (ret < 0) { encrypted_file_destroy(enc); @@ -390,7 +390,7 @@ static int chroot_encrypted_rename(struct libos_dentry* old, struct libos_dentry goto out; ret = encrypted_file_rename(enc, new_uri); - encrypted_file_put(enc); + encrypted_file_put(enc, true); out: unlock(&old->inode->lock); free(new_uri); @@ -476,7 +476,7 @@ static int chroot_encrypted_close(struct libos_handle* hdl) { assert(enc); lock(&hdl->inode->lock); - encrypted_file_put(enc); + encrypted_file_put(enc, hdl->inode == hdl->dentry->inode); unlock(&hdl->inode->lock); return 0; diff --git a/libos/src/fs/libos_fs_encrypted.c b/libos/src/fs/libos_fs_encrypted.c index 4c33b28ed6..67126ba5d5 100644 --- a/libos/src/fs/libos_fs_encrypted.c +++ b/libos/src/fs/libos_fs_encrypted.c @@ -284,12 +284,13 @@ int parse_pf_key(const char* key_str, pf_key_t* pf_key) { return 0; } -static void encrypted_file_internal_close(struct libos_encrypted_file* enc) { +static void encrypted_file_internal_close(struct libos_encrypted_file* enc, bool fs_reachable) { assert(enc->pf); pf_mac_t closing_root_gmac; pf_status_t pfs = pf_close(enc->pf, &closing_root_gmac); - log_debug("file '%s' closed with MAC=" MAC_PRINTF_PATTERN, enc->norm_path, + log_debug("%sreachable file '%s' closed with MAC=" MAC_PRINTF_PATTERN, + (fs_reachable ? "" : "un"), enc->norm_path, MAC_PRINTF_ARGS(closing_root_gmac)); // TODO (MST): remove me eventually? lock(&(enc->volume->files_state_map_lock)); struct libos_encrypted_volume_state_map* file_state = NULL; @@ -300,9 +301,7 @@ static void encrypted_file_internal_close(struct libos_encrypted_file* enc) { file_state->state = PF_FILE_ERROR; pf_set_corrupted(enc->pf); } else { - // TODO (MST): Below also has to rule out that our file is stale, i.e., somebody has renamed - // a file to our own original file name - if (file_state->state != PF_FILE_DELETED) { + if (file_state->state != PF_FILE_DELETED && fs_reachable) { // TODO (MST): omit below if read-only file? memcpy(file_state->last_seen_root_gmac, closing_root_gmac, sizeof(pf_mac_t)); file_state->state = PF_FILE_CLOSED; @@ -617,12 +616,12 @@ int encrypted_file_get(struct libos_encrypted_file* enc) { return 0; } -void encrypted_file_put(struct libos_encrypted_file* enc) { +void encrypted_file_put(struct libos_encrypted_file* enc, bool fs_reachable) { assert(enc->use_count > 0); assert(enc->pf); enc->use_count--; if (enc->use_count == 0) { - encrypted_file_internal_close(enc); + encrypted_file_internal_close(enc, fs_reachable); } } diff --git a/libos/test/regression/rename_unlink.c b/libos/test/regression/rename_unlink.c index 52241dfe4d..ca7f671eb6 100644 --- a/libos/test/regression/rename_unlink.c +++ b/libos/test/regression/rename_unlink.c @@ -156,6 +156,7 @@ static void test_rename_replace(const char* path1, const char* path2) { err(1, "rename"); should_not_exist(path1); + should_exist(path2, message1_len); /* We expect `fd` to still point to old data, even though we replaced the file under its path */ @@ -177,6 +178,48 @@ static void test_rename_replace(const char* path1, const char* path2) { err(1, "unlink %s", path2); } +static void test_rename_follow(const char* path1, const char* path2) { + printf("%s...\n", __func__); + + int fd = create_file(path1, message1, message1_len); + + if (fd < 0) + err(1, "open %s", path1); + + if (rename(path1, path2) != 0) + err(1, "rename"); + + should_not_exist(path1); + should_exist(path2, message1_len); + + if (lseek(fd, 0, SEEK_SET) != 0) + err(1, "lseek"); + + ssize_t n = posix_fd_write(fd, message2, message2_len); + if (n < 0) + errx(1, "posix_fd_write failed"); + if ((size_t)n != message2_len) + errx(1, "wrote less bytes than expected"); + + should_contain("file opened before it's renamed", fd, message2, message2_len); + + if (close(fd) != 0) + err(1, "close %s", path2); + + fd = open(path2, O_RDONLY, 0); + if (fd < 0) + err(1, "open %s", path2); + + /* We expect `fd` to point to new data, even though we changed data via old fd after rename */ + should_contain("file opened after it's renamed", fd, message2, message2_len); + + if (close(fd) != 0) + err(1, "close %s", path2); + + if (unlink(path2) != 0) + err(1, "unlink %s", path2); +} + static void test_rename_open_file(const char* path1, const char* path2) { printf("%s...\n", __func__); @@ -271,6 +314,7 @@ int main(int argc, char* argv[]) { test_rename_same_file(path1); test_simple_rename(path1, path2); test_rename_replace(path1, path2); + test_rename_follow(path1, path2); test_rename_open_file(path1, path2); test_unlink_and_recreate(path1); test_unlink_and_write(path1);