ソースを参照

Fixed synthetic move underflows in lfs_dir_get

By "luck" the previous code somehow managed to not be broken, though it
was possible to traverse the same file twice in lfs_fs_traverse/size
(which is not an error).

The problem was an underlying assumption in lfs_dir_get that it would
never be called when the requested id is pending removal because of a
powerloss. The assumption was either:

1. lfs_dir_find would need to be called first to find the id, and it
   would correctly toss out pending-rms with LFS_ERR_NOENT.

2. lfs_fs_mkconsistent would be implicitly called before any filesystem
   traversals, cleaning up any pending-rms. This is at least true for
   allocator scans.

But, as noted by andriyndev, both lfs_fs_traverse and lfs_fs_size can
call lfs_fs_get with a pending-rm id if called in a readonly context.

---

By "luck" this somehow manages to not break anything:

1. If the pending-rm id is >0, the id is decremented by 1 in lfs_fs_get,
   returning the previous file entry during traversal. Worst case, this
   reports any blocks owned by the previous file entry twice.

   Note this is not an error, lfs_fs_traverse/size may return the same
   block multiple times due to underlying copy-on-write structures.

2. More concerning, if the pending-rm id is 0, the id is decremented by
   1 in lfs_fs_get and underflows. This underflow propagates into the
   type field of the tag we are searching for, decrementing it from
   0x200 (LFS_TYPE_STRUCT) to 0x1ff (LFS_TYPE_INTERNAL(UNUSED)).

   Fortunately, since this happens to underflow to the INTERNAL tag
   type, the type intended to never exist on disk, we should never find
   a matching tag during our lfs_fs_get search. The result? lfs_dir_get
   returns LFS_ERR_NOENT, which is actually what we want.

Also note that LFS_ERR_NOENT does not terminate the mdir traversal
early. If it did we would have missed files instead of duplicating
files, which is a slightly worse situation.

---

The fix is to add an explicit check for pending-rms in lfs_dir_get, just
like in lfs_dir_find. This avoids relying on unintended underflow
propagation, and should make the internal API behavior more consistent.

This is especially important for potential future gc extensions.

Found by andriyndev
Christopher Haster 1 年間 前
コミット
ddbfcaa722
1 ファイル変更7 行追加4 行削除
  1. 7 4
      lfs.c

+ 7 - 4
lfs.c

@@ -710,11 +710,14 @@ static lfs_stag_t lfs_dir_getslice(lfs_t *lfs, const lfs_mdir_t *dir,
     lfs_tag_t ntag = dir->etag;
     lfs_stag_t gdiff = 0;
 
+    // synthetic moves
     if (lfs_gstate_hasmovehere(&lfs->gdisk, dir->pair) &&
-            lfs_tag_id(gmask) != 0 &&
-            lfs_tag_id(lfs->gdisk.tag) <= lfs_tag_id(gtag)) {
-        // synthetic moves
-        gdiff -= LFS_MKTAG(0, 1, 0);
+            lfs_tag_id(gmask) != 0) {
+        if (lfs_tag_id(lfs->gdisk.tag) == lfs_tag_id(gtag)) {
+            return LFS_ERR_NOENT;
+        } else if (lfs_tag_id(lfs->gdisk.tag) < lfs_tag_id(gtag)) {
+            gdiff -= LFS_MKTAG(0, 1, 0);
+        }
     }
 
     // iterate over dir block backwards (for faster lookups)