[PATCH 0/2] ntfs: fix the undo path of $MFT data extension

[PATCH 0/2] ntfs: fix the undo path of $MFT data extension

[Matthias Goergens]

These fix two bugs in the undo path of
ntfs_mft_data_extend_allocation_nolock().  Patch 1 makes the $MFT
runlist locking there consistent: a map_mft_record() failure leaks the
lock, a lookup failure in restore_undo_alloc releases it without holding
it, and the clusters are freed and the runlist truncated without it.
Patch 2 fixes a use-after-free of a pointer into the runlist across the
truncation.

Both paths only run when something fails while $MFT grows, so I tested
them in QEMU by forcing each failure once with a debug patch.  The
debug patch, the test scripts and the images are at
https://github.com/matthiasgoergens/linux/tree/reproducer/2026-09-27-ntfs-mft-extend-undo

Thanks,
Matthias

Matthias Goergens (2):
  ntfs: balance the $MFT runlist lock in data extension error paths
  ntfs: do not use a stale runlist pointer when undoing $MFT extension

 fs/ntfs/mft.c | 63 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 10 deletions(-)


base-commit: 259abb551e2944998cad4214c201954ab1ac5c8d
-- 
2.55.0

[PATCH 1/2] ntfs: balance the $MFT runlist lock in data extension error paths

[Matthias Goergens]

ntfs_mft_data_extend_allocation_nolock() drops the $MFT runlist lock
before allocating clusters, and every path into undo_alloc arrives
without it, except a map_mft_record() failure, which takes it first.
undo_alloc never releases it, so the lock leaks and the next $MFT
extension and $MFT writeback hang.  The restore_undo_alloc failure
path, on the other hand, releases the lock without holding it.  And
undo_alloc frees the clusters with ntfs_cluster_free() and truncates
the runlist without the lock, although both require it.

Enter undo_alloc without the lock on every path.  Under the lock, copy
the new runs and truncate the runlist; after dropping it, free the
clusters from the copy with ntfs_cluster_free_from_rl().  That keeps
lcnbmp_lock outside the runlist lock, as the function documents.  Like
the runlist merge failure above, this does not discard the clusters,
which were never written.  If the copy cannot be allocated, the
clusters stay allocated and the volume is marked for chkdsk.

A shorter fix would keep calling ntfs_cluster_free() on the live
runlist without the lock, relying on $MFT's runlist being fully mapped
and changed only by this function under mrec_lock.  That breaks
ntfs_cluster_free()'s documented locking rule on an argument about the
rest of the driver, so this patch does not do that.

Fixes: 115380f9a2f9 ("ntfs: update mft operations")
Cc: stable@vger.kernel.org
Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
---
Without this patch, a forced map_mft_record() failure gives "WARNING:
lock held when returning to user space" for the $MFT runlist lock, and
the next file create and $MFT writeback block on it.  A forced lookup
failure in restore_undo_alloc gives "bad unlock balance".  With it, the
create fails with EIO, lockdep stays quiet and the volume keeps working,
also when the copy allocation is forced to fail.  $MFT growth on a
fragmented volume behaves as before.

 fs/ntfs/mft.c | 54 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index e01e367a588d9..58676042444b6 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -1764,6 +1764,36 @@ static int ntfs_mft_bitmap_extend_initialized_nolock(struct ntfs_volume *vol)
 	return ret;
 }
 
