Skip to content

Commit 79890bf

Browse files
committed
fixup! Single-process-lifetime rollback protection for protected files (WIP)
Signed-off-by: g2flyer <[email protected]>
1 parent 8e01495 commit 79890bf

File tree

4 files changed

+54
-11
lines changed

4 files changed

+54
-11
lines changed

libos/include/libos_fs_encrypted.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ int encrypted_file_get(struct libos_encrypted_file* enc);
190190
*
191191
* This decreases `use_count`, and closes the file if it reaches 0.
192192
*/
193-
void encrypted_file_put(struct libos_encrypted_file* enc);
193+
void encrypted_file_put(struct libos_encrypted_file* enc, bool fs_reachable);
194194

195195
/*
196196
* \brief Flush pending writes to an encrypted file.

libos/src/fs/chroot/encrypted.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ static int chroot_encrypted_lookup(struct libos_dentry* dent) {
200200
}
201201
} else {
202202
ret = encrypted_file_get_size(enc, &size);
203-
encrypted_file_put(enc);
203+
encrypted_file_put(enc, true);
204204

205205
if (ret < 0) {
206206
encrypted_file_destroy(enc);
@@ -390,7 +390,7 @@ static int chroot_encrypted_rename(struct libos_dentry* old, struct libos_dentry
390390
goto out;
391391

392392
ret = encrypted_file_rename(enc, new_uri);
393-
encrypted_file_put(enc);
393+
encrypted_file_put(enc, true);
394394
out:
395395
unlock(&old->inode->lock);
396396
free(new_uri);
@@ -476,7 +476,7 @@ static int chroot_encrypted_close(struct libos_handle* hdl) {
476476
assert(enc);
477477

478478
lock(&hdl->inode->lock);
479-
encrypted_file_put(enc);
479+
encrypted_file_put(enc, hdl->inode == hdl->dentry->inode);
480480
unlock(&hdl->inode->lock);
481481

482482
return 0;

libos/src/fs/libos_fs_encrypted.c

+6-7
Original file line numberDiff line numberDiff line change
@@ -284,12 +284,13 @@ int parse_pf_key(const char* key_str, pf_key_t* pf_key) {
284284
return 0;
285285
}
286286

287-
static void encrypted_file_internal_close(struct libos_encrypted_file* enc) {
287+
static void encrypted_file_internal_close(struct libos_encrypted_file* enc, bool fs_reachable) {
288288
assert(enc->pf);
289289

290290
pf_mac_t closing_root_gmac;
291291
pf_status_t pfs = pf_close(enc->pf, &closing_root_gmac);
292-
log_debug("file '%s' closed with MAC=" MAC_PRINTF_PATTERN, enc->norm_path,
292+
log_debug("%sreachable file '%s' closed with MAC=" MAC_PRINTF_PATTERN,
293+
(fs_reachable ? "" : "un"), enc->norm_path,
293294
MAC_PRINTF_ARGS(closing_root_gmac)); // TODO (MST): remove me eventually?
294295
lock(&(enc->volume->files_state_map_lock));
295296
struct libos_encrypted_volume_state_map* file_state = NULL;
@@ -300,9 +301,7 @@ static void encrypted_file_internal_close(struct libos_encrypted_file* enc) {
300301
file_state->state = PF_FILE_ERROR;
301302
pf_set_corrupted(enc->pf);
302303
} else {
303-
// TODO (MST): Below also has to rule out that our file is stale, i.e., somebody has renamed
304-
// a file to our own original file name
305-
if (file_state->state != PF_FILE_DELETED) {
304+
if (file_state->state != PF_FILE_DELETED && fs_reachable) {
306305
// TODO (MST): omit below if read-only file?
307306
memcpy(file_state->last_seen_root_gmac, closing_root_gmac, sizeof(pf_mac_t));
308307
file_state->state = PF_FILE_CLOSED;
@@ -617,12 +616,12 @@ int encrypted_file_get(struct libos_encrypted_file* enc) {
617616
return 0;
618617
}
619618

620-
void encrypted_file_put(struct libos_encrypted_file* enc) {
619+
void encrypted_file_put(struct libos_encrypted_file* enc, bool fs_reachable) {
621620
assert(enc->use_count > 0);
622621
assert(enc->pf);
623622
enc->use_count--;
624623
if (enc->use_count == 0) {
625-
encrypted_file_internal_close(enc);
624+
encrypted_file_internal_close(enc, fs_reachable);
626625
}
627626
}
628627

libos/test/regression/rename_unlink.c

+44
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ static void test_rename_replace(const char* path1, const char* path2) {
156156
err(1, "rename");
157157

158158
should_not_exist(path1);
159+
159160
should_exist(path2, message1_len);
160161

161162
/* 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) {
177178
err(1, "unlink %s", path2);
178179
}
179180

181+
static void test_rename_follow(const char* path1, const char* path2) {
182+
printf("%s...\n", __func__);
183+
184+
int fd = create_file(path1, message1, message1_len);
185+
186+
if (fd < 0)
187+
err(1, "open %s", path1);
188+
189+
if (rename(path1, path2) != 0)
190+
err(1, "rename");
191+
192+
should_not_exist(path1);
193+
should_exist(path2, message1_len);
194+
195+
if (lseek(fd, 0, SEEK_SET) != 0)
196+
err(1, "lseek");
197+
198+
ssize_t n = posix_fd_write(fd, message2, message2_len);
199+
if (n < 0)
200+
errx(1, "posix_fd_write failed");
201+
if ((size_t)n != message2_len)
202+
errx(1, "wrote less bytes than expected");
203+
204+
should_contain("file opened before it's renamed", fd, message2, message2_len);
205+
206+
if (close(fd) != 0)
207+
err(1, "close %s", path2);
208+
209+
fd = open(path2, O_RDONLY, 0);
210+
if (fd < 0)
211+
err(1, "open %s", path2);
212+
213+
/* We expect `fd` to point to new data, even though we changed data via old fd after rename */
214+
should_contain("file opened after it's renamed", fd, message2, message2_len);
215+
216+
if (close(fd) != 0)
217+
err(1, "close %s", path2);
218+
219+
if (unlink(path2) != 0)
220+
err(1, "unlink %s", path2);
221+
}
222+
180223
static void test_rename_open_file(const char* path1, const char* path2) {
181224
printf("%s...\n", __func__);
182225

@@ -271,6 +314,7 @@ int main(int argc, char* argv[]) {
271314
test_rename_same_file(path1);
272315
test_simple_rename(path1, path2);
273316
test_rename_replace(path1, path2);
317+
test_rename_follow(path1, path2);
274318
test_rename_open_file(path1, path2);
275319
test_unlink_and_recreate(path1);
276320
test_unlink_and_write(path1);

0 commit comments

Comments
 (0)