Skip to content

Commit

Permalink
fixup! Single-process-lifetime rollback protection for protected file…
Browse files Browse the repository at this point in the history
…s (WIP)

Signed-off-by: g2flyer <[email protected]>
  • Loading branch information
g2flyer committed May 2, 2024
1 parent 6eca516 commit 11858ac
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 11 deletions.
2 changes: 1 addition & 1 deletion libos/include/libos_fs_encrypted.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions libos/src/fs/chroot/encrypted.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 6 additions & 7 deletions libos/src/fs/libos_fs_encrypted.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
44 changes: 44 additions & 0 deletions libos/test/regression/rename_unlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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__);

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 11858ac

Please sign in to comment.