+/*
+ * ntfs_mft_copy_tail - copy the runs of a runlist from a vcn onwards
+ * @rl:		runlist to copy from
+ * @vcn:	first vcn to copy
+ *
+ * Return a terminated copy of the runs of @rl covering @vcn and everything
+ * after it, NULL if there are none, or ERR_PTR(-ENOMEM).  The caller must
+ * hold the runlist lock and free the copy with kfree().
+ */
+static struct runlist_element *ntfs_mft_copy_tail(struct runlist_element *rl, s64 vcn)
+{
+	struct runlist_element *end, *copy;
+	s64 delta;
+
+	rl = ntfs_rl_find_vcn_nolock(rl, vcn);
+	if (!rl || !rl->length)
+		return NULL;
+	for (end = rl; end->length; end++)
+		;
+	copy = kmemdup(rl, (end - rl + 1) * sizeof(*rl), GFP_NOFS);
+	if (!copy)
+		return ERR_PTR(-ENOMEM);
+	delta = vcn - copy->vcn;
+	copy->vcn = vcn;
+	copy->length -= delta;
+	if (copy->lcn >= 0)
+		copy->lcn += delta;
+	return copy;
+}
+
 /*
  * ntfs_mft_data_extend_allocation_nolock - extend mft data attribute
  * @vol:	volume on which to extend the mft data attribute
@@ -1791,7 +1821,7 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
 	s64 min_nr, nr, ll;
 	unsigned long flags;
 	struct ntfs_inode *mft_ni;
-	struct runlist_element *rl, *rl2;
+	struct runlist_element *rl, *rl2, *tail_rl;
 	struct ntfs_attr_search_ctx *ctx = NULL;
 	struct mft_record *mrec;
 	struct attr_record *a = NULL;
@@ -1904,7 +1934,6 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
 	if (IS_ERR(mrec)) {
 		ntfs_error(vol->sb, "Failed to map mft record.");
 		ret = PTR_ERR(mrec);
-		down_write(&mft_ni->runlist.lock);
 		goto undo_alloc;
 	}
 	ctx = ntfs_attr_get_search_ctx(mft_ni, mrec);
@@ -2015,7 +2044,6 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
 		write_unlock_irqrestore(&mft_ni->size_lock, flags);
 		ntfs_attr_put_search_ctx(ctx);
 		unmap_mft_record(mft_ni);
-		up_write(&mft_ni->runlist.lock);
 		/*
 		 * The only thing that is now wrong is ->allocated_size of the
 		 * base attribute extent which chkdsk should be able to fix.
@@ -2026,15 +2054,25 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
 	ctx->attr->data.non_resident.highest_vcn =
 			cpu_to_le64(old_last_vcn - 1);
 undo_alloc:
-	if (ntfs_cluster_free(mft_ni, old_last_vcn, -1, ctx) < 0) {
-		ntfs_error(vol->sb, "Failed to free clusters from mft data attribute.%s", es);
-		NVolSetErrors(vol);
-	}
-
+	/*
+	 * Entered without the runlist lock.  Take the new runs off the
+	 * runlist under it, and free their clusters from a copy once it is
+	 * dropped, as lcnbmp_lock nests outside it (see above).  If the copy
+	 * cannot be allocated, the clusters stay allocated until chkdsk.
+	 */
+	down_write(&mft_ni->runlist.lock);
+	tail_rl = ntfs_mft_copy_tail(mft_ni->runlist.rl, old_last_vcn);
 	if (ntfs_rl_truncate_nolock(vol, &mft_ni->runlist, old_last_vcn)) {
 		ntfs_error(vol->sb, "Failed to truncate mft data attribute runlist.%s", es);
 		NVolSetErrors(vol);
 	}
