From 2ff32d2dfb879e38cc99f4c759683e95bd11faef Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 8 Jul 2018 14:21:29 -0500 Subject: [PATCH] Fixed bug where globals were poisoning move commits The issue lies in the reuse of the id field for globals. Before globals, the only tags with a non-null (0x3ff) id field were names, structs, and other file-specific metadata. But globals are also using this field for the indirect delete, since otherwise the globals structure would be very unaligned (74-bits long). To make matters worse, the id field for globals contains the delta used to reconstruct the globals at mount time. Which means the id field could take on very absurd values and break the dir fetch logic if we're not careful. Solution is to use the scope portion of the type field where necessary, although unforunately this does add some code cost. --- lfs.c | 18 +++++++++++++++++ lfs.h | 2 +- tests/test_move.sh | 50 +++++++++++++++++++++++++++++++++++----------- 3 files changed, 57 insertions(+), 13 deletions(-) diff --git a/lfs.c b/lfs.c index 44f99b1..ba3cfb1 100644 --- a/lfs.c +++ b/lfs.c @@ -664,6 +664,13 @@ static int lfs_commit_movescan(lfs_t *lfs, void *p, lfs_mattr_t attr) { return 0; } + // TODO AHHHH, scopes, what about user scope above? + if (lfs_tag_scope(attr.tag) == LFS_SCOPE_FS || + lfs_tag_scope(attr.tag) == LFS_SCOPE_DIR) { + // ignore non-matching ids + return 0; + } + if (lfs_tag_id(attr.tag) != move->id.from) { // ignore non-matching ids return 0; @@ -1023,6 +1030,9 @@ static int lfs_dir_compact(lfs_t *lfs, lfs_mdir_t *dir, lfs_mattrlist_t *list, // There's nothing special about our global delta, so feed it back // into the global global delta + // TODO IMMENSE HMM globals get bleed into from above, need to be fixed after commits due to potential moves + lfs_globals_t gtemp = dir->globals; // TODO hmm, why did we have different variables then? + lfs->diff = lfs_globals_xor(&lfs->diff, &dir->globals); dir->globals = (lfs_globals_t){0}; @@ -1196,6 +1206,8 @@ relocate: lfs->diff = (lfs_globals_t){0}; } + lfs->globals = lfs_globals_xor(&lfs->globals, >emp); // TODO hmm, why did we have different variables then? + return 0; } @@ -1244,6 +1256,8 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir, lfs_mattrlist_t *list) { // successful commit, lets update dir dir->off = commit.off; dir->etag = commit.ptag; +// // TODO hm +// dir->globals = lfs_globals_xor(&dir->globals, &lfs->diff); lfs->globals = lfs_globals_xor(&lfs->globals, &lfs->diff); lfs->diff = (lfs_globals_t){0}; break; @@ -2954,6 +2968,10 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { return res; } + // TODO test for global state stealing? + // steal global state + lfs->globals = lfs_globals_xor(&lfs->globals, &prevdir.globals); + LFS_ASSERT(res); // must have pred newcwd.tail[0] = prevdir.tail[0]; newcwd.tail[1] = prevdir.tail[1]; diff --git a/lfs.h b/lfs.h index 1a9e2dd..ff29e84 100644 --- a/lfs.h +++ b/lfs.h @@ -129,7 +129,7 @@ enum lfs_type { // internal sources LFS_FROM_REGION = 0x000, LFS_FROM_DISK = 0x200, - LFS_FROM_MOVE = 0x0ff, + LFS_FROM_MOVE = 0x03f, }; // File open flags diff --git a/tests/test_move.sh b/tests/test_move.sh index 11dfecc..5b851c5 100755 --- a/tests/test_move.sh +++ b/tests/test_move.sh @@ -109,7 +109,7 @@ tests/test.py << TEST TEST echo "--- Move file after corrupt ---" -tests/test.py -s << TEST +tests/test.py << TEST lfs_mount(&lfs, &cfg) => 0; lfs_rename(&lfs, "c/hello", "d/hello") => 0; lfs_unmount(&lfs) => 0; @@ -166,7 +166,7 @@ tests/test.py << TEST lfs_rename(&lfs, "b/hi", "c/hi") => 0; lfs_unmount(&lfs) => 0; TEST -rm -v blocks/7 +truncate -s-11 blocks/7 tests/test.py << TEST lfs_mount(&lfs, &cfg) => 0; lfs_dir_open(&lfs, &dir[0], "b") => 0; @@ -182,8 +182,6 @@ tests/test.py << TEST lfs_dir_read(&lfs, &dir[0], &info) => 1; strcmp(info.name, "..") => 0; lfs_dir_read(&lfs, &dir[0], &info) => 1; - strcmp(info.name, "hello") => 0; - lfs_dir_read(&lfs, &dir[0], &info) => 1; strcmp(info.name, "hi") => 0; lfs_dir_read(&lfs, &dir[0], &info) => 0; lfs_unmount(&lfs) => 0; @@ -195,8 +193,8 @@ tests/test.py << TEST lfs_rename(&lfs, "c/hi", "d/hi") => 0; lfs_unmount(&lfs) => 0; TEST -rm -v blocks/9 -rm -v blocks/a +truncate -s-11 blocks/9 +truncate -s-11 blocks/b tests/test.py << TEST lfs_mount(&lfs, &cfg) => 0; lfs_dir_open(&lfs, &dir[0], "c") => 0; @@ -205,8 +203,6 @@ tests/test.py << TEST lfs_dir_read(&lfs, &dir[0], &info) => 1; strcmp(info.name, "..") => 0; lfs_dir_read(&lfs, &dir[0], &info) => 1; - strcmp(info.name, "hello") => 0; - lfs_dir_read(&lfs, &dir[0], &info) => 1; strcmp(info.name, "hi") => 0; lfs_dir_read(&lfs, &dir[0], &info) => 0; lfs_dir_close(&lfs, &dir[0]) => 0; @@ -215,6 +211,36 @@ tests/test.py << TEST strcmp(info.name, ".") => 0; lfs_dir_read(&lfs, &dir[0], &info) => 1; strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hello") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Move dir after corrupt ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_rename(&lfs, "c/hi", "d/hi") => 0; + lfs_unmount(&lfs) => 0; +TEST +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_dir_open(&lfs, &dir[0], "c") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_dir_close(&lfs, &dir[0]) => 0; + lfs_dir_open(&lfs, &dir[0], "d") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hello") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hi") => 0; lfs_dir_read(&lfs, &dir[0], &info) => 0; lfs_unmount(&lfs) => 0; TEST @@ -225,9 +251,9 @@ tests/test.py << TEST lfs_dir_open(&lfs, &dir[0], "a/hi") => LFS_ERR_NOENT; lfs_dir_open(&lfs, &dir[0], "b/hi") => LFS_ERR_NOENT; - lfs_dir_open(&lfs, &dir[0], "d/hi") => LFS_ERR_NOENT; + lfs_dir_open(&lfs, &dir[0], "c/hi") => LFS_ERR_NOENT; - lfs_dir_open(&lfs, &dir[0], "c/hi") => 0; + lfs_dir_open(&lfs, &dir[0], "d/hi") => 0; lfs_dir_read(&lfs, &dir[0], &info) => 1; strcmp(info.name, ".") => 0; lfs_dir_read(&lfs, &dir[0], &info) => 1; @@ -243,9 +269,9 @@ tests/test.py << TEST lfs_dir_open(&lfs, &dir[0], "a/hello") => LFS_ERR_NOENT; lfs_dir_open(&lfs, &dir[0], "b/hello") => LFS_ERR_NOENT; - lfs_dir_open(&lfs, &dir[0], "d/hello") => LFS_ERR_NOENT; + lfs_dir_open(&lfs, &dir[0], "c/hello") => LFS_ERR_NOENT; - lfs_file_open(&lfs, &file[0], "c/hello", LFS_O_RDONLY) => 0; + lfs_file_open(&lfs, &file[0], "d/hello", LFS_O_RDONLY) => 0; lfs_file_read(&lfs, &file[0], buffer, 5) => 5; memcmp(buffer, "hola\n", 5) => 0; lfs_file_read(&lfs, &file[0], buffer, 8) => 8;