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

[Hyunchul Lee <hyc.lee@gmail.com>]

Hi Matthias,

On Tue, Sep 22, 2026 at 11:39:31PM +0800, Matthias Goergens wrote:
> 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@vger.kernel.org
> Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
> ---
> 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);

The scope of the bootstrp flag seems to be too narrow.
The outer loop continues to call ntfs_attr_lookup() until all $DATA
extents are enumerated. ntfs_attr_lookup() can call map_mft_record()
also, when mapping pairs is stored in another MFT record. So the same
folio deadlock can occur.

I think that NVolMftBootstrap should remain set until all $DATA
extent enumeration has completed.

>  			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
> 

-- 
Thanks,
Hyunchul