+	up_write(&mft_ni->runlist.lock);
+	if (IS_ERR(tail_rl) || ntfs_cluster_free_from_rl(vol, tail_rl)) {
+		ntfs_error(vol->sb, "Failed to free clusters from mft data attribute.%s", es);
+		NVolSetErrors(vol);
+	}
+	if (!IS_ERR(tail_rl))
+		kfree(tail_rl);
 	if (mp_extended && ntfs_attr_update_mapping_pairs(mft_ni, 0)) {
 		ntfs_error(vol->sb, "Failed to restore mapping pairs.%s",
 			   es);
-- 
2.55.0

Re: [PATCH 1/2] ntfs: balance the $MFT runlist lock in data extension error paths

[liubaolin]

在 2026/9/27 18:57, Matthias Goergens 写道:
> ntfs_mft_data_extend_allocation_nolock() drops the $MFT runlist lock
> before allocating clusters, and every path into undo_alloc arrives
> without it, except a map_mft_record() failure, which takes it first.
> undo_alloc never releases it, so the lock leaks and the next $MFT
> extension and $MFT writeback hang.  The restore_undo_alloc failure
> path, on the other hand, releases the lock without holding it.  And
> undo_alloc frees the clusters with ntfs_cluster_free() and truncates
> the runlist without the lock, although both require it.
> 
> Enter undo_alloc without the lock on every path.  Under the lock, copy
> the new runs and truncate the runlist; after dropping it, free the
> clusters from the copy with ntfs_cluster_free_from_rl().  That keeps
> lcnbmp_lock outside the runlist lock, as the function documents.  Like
> the runlist merge failure above, this does not discard the clusters,
> which were never written.  If the copy cannot be allocated, the
> clusters stay allocated and the volume is marked for chkdsk.
> 
> A shorter fix would keep calling ntfs_cluster_free() on the live
> runlist without the lock, relying on $MFT's runlist being fully mapped
> and changed only by this function under mrec_lock.  That breaks
> ntfs_cluster_free()'s documented locking rule on an argument about the
> rest of the driver, so this patch does not do that.
> 
> Fixes: 115380f9a2f9 ("ntfs: update mft operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
> ---
> Without this patch, a forced map_mft_record() failure gives "WARNING:
> lock held when returning to user space" for the $MFT runlist lock, and
> the next file create and $MFT writeback block on it.  A forced lookup
> failure in restore_undo_alloc gives "bad unlock balance".  With it, the
> create fails with EIO, lockdep stays quiet and the volume keeps working,
> also when the copy allocation is forced to fail.  $MFT growth on a
> fragmented volume behaves as before.
> 
>   fs/ntfs/mft.c | 54 +++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
> index e01e367a588d9..58676042444b6 100644
> --- a/fs/ntfs/mft.c
> +++ b/fs/ntfs/mft.c
> @@ -1764,6 +1764,36 @@ static int ntfs_mft_bitmap_extend_initialized_nolock(struct ntfs_volume *vol)
>   	return ret;
>   }
>   
> +/*
> + * ntfs_mft_copy_tail - copy the runs of a runlist from a vcn onwards
> + * @rl:		runlist to copy from
> + * @vcn:	first vcn to copy
> + *
> + * Return a terminated copy of the runs of @rl covering @vcn and everything
> + * after it, NULL if there are none, or ERR_PTR(-ENOMEM).  The caller must
> + * hold the runlist lock and free the copy with kfree().
> + */
> +static struct runlist_element *ntfs_mft_copy_tail(struct runlist_element *rl, s64 vcn)
> +{
> +	struct runlist_element *end, *copy;
> +	s64 delta;
> +
> +	rl = ntfs_rl_find_vcn_nolock(rl, vcn);
> +	if (!rl || !rl->length)
> +		return NULL;
> +	for (end = rl; end->length; end++)
> +		;
> +	copy = kmemdup(rl, (end - rl + 1) * sizeof(*rl), GFP_NOFS);
> +	if (!copy)
> +		return ERR_PTR(-ENOMEM);
> +	delta = vcn - copy->vcn;
> +	copy->vcn = vcn;
> +	copy->length -= delta;
> +	if (copy->lcn >= 0)
> +		copy->lcn += delta;
> +	return copy;
> +}
> +
>   /*
>    * ntfs_mft_data_extend_allocation_nolock - extend mft data attribute
>    * @vol:	volume on which to extend the mft data attribute
> @@ -1791,7 +1821,7 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
>   	s64 min_nr, nr, ll;
>   	unsigned long flags;
>   	struct ntfs_inode *mft_ni;
> -	struct runlist_element *rl, *rl2;
> +	struct runlist_element *rl, *rl2, *tail_rl;
>   	struct ntfs_attr_search_ctx *ctx = NULL;
>   	struct mft_record *mrec;
>   	struct attr_record *a = NULL;
> @@ -1904,7 +1934,6 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
>   	if (IS_ERR(mrec)) {
>   		ntfs_error(vol->sb, "Failed to map mft record.");
>   		ret = PTR_ERR(mrec);
> -		down_write(&mft_ni->runlist.lock);
>   		goto undo_alloc;
>   	}
>   	ctx = ntfs_attr_get_search_ctx(mft_ni, mrec);
> @@ -2015,7 +2044,6 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
>   		write_unlock_irqrestore(&mft_ni->size_lock, flags);
>   		ntfs_attr_put_search_ctx(ctx);
>   		unmap_mft_record(mft_ni);
> -		up_write(&mft_ni->runlist.lock);
>   		/*
>   		 * The only thing that is now wrong is ->allocated_size of the
>   		 * base attribute extent which chkdsk should be able to fix.
> @@ -2026,15 +2054,25 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
>   	ctx->attr->data.non_resident.highest_vcn =
>   			cpu_to_le64(old_last_vcn - 1);
>   undo_alloc:
> -	if (ntfs_cluster_free(mft_ni, old_last_vcn, -1, ctx) < 0) {
> -		ntfs_error(vol->sb, "Failed to free clusters from mft data attribute.%s", es);
> -		NVolSetErrors(vol);
> -	}
> -
> +	/*
> +	 * Entered without the runlist lock.  Take the new runs off the
> +	 * runlist under it, and free their clusters from a copy once it is
> +	 * dropped, as lcnbmp_lock nests outside it (see above).  If the copy
> +	 * cannot be allocated, the clusters stay allocated until chkdsk.
> +	 */
> +	down_write(&mft_ni->runlist.lock);
> +	tail_rl = ntfs_mft_copy_tail(mft_ni->runlist.rl, old_last_vcn);
>   	if (ntfs_rl_truncate_nolock(vol, &mft_ni->runlist, old_last_vcn)) {
>   		ntfs_error(vol->sb, "Failed to truncate mft data attribute runlist.%s", es);
>   		NVolSetErrors(vol);
>   	}
> +	up_write(&mft_ni->runlist.lock);
> +	if (IS_ERR(tail_rl) || ntfs_cluster_free_from_rl(vol, tail_rl)) {
> +		ntfs_error(vol->sb, "Failed to free clusters from mft data attribute.%s", es);
> +		NVolSetErrors(vol);
> +	}
> +	if (!IS_ERR(tail_rl))
> +		kfree(tail_rl);
>   	if (mp_extended && ntfs_attr_update_mapping_pairs(mft_ni, 0)) {
>   		ntfs_error(vol->sb, "Failed to restore mapping pairs.%s",
>   			   es);


Looks good to me.

Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>

Re: [PATCH 1/2] ntfs: balance the $MFT runlist lock in data extension error paths

[Hyunchul Lee]

On Sun, Sep 27, 2026 at 06:57:05PM +0800, Matthias Goergens wrote:
> ntfs_mft_data_extend_allocation_nolock() drops the $MFT runlist lock
> before allocating clusters, and every path into undo_alloc arrives
> without it, except a map_mft_record() failure, which takes it first.
> undo_alloc never releases it, so the lock leaks and the next $MFT
> extension and $MFT writeback hang.  The restore_undo_alloc failure
> path, on the other hand, releases the lock without holding it.  And
> undo_alloc frees the clusters with ntfs_cluster_free() and truncates
> the runlist without the lock, although both require it.
> 
> Enter undo_alloc without the lock on every path.  Under the lock, copy
> the new runs and truncate the runlist; after dropping it, free the
> clusters from the copy with ntfs_cluster_free_from_rl().  That keeps
> lcnbmp_lock outside the runlist lock, as the function documents.  Like
> the runlist merge failure above, this does not discard the clusters,
> which were never written.  If the copy cannot be allocated, the
> clusters stay allocated and the volume is marked for chkdsk.
> 
> A shorter fix would keep calling ntfs_cluster_free() on the live
> runlist without the lock, relying on $MFT's runlist being fully mapped
> and changed only by this function under mrec_lock.  That breaks
> ntfs_cluster_free()'s documented locking rule on an argument about the
> rest of the driver, so this patch does not do that.
> 
> Fixes: 115380f9a2f9 ("ntfs: update mft operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
> ---
> Without this patch, a forced map_mft_record() failure gives "WARNING:
> lock held when returning to user space" for the $MFT runlist lock, and
> the next file create and $MFT writeback block on it.  A forced lookup
> failure in restore_undo_alloc gives "bad unlock balance".  With it, the
> create fails with EIO, lockdep stays quiet and the volume keeps working,
> also when the copy allocation is forced to fail.  $MFT growth on a
> fragmented volume behaves as before.
> 
>  fs/ntfs/mft.c | 54 +++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
> index e01e367a588d9..58676042444b6 100644
> --- a/fs/ntfs/mft.c
> +++ b/fs/ntfs/mft.c
> @@ -1764,6 +1764,36 @@ static int ntfs_mft_bitmap_extend_initialized_nolock(struct ntfs_volume *vol)
>  	return ret;
>  }
>  
> +/*
> + * ntfs_mft_copy_tail - copy the runs of a runlist from a vcn onwards
> + * @rl:		runlist to copy from
> + * @vcn:	first vcn to copy
> + *
> + * Return a terminated copy of the runs of @rl covering @vcn and everything
> + * after it, NULL if there are none, or ERR_PTR(-ENOMEM).  The caller must
> + * hold the runlist lock and free the copy with kfree().
> + */
> +static struct runlist_element *ntfs_mft_copy_tail(struct runlist_element *rl, s64 vcn)
> +{
> +	struct runlist_element *end, *copy;
> +	s64 delta;
> +
> +	rl = ntfs_rl_find_vcn_nolock(rl, vcn);
> +	if (!rl || !rl->length)
> +		return NULL;
> +	for (end = rl; end->length; end++)
> +		;
> +	copy = kmemdup(rl, (end - rl + 1) * sizeof(*rl), GFP_NOFS);
> +	if (!copy)
> +		return ERR_PTR(-ENOMEM);
> +	delta = vcn - copy->vcn;
> +	copy->vcn = vcn;
> +	copy->length -= delta;
> +	if (copy->lcn >= 0)
> +		copy->lcn += delta;
> +	return copy;
> +}
> +
>  /*
>   * ntfs_mft_data_extend_allocation_nolock - extend mft data attribute
>   * @vol:	volume on which to extend the mft data attribute
> @@ -1791,7 +1821,7 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
>  	s64 min_nr, nr, ll;
>  	unsigned long flags;
>  	struct ntfs_inode *mft_ni;
> -	struct runlist_element *rl, *rl2;
> +	struct runlist_element *rl, *rl2, *tail_rl;
>  	struct ntfs_attr_search_ctx *ctx = NULL;
>  	struct mft_record *mrec;
>  	struct attr_record *a = NULL;
> @@ -1904,7 +1934,6 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
>  	if (IS_ERR(mrec)) {
>  		ntfs_error(vol->sb, "Failed to map mft record.");
>  		ret = PTR_ERR(mrec);
> -		down_write(&mft_ni->runlist.lock);
>  		goto undo_alloc;
>  	}
>  	ctx = ntfs_attr_get_search_ctx(mft_ni, mrec);
> @@ -2015,7 +2044,6 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
>  		write_unlock_irqrestore(&mft_ni->size_lock, flags);
>  		ntfs_attr_put_search_ctx(ctx);
>  		unmap_mft_record(mft_ni);
> -		up_write(&mft_ni->runlist.lock);
>  		/*
>  		 * The only thing that is now wrong is ->allocated_size of the
>  		 * base attribute extent which chkdsk should be able to fix.
> @@ -2026,15 +2054,25 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
>  	ctx->attr->data.non_resident.highest_vcn =
>  			cpu_to_le64(old_last_vcn - 1);
>  undo_alloc:
> -	if (ntfs_cluster_free(mft_ni, old_last_vcn, -1, ctx) < 0) {
> -		ntfs_error(vol->sb, "Failed to free clusters from mft data attribute.%s", es);
> -		NVolSetErrors(vol);
> -	}
> -
> +	/*
> +	 * Entered without the runlist lock.  Take the new runs off the
> +	 * runlist under it, and free their clusters from a copy once it is
> +	 * dropped, as lcnbmp_lock nests outside it (see above).  If the copy
> +	 * cannot be allocated, the clusters stay allocated until chkdsk.
> +	 */
> +	down_write(&mft_ni->runlist.lock);
> +	tail_rl = ntfs_mft_copy_tail(mft_ni->runlist.rl, old_last_vcn);
>  	if (ntfs_rl_truncate_nolock(vol, &mft_ni->runlist, old_last_vcn)) {
>  		ntfs_error(vol->sb, "Failed to truncate mft data attribute runlist.%s", es);
>  		NVolSetErrors(vol);
>  	}
> +	up_write(&mft_ni->runlist.lock);
> +	if (IS_ERR(tail_rl) || ntfs_cluster_free_from_rl(vol, tail_rl)) {

If ntfs_rl_truncate_nolock() fails, we must not call
ntfs_cluster_free_from_rl(), because mft_ni->runlist may still reference
those clusters.

> +		ntfs_error(vol->sb, "Failed to free clusters from mft data attribute.%s", es);
> +		NVolSetErrors(vol);
> +	}
> +	if (!IS_ERR(tail_rl))
> +		kfree(tail_rl);
>  	if (mp_extended && ntfs_attr_update_mapping_pairs(mft_ni, 0)) {
>  		ntfs_error(vol->sb, "Failed to restore mapping pairs.%s",
>  			   es);
> -- 
> 2.55.0
> 

-- 
Thanks,
Hyunchul

[PATCH 2/2] ntfs: do not use a stale runlist pointer when undoing $MFT extension

[Matthias Goergens]

When ntfs_mft_data_extend_allocation_nolock() fails after it has
rebuilt the mapping pairs of the last $MFT data extent, undo_alloc
truncates the runlist and then rebuilds the old mapping pairs from rl2,
a pointer into the runlist taken before the truncation.
ntfs_rl_truncate_nolock() can reallocate the runlist, and KASAN then
reports a use-after-free in ntfs_mapping_pairs_build().

Pass the start of the runlist instead, and hold the runlist lock for
reading while the mapping pairs are built from it.
ntfs_mapping_pairs_build() skips to the element containing the first
vcn by itself.

Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
Cc: stable@vger.kernel.org
Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
---
A forced lookup failure of the first $MFT data extent, on a volume
whose $MFT has several extents, gives the KASAN report before this
patch and nothing after it.

 fs/ntfs/mft.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 58676042444b6..5831ea7600315 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -2081,11 +2081,16 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
 	if (ctx) {
 		a = ctx->attr;
 		if (mp_rebuilt && !IS_ERR(ctx->mrec)) {
-			if (ntfs_mapping_pairs_build(vol, (u8 *)a + le16_to_cpu(
+			int err;
+
+			down_read(&mft_ni->runlist.lock);
+			err = ntfs_mapping_pairs_build(vol, (u8 *)a + le16_to_cpu(
 				a->data.non_resident.mapping_pairs_offset),
 				old_alen - le16_to_cpu(
 					a->data.non_resident.mapping_pairs_offset),
-				rl2, ll, -1, NULL, NULL, NULL)) {
+				mft_ni->runlist.rl, ll, -1, NULL, NULL, NULL);
+			up_read(&mft_ni->runlist.lock);
+			if (err) {
 				ntfs_error(vol->sb, "Failed to restore mapping pairs array.%s", es);
 				NVolSetErrors(vol);
 			}
-- 
2.55.0

Re: [PATCH 2/2] ntfs: do not use a stale runlist pointer when undoing $MFT extension

[liubaolin]

在 2026/9/27 18:57, Matthias Goergens 写道:
> When ntfs_mft_data_extend_allocation_nolock() fails after it has
> rebuilt the mapping pairs of the last $MFT data extent, undo_alloc
> truncates the runlist and then rebuilds the old mapping pairs from rl2,
> a pointer into the runlist taken before the truncation.
> ntfs_rl_truncate_nolock() can reallocate the runlist, and KASAN then
> reports a use-after-free in ntfs_mapping_pairs_build().
> 
> Pass the start of the runlist instead, and hold the runlist lock for
> reading while the mapping pairs are built from it.
> ntfs_mapping_pairs_build() skips to the element containing the first
> vcn by itself.
> 
> Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
> ---
> A forced lookup failure of the first $MFT data extent, on a volume
> whose $MFT has several extents, gives the KASAN report before this
> patch and nothing after it.
> 
>   fs/ntfs/mft.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
> index 58676042444b6..5831ea7600315 100644
> --- a/fs/ntfs/mft.c
> +++ b/fs/ntfs/mft.c
> @@ -2081,11 +2081,16 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
>   	if (ctx) {
>   		a = ctx->attr;
>   		if (mp_rebuilt && !IS_ERR(ctx->mrec)) {
> -			if (ntfs_mapping_pairs_build(vol, (u8 *)a + le16_to_cpu(
> +			int err;
> +
> +			down_read(&mft_ni->runlist.lock);
> +			err = ntfs_mapping_pairs_build(vol, (u8 *)a + le16_to_cpu(
>   				a->data.non_resident.mapping_pairs_offset),
>   				old_alen - le16_to_cpu(
>   					a->data.non_resident.mapping_pairs_offset),
> -				rl2, ll, -1, NULL, NULL, NULL)) {
> +				mft_ni->runlist.rl, ll, -1, NULL, NULL, NULL);
> +			up_read(&mft_ni->runlist.lock);
> +			if (err) {
>   				ntfs_error(vol->sb, "Failed to restore mapping pairs array.%s", es);
>   				NVolSetErrors(vol);
>   			}


Looks good to me.

Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>

Re: [PATCH 0/2] ntfs: fix the undo path of $MFT data extension

[liubaolin]

在 2026/9/27 18:57, Matthias Goergens 写道:
> These fix two bugs in the undo path of
> ntfs_mft_data_extend_allocation_nolock().  Patch 1 makes the $MFT
> runlist locking there consistent: a map_mft_record() failure leaks the
> lock, a lookup failure in restore_undo_alloc releases it without holding
> it, and the clusters are freed and the runlist truncated without it.
> Patch 2 fixes a use-after-free of a pointer into the runlist across the
> truncation.
> 
> Both paths only run when something fails while $MFT grows, so I tested
> them in QEMU by forcing each failure once with a debug patch.  The
> debug patch, the test scripts and the images are at
> https://github.com/matthiasgoergens/linux/tree/reproducer/2026-09-27-ntfs-mft-extend-undo
> 
> Thanks,
> Matthias
> 
> Matthias Goergens (2):
>    ntfs: balance the $MFT runlist lock in data extension error paths
>    ntfs: do not use a stale runlist pointer when undoing $MFT extension
> 
>   fs/ntfs/mft.c | 63 +++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 53 insertions(+), 10 deletions(-)
> 
> 
> base-commit: 259abb551e2944998cad4214c201954ab1ac5c8d

The series looks good to me.

Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>