From 7fad472af503094506e1b41eeae5829ff4bfd191 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 9 Jun 2024 13:12:27 -0500 Subject: [PATCH] Added detection/handling of unknown file types This adds a couple things so our unknown file types don't just cause our filesystem to fall over: - lfsr_mount now prints a warning on any unknown file types found at mount time. Since we're already iterating over all files to find orphans, this is basically free. - Added LFS_TYPE_UNKNOWN to represent files with an unknown/unsupported type. This is now returned by lfsr_stat/lfsr_dir_read for files of any unknow type. - Added LFS_ERR_NOTSUP. This is now returned by functions that attempt to modify a file of unknown type, and my have more use cases in the future. It's tempting to allow remove/rename on unknown file types, but since we don't know what data structures these may be referencing, doing so would likely leak storage. Or worse. Shrubs for example would just explode if you only moved the metadata entry. This also adds test_incompat_unknown to test these cases. Code changes are minimal, though there are a number of extra conditions to check for unknown file types. The lfsr_mount condition is particularly fun as it should be completely optimized out when debug statements are disabled: code stack before: 33670 2592 after: 33710 (+0.1%) 2592 (+0.0%) --- lfs.c | 60 +++++++++++++++++----- lfs.h | 4 +- tests/test_incompat.toml | 106 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 14 deletions(-) diff --git a/lfs.c b/lfs.c index 0fedf191..9fa01993 100644 --- a/lfs.c +++ b/lfs.c @@ -910,6 +910,13 @@ static inline bool lfsr_tag_q(lfsr_tag_t tag) { return tag & LFSR_TAG_Q; } +static inline bool lfsr_tag_isunknown(lfsr_tag_t tag) { + return tag != LFSR_TAG_REG + && tag != LFSR_TAG_DIR + && tag != LFSR_TAG_BOOKMARK + && tag != LFSR_TAG_ORPHAN; +} + static inline bool lfsr_tag_isinternal(lfsr_tag_t tag) { return tag & LFSR_TAG_INTERNAL; } @@ -7486,6 +7493,7 @@ enum { // - 0 => path is valid, file NOT found // - EXIST => path is valid, file found // - INVAL => path is valid, but points to root +// - NOTSUP => path is valid, but points to unknown file type // - NOENT => path is NOT valid, intermediate dir missing // - NOTDIR => path is NOT valid, intermediate dir is not a dir // @@ -7559,8 +7567,11 @@ static int lfsr_mtree_pathlookup(lfs_t *lfs, const lfsr_mtree_t *mtree, // the root dir doesn't have an mdir really, so it's always // a special case return (mdir.mid == -1) - ? LFS_ERR_INVAL - : LFS_ERR_EXIST; + ? LFS_ERR_INVAL + // unknown file type? + : (lfsr_tag_isunknown(tag)) + ? LFS_ERR_NOTSUP + : LFS_ERR_EXIST; } // found another name @@ -8589,21 +8600,34 @@ static int lfsr_mountinited(lfs_t *lfs) { // check for any orphaned files for (lfs_size_t rid = 0; rid < tinfo.u.mdir.rbyd.weight; rid++) { - err = lfsr_rbyd_lookup(lfs, &tinfo.u.mdir.rbyd, - rid, LFSR_TAG_ORPHAN, - NULL); - if (err && err != LFS_ERR_NOENT) { + lfsr_tag_t tag; + err = lfsr_rbyd_sublookup(lfs, &tinfo.u.mdir.rbyd, + rid, LFSR_TAG_NAME, + &tag, NULL); + if (err) { + LFS_ASSERT(err != LFS_ERR_NOENT); return err; } + // name 0 should be reserved + LFS_ASSERT(tag != (LFSR_TAG_NAME + 0)); // found an orphaned file? - if (err != LFS_ERR_NOENT) { + if (tag == LFSR_TAG_ORPHAN) { LFS_DEBUG("Found orphaned file " "%"PRId32".%"PRId32, lfsr_mid_bid(lfs, tinfo.u.mdir.mid) >> lfs->mdir_bits, rid); lfs->hasorphans = true; + + // found an unknown file type? + } else if (lfsr_tag_isunknown(tag)) { + LFS_WARN("Found unknown file type " + "%"PRId32".%"PRId32" 0x%"PRIx16, + lfsr_mid_bid(lfs, tinfo.u.mdir.mid) + >> lfs->mdir_bits, + rid, + lfsr_tag_subtype(tag)); } } @@ -9136,11 +9160,13 @@ int lfsr_mkdir(lfs_t *lfs, const char *path) { err = lfsr_mtree_pathlookup(lfs, &lfs->mtree, path, &mdir, &tag, &did, &name, &name_size); - if (err && err != LFS_ERR_EXIST && err != LFS_ERR_INVAL) { + if (err && err != LFS_ERR_EXIST + && err != LFS_ERR_INVAL + && err != LFS_ERR_NOTSUP) { return err; } // already exists? note orphans don't really exist - bool exists = (err == LFS_ERR_EXIST || err == LFS_ERR_INVAL); + bool exists = (bool)err; if (exists && tag != LFSR_TAG_ORPHAN) { return LFS_ERR_EXIST; } @@ -9581,7 +9607,11 @@ static int lfsr_stat_(lfs_t *lfs, const lfsr_mdir_t *mdir, lfsr_tag_t tag, lfsr_data_t name, struct lfs_info *info) { // get file type from the tag - info->type = lfsr_tag_subtype(tag); + if (tag == LFSR_TAG_REG || tag == LFSR_TAG_DIR) { + info->type = lfsr_tag_subtype(tag); + } else { + info->type = LFS_TYPE_UNKNOWN; + } // read the file name LFS_ASSERT(lfsr_data_size(name) <= LFS_NAME_MAX); @@ -9633,7 +9663,9 @@ int lfsr_stat(lfs_t *lfs, const char *path, struct lfs_info *info) { int err = lfsr_mtree_pathlookup(lfs, &lfs->mtree, path, &mdir, &tag, NULL, &name, &name_size); - if (err && err != LFS_ERR_EXIST && err != LFS_ERR_INVAL) { + if (err && err != LFS_ERR_EXIST + && err != LFS_ERR_INVAL + && err != LFS_ERR_NOTSUP) { return err; } // doesn't exist? note orphans don't really exist @@ -9666,7 +9698,8 @@ int lfsr_dir_open(lfs_t *lfs, lfsr_dir_t *dir, const char *path) { int err = lfsr_mtree_pathlookup(lfs, &lfs->mtree, path, &mdir, &tag, NULL, NULL, NULL); - if (err && err != LFS_ERR_EXIST && err != LFS_ERR_INVAL) { + if (err && err != LFS_ERR_EXIST + && err != LFS_ERR_INVAL) { return err; } // doesn't exist? note orphans don't really exist @@ -10025,7 +10058,8 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file, int err = lfsr_mtree_pathlookup(lfs, &lfs->mtree, path, &file->o.mdir, &tag, &did, &name, &name_size); - if (err && err != LFS_ERR_EXIST && err != LFS_ERR_INVAL) { + if (err && err != LFS_ERR_EXIST + && err != LFS_ERR_INVAL) { return err; } diff --git a/lfs.h b/lfs.h index 551de1de..8c22b885 100644 --- a/lfs.h +++ b/lfs.h @@ -94,6 +94,8 @@ typedef int32_t lfsr_sdid_t; // valid positive return values enum lfs_error { LFS_ERR_OK = 0, // No error + LFS_ERR_INVAL = -22, // Invalid parameter + LFS_ERR_NOTSUP = -95, // Operation not supported LFS_ERR_IO = -5, // Error during device operation LFS_ERR_CORRUPT = -84, // Corrupted LFS_ERR_NOENT = -2, // No directory entry @@ -102,7 +104,6 @@ enum lfs_error { LFS_ERR_ISDIR = -21, // Entry is a dir LFS_ERR_NOTEMPTY = -39, // Dir is not empty LFS_ERR_FBIG = -27, // File too large - LFS_ERR_INVAL = -22, // Invalid parameter LFS_ERR_NOSPC = -28, // No space left on device LFS_ERR_NOMEM = -12, // No more memory available LFS_ERR_NOATTR = -61, // No data/attr available @@ -113,6 +114,7 @@ enum lfs_error { // File types enum lfs_type { // file types + LFS_TYPE_UNKNOWN = 0, LFS_TYPE_REG = 1, LFS_TYPE_DIR = 2, diff --git a/tests/test_incompat.toml b/tests/test_incompat.toml index dc862e59..6e24febd 100644 --- a/tests/test_incompat.toml +++ b/tests/test_incompat.toml @@ -316,3 +316,109 @@ code = ''' lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL; ''' +# test what happens if we find an unknown file type +[cases.test_incompat_unknown] +in = 'lfs.c' +code = ''' + // create a superblock + lfs_t lfs; + lfsr_format(&lfs, CFG) => 0; + + // create some files + lfsr_mount(&lfs, CFG) => 0; + lfsr_file_t file; + lfsr_file_open(&lfs, &file, "a", + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + lfsr_file_write(&lfs, &file, "hi a!", strlen("hi a!")) => strlen("hi a!"); + lfsr_file_close(&lfs, &file) => 0; + lfsr_file_open(&lfs, &file, "b", + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + lfsr_file_write(&lfs, &file, "hi b!", strlen("hi b!")) => strlen("hi b!"); + lfsr_file_close(&lfs, &file) => 0; + lfsr_file_open(&lfs, &file, "c", + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + lfsr_file_write(&lfs, &file, "hi c!", strlen("hi c!")) => strlen("hi c!"); + lfsr_file_close(&lfs, &file) => 0; + lfsr_unmount(&lfs) => 0; + + // change a file's type to something unknown + lfsr_mount(&lfs, CFG) => 0; + lfsr_mdir_t mdir; + lfsr_did_t did; + const char *name; + lfs_size_t name_size; + lfsr_mtree_pathlookup(&lfs, &lfs.mtree, "b", + &mdir, NULL, + &did, &name, &name_size) => LFS_ERR_EXIST; + lfsr_mdir_commit(&lfs, &mdir, LFSR_ATTRS( + LFSR_ATTR_NAME( + LFSR_TAG_SUB | (LFSR_TAG_NAME + 0x13), 0, + did, name, name_size))) => 0; + lfsr_unmount(&lfs) => 0; + + // mount + lfsr_mount(&lfs, CFG) => 0; + + // our file should appear as unknown + struct lfs_info info; + lfsr_stat(&lfs, "b", &info) => 0; + assert(strcmp(info.name, "b") == 0); + assert(info.type == LFS_TYPE_UNKNOWN); + assert(info.size == 0); + + lfsr_dir_t dir; + lfsr_dir_open(&lfs, &dir, "/") => 0; + lfsr_dir_read(&lfs, &dir, &info) => 0; + assert(strcmp(info.name, ".") == 0); + assert(info.type == LFS_TYPE_DIR); + assert(info.size == 0); + lfsr_dir_read(&lfs, &dir, &info) => 0; + assert(strcmp(info.name, "..") == 0); + assert(info.type == LFS_TYPE_DIR); + assert(info.size == 0); + lfsr_dir_read(&lfs, &dir, &info) => 0; + assert(strcmp(info.name, "a") == 0); + assert(info.type == LFS_TYPE_REG); + assert(info.size == strlen("hi a!")); + lfsr_dir_read(&lfs, &dir, &info) => 0; + assert(strcmp(info.name, "b") == 0); + assert(info.type == LFS_TYPE_UNKNOWN); + assert(info.size == 0); + lfsr_dir_read(&lfs, &dir, &info) => 0; + assert(strcmp(info.name, "c") == 0); + assert(info.type == LFS_TYPE_REG); + assert(info.size == strlen("hi c!")); + lfsr_dir_read(&lfs, &dir, &info) => LFS_ERR_NOENT; + lfsr_dir_close(&lfs, &dir) => 0; + + // removing/renaming unknown files should return NOTSUP, we could + // remove the metadata entry, but we would probably leak stuff + lfsr_remove(&lfs, "b") => LFS_ERR_NOTSUP; + lfsr_rename(&lfs, "b", "d") => LFS_ERR_NOTSUP; + lfsr_rename(&lfs, "b", "c") => LFS_ERR_NOTSUP; + lfsr_rename(&lfs, "a", "b") => LFS_ERR_NOTSUP; + + lfsr_file_open(&lfs, &file, "b", + LFS_O_RDONLY) => LFS_ERR_NOTSUP; + lfsr_file_open(&lfs, &file, "b", + LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTSUP; + + lfsr_mkdir(&lfs, "b") => LFS_ERR_EXIST; + + // we should also not try to use unknown files as dirs, which can + // be a bit tricky + lfsr_stat(&lfs, "b/d", &info) => LFS_ERR_NOTDIR; + lfsr_remove(&lfs, "b/e") => LFS_ERR_NOTDIR; + lfsr_rename(&lfs, "b/e", "d") => LFS_ERR_NOTDIR; + lfsr_rename(&lfs, "b/e", "c") => LFS_ERR_NOTDIR; + lfsr_rename(&lfs, "a", "b/e") => LFS_ERR_NOTDIR; + + lfsr_file_open(&lfs, &file, "b/e", + LFS_O_RDONLY) => LFS_ERR_NOTDIR; + lfsr_file_open(&lfs, &file, "b/e", + LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTDIR; + + lfsr_mkdir(&lfs, "b/e") => LFS_ERR_NOTDIR; + + lfsr_unmount(&lfs) => 0; +'''