[PATCH] ntfs: fail the mount when $MFT needs its own extent records

[Matthias Goergens]

Mounting a crafted image hangs the mount process forever: no output, no
progress, unkillable, 0% CPU.  Nothing is reported, because the task is
not blocked on any lock lockdep tracks.  With PROVE_LOCKING and
DEBUG_MUTEXES on it stays silent, and only the hung-task detector
notices:

  INFO: task mount:74 blocked in I/O wait for more than 30 seconds.
  locks held by mount/74: 1
    #0: (&type->s_umount_key#26/1) at sget_fc

It is a folio lock, which lockdep does not track:

  folio_wait_bit_common      <- waits forever
  filemap_read_folio
  map_mft_record_folio
  map_mft_record
  ntfs_map_runlist_nolock
  ntfs_attr_vcn_to_rl
  __ntfs_read_iomap_begin
  iomap_read_folio
  ntfs_read_folio            <- already holds that folio's lock
  ntfs_read_inode_mount
  ntfs_fill_super

ntfs_read_inode_mount() builds the first extent of $MFT/$DATA by hand
and then calls ntfs_read_locked_inode() to pick up $MFT's remaining
attributes.  The comment there says what that assumes:

	 * ... we would hope that we don't need
	 * further extents in order to find the other
	 * attributes belonging to $MFT. ... But lets
	 * hope this never happens...

An image whose $MFT attribute list puts those attributes in one of
$MFT's own extent records makes it false.  Mapping that extent record
reads $MFT at a vcn the half-built runlist does not cover, so the read
comes back into ntfs_map_runlist_nolock() for $MFT while it already
holds the lock on the folio it then waits for.

So check it.  Mark the volume while the bootstrap is assembling $MFT's
runlist, and refuse to map $MFT's runlist through the general path
while that mark is set.  The bootstrap builds the runlist itself with
ntfs_mapping_pairs_decompress() and never goes through the guarded
path, so only the re-entrant case is rejected.

The mount now fails instead of hanging:

  ntfs: (device vda): ntfs_map_runlist_nolock(): $MFT needs its own
    extent records to describe itself. $MFT is corrupt. Run chkdsk.
  ntfs: (device vda): ntfs_read_inode_mount(): ntfs_read_inode() of
    $MFT failed.

The guard bails before the function touches anything: no search
context, no mapped record, no lock taken.  -EIO is not a new return
value here either, and all six call sites already handle a negative
one.  ntfs_map_runlist() and ntfs_empty_logfile() propagate it,
ntfs_attr_vcn_to_lcn_nolock() maps it to LCN_EIO,
ntfs_attr_find_vcn_nolock() passes it through, and
ntfs_attr_vcn_to_rl() and ntfs_write_simple_iomap_begin_non_resident()
only act on success, falling through to the unmapped-runlist handling
they already have.

Found by fuzzing mountable images.  Checked under qemu with KASAN,
PROVE_LOCKING and the hung-task detector: two crafted images hang an
unpatched kernel and are rejected by a patched one, and eight
well-formed images mount on both.

Fixes: b041ca562526 ("ntfs: update iomap and address space operations")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
---
The two crafted images are 28 KB each; happy to send them or put them
somewhere fetchable.

fs/ntfs3 rejects both while loading $Volume, so it is not affected by
these images, but I did not try fuzzing its parser.

 fs/ntfs/attrib.c | 11 +++++++++++
 fs/ntfs/inode.c  |  2 ++
 fs/ntfs/volume.h |  3 +++
 3 files changed, 16 insertions(+)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index c949ff765075..f94e928f145f 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -103,6 +103,17 @@ int ntfs_map_runlist_nolock(struct ntfs_inode *ni, s64 vcn, struct ntfs_attr_sea
 		base_ni = ni;
 	else
 		base_ni = ni->ext.base_ntfs_ino;
+	/*
+	 * ntfs_read_inode_mount() builds $MFT's runlist itself, so nothing
+	 * should reach here for $MFT.  A crafted image can: the read that
+	 * gets here already holds the $MFT folio lock it would wait on.
+	 */
+	if (unlikely(NVolMftBootstrap(ni->vol) &&
+		     base_ni == NTFS_I(ni->vol->mft_ino))) {
+		ntfs_error(ni->vol->sb,
+			   "$MFT needs its own extent records to describe itself. $MFT is corrupt. Run chkdsk.");
+		return -EIO;
+	}
 	if (!ctx) {
 		ctx_is_temporary = ctx_needs_reset = true;
 		m = map_mft_record(base_ni);
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index a777de8a80c7..55f6d3334238 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -2150,7 +2150,9 @@ int ntfs_read_inode_mount(struct inode *vi)
 			 * ntfs_read_inode() on extents of $MFT/$DATA. But lets
 			 * hope this never happens...
 			 */
+			NVolSetMftBootstrap(vol);
 			err = ntfs_read_locked_inode(vi);
+			NVolClearMftBootstrap(vol);
 			if (err) {
 				ntfs_error(sb, "ntfs_read_inode() of $MFT failed.\n");
 				ntfs_attr_put_search_ctx(ctx);
diff --git a/fs/ntfs/volume.h b/fs/ntfs/volume.h
index bc85a9592245..f9ecbbe205ad 100644
--- a/fs/ntfs/volume.h
+++ b/fs/ntfs/volume.h
@@ -184,6 +184,7 @@ struct ntfs_volume {
  * NV_Discard			Issue discard/TRIM commands for freed clusters.
  * NV_DisableSparse		Disable creation of sparse regions.
  * NV_NativeSymlinkRel		Translate absolute Windows reparse targets (native_symlink=rel).
+ * NV_MftBootstrap		Mount is still assembling $MFT's own runlist.
  */
 enum {
 	NV_Errors,
@@ -203,6 +204,7 @@ enum {
 	NV_DisableSparse,
 	NV_NativeSymlinkRel,
 	NV_SymlinkNative,
+	NV_MftBootstrap,
 };
 
 /*
@@ -241,6 +243,7 @@ DEFINE_NVOL_BIT_OPS(Discard)
 DEFINE_NVOL_BIT_OPS(DisableSparse)
 DEFINE_NVOL_BIT_OPS(NativeSymlinkRel)
 DEFINE_NVOL_BIT_OPS(SymlinkNative)
+DEFINE_NVOL_BIT_OPS(MftBootstrap)
 
 static inline void ntfs_inc_free_clusters(struct ntfs_volume *vol, s64 nr)
 {

base-commit: 40288c9206c17eb66a603262e06a58d300d0f279
-- 
2.55.0