[PATCH 3/3] ntfs: convert runtime error paths to mark volume dirty after recording errors
From: Hongling Zeng
Date: Wed Sep 02 2026 - 22:25:53 EST
Follow-up to the helper introduction: the runtime metadata-corruption
paths in mft.c, lcnalloc.c, bitmap.c and inode.c still record the error
flag with a bare NVolSetErrors(), leaving the same window in which
ntfs_sync_fs() can acquire ni->mrec_lock, find NVolErrors() unset, and
persist the volume as clean before the erroring thread records its
error.
Convert the call sites that can run concurrently with sync and hold no
mrec_lock, so the helper can take the $Volume inode's mrec_lock to
persist VOLUME_IS_DIRTY:
- the mft record allocation and $MFT data/$BITMAP extend rollback
paths (mft.c), which hold the runlist, mftbmp and lcnbmp locks,
all outer to the $Volume mrec_lock;
- the cluster allocation/free rollback paths (lcnalloc.c), which
hold vol->lcnbmp_lock;
- the bitmap rollback failure path (bitmap.c), which holds no
relevant lock;
- the inode read and writeback failure paths (inode.c), which run
under I_NEW or with the mrec_lock already released at err_out.
The remaining call sites keep calling NVolSetErrors() directly, now
with comments explaining why:
- the mft record writeback paths (write_mft_record_nolock() and
ntfs_write_mft_block()) can run with a caller's mrec_lock held,
which for the $Volume inode itself would self-deadlock the helper;
- the ntfs_attr_lookup() failure paths (ntfs_attr_find(),
ntfs_external_attr_find() and the ntfs_attr_make_non_resident()
rollback) run with the caller's mrec_lock held for the same
reason.
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Hongling Zeng <zenghongling@xxxxxxxxxx>
---
fs/ntfs/attrib.c | 18 ++++++++++++++-
fs/ntfs/bitmap.c | 2 +-
fs/ntfs/inode.c | 8 +++----
fs/ntfs/lcnalloc.c | 4 ++--
fs/ntfs/mft.c | 55 ++++++++++++++++++++++++++++------------------
5 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 60264833bb63..5965a67b5971 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -964,6 +964,11 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
}
ntfs_error(vol->sb, "mft %#llx, type %#x is corrupt. Run chkdsk.",
(long long)ctx->ntfs_ino->mft_no, le32_to_cpu(type));
+ /*
+ * ntfs_attr_lookup() runs with the caller's mrec_lock held, so the
+ * dirty bit cannot be written here (it would self-deadlock for the
+ * $Volume inode); record the error flag only.
+ */
NVolSetErrors(vol);
return -EIO;
}
@@ -1501,8 +1506,14 @@ static int ntfs_external_attr_find(const __le32 type,
err = -EIO;
}
- if (err != -ENOMEM)
+ if (err != -ENOMEM) {
+ /*
+ * ntfs_attr_lookup() runs with the caller's mrec_lock
+ * held, so the dirty bit cannot be written here; record
+ * the error flag only.
+ */
NVolSetErrors(vol);
+ }
return err;
not_found:
/*
@@ -2236,6 +2247,11 @@ int ntfs_attr_make_non_resident(struct ntfs_inode *ni, const u32 data_size)
if (ntfs_cluster_free_from_rl(vol, rl) < 0) {
ntfs_error(vol->sb,
"Failed to release allocated cluster(s) in error code path. Run chkdsk to recover the lost cluster(s).");
+ /*
+ * The caller may hold the mrec_lock of the inode
+ * being modified, so the dirty bit cannot be
+ * written here; record the error flag only.
+ */
NVolSetErrors(vol);
}
kvfree(rl);
diff --git a/fs/ntfs/bitmap.c b/fs/ntfs/bitmap.c
index b1436b3151b9..6120cb301665 100644
--- a/fs/ntfs/bitmap.c
+++ b/fs/ntfs/bitmap.c
@@ -284,7 +284,7 @@ int __ntfs_bitmap_set_bits_in_run(struct inode *vi, const s64 start_bit,
ntfs_error(vi->i_sb,
"Failed to map subsequent page (error %i) and rollback failed (error %i). Aborting and leaving inconsistent metadata. Unmount and run chkdsk.",
err, pos);
- NVolSetErrors(NTFS_SB(vi->i_sb));
+ ntfs_mark_volume_dirty_with_error(NTFS_SB(vi->i_sb));
}
return err;
}
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 32edb4045178..0288f6a101dc 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -1245,7 +1245,7 @@ static int ntfs_read_locked_inode(struct inode *vi)
ntfs_error(vol->sb,
"Failed with error code %i. Marking corrupt inode 0x%llx as bad. Run chkdsk.",
err, ni->mft_no);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
return err;
}
@@ -1473,7 +1473,7 @@ static int ntfs_read_locked_attr_inode(struct inode *base_vi, struct inode *vi)
err, ni->mft_no, ni->type, ni->name_len,
base_ni->mft_no);
if (err != -ENOENT && err != -ENOMEM)
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
return err;
}
@@ -1725,7 +1725,7 @@ static int ntfs_read_locked_index_inode(struct inode *base_vi, struct inode *vi)
"Failed with error code %i while reading index inode (mft_no 0x%llx, name_len %i.",
err, ni->mft_no, ni->name_len);
if (err != -EOPNOTSUPP && err != -ENOMEM)
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
return err;
}
@@ -2883,7 +2883,7 @@ int __ntfs_write_inode(struct inode *vi, int sync)
mark_inode_dirty(vi);
else {
ntfs_error(vi->i_sb, "Failed (error %i): Run chkdsk.", -err);
- NVolSetErrors(ni->vol);
+ ntfs_mark_volume_dirty_with_error(ni->vol);
}
if (need_iput)
iput(vi);
diff --git a/fs/ntfs/lcnalloc.c b/fs/ntfs/lcnalloc.c
index aa2e017a4384..dd6854f16136 100644
--- a/fs/ntfs/lcnalloc.c
+++ b/fs/ntfs/lcnalloc.c
@@ -763,7 +763,7 @@ switch_to_data1_zone: search_zone = 2;
ntfs_error(vol->sb,
"Failed to rollback (error %i). Leaving inconsistent metadata! Unmount and run chkdsk.",
err2);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
/* Free the runlist. */
kvfree(rl);
@@ -1044,7 +1044,7 @@ s64 __ntfs_cluster_free(struct ntfs_inode *ni, const s64 start_vcn, s64 count,
ntfs_error(vol->sb,
"Failed to rollback (error %i). Leaving inconsistent metadata! Unmount and run chkdsk.",
(int)delta);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
ntfs_dec_free_clusters(vol, delta);
up_write(&vol->lcnbmp_lock);
diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 984a0827f9ac..d8791d7b9678 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -657,8 +657,15 @@ int write_mft_record_nolock(struct ntfs_inode *ni, struct mft_record *m, int syn
"Not enough memory to write mft record. Redirtying so the write is retried later.");
mark_mft_record_dirty(ni);
err = 0;
- } else
+ } else {
+ /*
+ * The writeback path can run with the caller's mrec_lock
+ * held, so the dirty bit cannot be written here (it would
+ * self-deadlock for the $Volume inode); record the error
+ * flag only.
+ */
NVolSetErrors(vol);
+ }
return err;
}
@@ -1150,7 +1157,7 @@ static int ntfs_mft_bitmap_extend_allocation_nolock(struct ntfs_volume *vol)
if (ntfs_cluster_free_from_rl(vol, rl2)) {
ntfs_error(vol->sb, "Failed to deallocate allocated cluster.%s",
es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
kvfree(rl2);
return PTR_ERR(rl);
@@ -1285,7 +1292,7 @@ static int ntfs_mft_bitmap_extend_allocation_nolock(struct ntfs_volume *vol)
* The only thing that is now wrong is ->allocated_size of the
* base attribute extent which chkdsk should be able to fix.
*/
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
return ret;
}
a = ctx->attr;
@@ -1306,7 +1313,7 @@ static int ntfs_mft_bitmap_extend_allocation_nolock(struct ntfs_volume *vol)
down_write(&vol->lcnbmp_lock);
if (ntfs_bitmap_clear_bit(vol->lcnbmp_ino, lcn)) {
ntfs_error(vol->sb, "Failed to free allocated cluster.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
} else
ntfs_inc_free_clusters(vol, 1);
up_write(&vol->lcnbmp_lock);
@@ -1317,16 +1324,16 @@ static int ntfs_mft_bitmap_extend_allocation_nolock(struct ntfs_volume *vol)
a->data.non_resident.mapping_pairs_offset),
rl2, ll, -1, NULL, NULL, NULL)) {
ntfs_error(vol->sb, "Failed to restore mapping pairs array.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {
ntfs_error(vol->sb, "Failed to restore attribute record.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
mark_mft_record_dirty(ctx->ntfs_ino);
} else if (status.mp_extended && ntfs_attr_update_mapping_pairs(mftbmp_ni, 0)) {
ntfs_error(vol->sb, "Failed to restore mapping pairs.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
if (ctx)
ntfs_attr_put_search_ctx(ctx);
@@ -1420,20 +1427,20 @@ static int ntfs_mft_bitmap_extend_initialized_nolock(struct ntfs_volume *vol)
mrec = map_mft_record(mft_ni);
if (IS_ERR(mrec)) {
ntfs_error(vol->sb, "Failed to map mft record.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
return ret;
}
ctx = ntfs_attr_get_search_ctx(mft_ni, mrec);
if (unlikely(!ctx)) {
ntfs_error(vol->sb, "Failed to get search context.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
goto unm_err_out;
}
if (ntfs_attr_lookup(mftbmp_ni->type, mftbmp_ni->name,
mftbmp_ni->name_len, CASE_SENSITIVE, 0, NULL, 0, ctx)) {
ntfs_error(vol->sb,
"Failed to find first attribute extent of mft bitmap attribute.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
put_err_out:
ntfs_attr_put_search_ctx(ctx);
unm_err_out:
@@ -1587,7 +1594,7 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
if (ntfs_cluster_free_from_rl(vol, rl2)) {
ntfs_error(vol->sb,
"Failed to deallocate clusters from the mft data attribute.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
kvfree(rl2);
return PTR_ERR(rl);
@@ -1721,7 +1728,7 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
* The only thing that is now wrong is ->allocated_size of the
* base attribute extent which chkdsk should be able to fix.
*/
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
return ret;
}
ctx->attr->data.non_resident.highest_vcn =
@@ -1729,17 +1736,17 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
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);
+ ntfs_mark_volume_dirty_with_error(vol);
}
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);
+ ntfs_mark_volume_dirty_with_error(vol);
}
if (mp_extended && ntfs_attr_update_mapping_pairs(mft_ni, 0)) {
ntfs_error(vol->sb, "Failed to restore mapping pairs.%s",
es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
if (ctx) {
a = ctx->attr;
@@ -1750,16 +1757,16 @@ static int ntfs_mft_data_extend_allocation_nolock(struct ntfs_volume *vol)
a->data.non_resident.mapping_pairs_offset),
rl2, ll, -1, NULL, NULL, NULL)) {
ntfs_error(vol->sb, "Failed to restore mapping pairs array.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
if (ntfs_attr_record_resize(ctx->mrec, a, old_alen)) {
ntfs_error(vol->sb, "Failed to restore attribute record.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
mark_mft_record_dirty(ctx->ntfs_ino);
} else if (IS_ERR(ctx->mrec)) {
ntfs_error(vol->sb, "Failed to restore attribute search context.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
ntfs_attr_put_search_ctx(ctx);
}
@@ -2322,7 +2329,7 @@ int ntfs_mft_record_alloc(struct ntfs_volume *vol, const int mode,
folio_unlock(folio);
kunmap_local(m);
folio_put(folio);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
goto search_free_rec;
}
/*
@@ -2475,7 +2482,7 @@ int ntfs_mft_record_alloc(struct ntfs_volume *vol, const int mode,
undo_mftbmp_alloc_nolock:
if (ntfs_bitmap_clear_bit(vol->mftbmp_ino, bit)) {
ntfs_error(vol->sb, "Failed to clear bit in mft bitmap.%s", es);
- NVolSetErrors(vol);
+ ntfs_mark_volume_dirty_with_error(vol);
}
if (!base_ni || base_ni->mft_no != FILE_MFT)
up_write(&vol->mftbmp_lock);
@@ -2815,8 +2822,14 @@ static int ntfs_write_mft_block(struct folio *folio, struct writeback_control *w
iput(ref_inos[nr_ref_inos]);
}
- if (unlikely(err && err != -ENOMEM))
+ if (unlikely(err && err != -ENOMEM)) {
+ /*
+ * The writeback path can run with a caller's mrec_lock
+ * held, so the dirty bit cannot be written here; record
+ * the error flag only.
+ */
NVolSetErrors(vol);
+ }
if (likely(!err))
ntfs_debug("Done.");
return err;
--
2.25.1