* [PATCH 1/3] ntfs: fix volume flag update races
@ 2026-09-03 2:19 Hongling Zeng
2026-09-03 2:19 ` [PATCH 2/3] ntfs: add helper to record error flag before setting volume dirty bit Hongling Zeng
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Hongling Zeng @ 2026-09-03 2:19 UTC (permalink / raw)
To: linkinjeon, hyc.lee
Cc: ntfs, linux-kernel, zhongling0719, Hongling Zeng, stable
ntfs_set_volume_flags() and ntfs_clear_volume_flags() both read
vol->vol_flags outside any lock to compute the new value before handing
it to ntfs_write_volume_flags(), which only takes ni->mrec_lock around
the actual write. The read-modify-write is therefore not atomic, and two
concurrent callers can lose an update: ntfs_sync_fs() may derive a
"clean" value from vol->vol_flags while a writer concurrently records an
error and sets VOLUME_IS_DIRTY; the locked write then silently
overwrites the freshly-set dirty bit. The on-disk volume looks clean
despite the recorded errors, so chkdsk will not run on the next mount
and corrupted metadata can persist.
Fix by moving the read-modify-write inside the mrec_lock: pass the bits
to set and to clear separately, and combine them with the current flag
state under the lock inside ntfs_write_volume_flags(). The set/clear
helpers pass only the bits to modify, not the complete flag state. The
bit manipulation is done on CPU-endian values, and the result is
converted back to little-endian before storing it. The wrappers keep
their signatures so callers are unchanged.
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
- Also fix the ntfs_sync_fs() race by checking NVolErrors() and clearing
VOLUME_IS_DIRTY under ni->mrec_lock.
- Keep ntfs_set_volume_flags() and ntfs_clear_volume_flags() semantics
unchanged.
- Do not tie setting VOLUME_IS_DIRTY to NVolSetErrors() in the generic
set helper.
---
fs/ntfs/super.c | 62 +++++++++++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 17 deletions(-)
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index a1813093222b..a7977b95b967 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -353,31 +353,45 @@ void ntfs_handle_error(struct super_block *sb)
}
/*
- * ntfs_write_volume_flags - write new flags to the volume information flags
+ * ntfs_write_volume_flags - apply flag changes to the volume information flags
* @vol: ntfs volume on which to modify the flags
- * @flags: new flags value for the volume information flags
+ * @set_bits: bits to set in the volume information flags
+ * @clear_bits: bits to clear in the volume information flags
*
* Internal function. You probably want to use ntfs_{set,clear}_volume_flags()
* instead (see below).
*
- * Replace the volume information flags on the volume @vol with the value
- * supplied in @flags. Note, this overwrites the volume information flags, so
- * make sure to combine the flags you want to modify with the old flags and use
- * the result when calling ntfs_write_volume_flags().
+ * Combine @set_bits and @clear_bits with the current in-memory flag state and
+ * write the result back. The set/clear helpers pass only the bits to modify,
+ * not the complete flag state. The read-modify-write happens under
+ * ni->mrec_lock so that concurrent set/clear operations cannot lose updates.
+ * All bit manipulation is done on CPU-endian values, and the result is
+ * converted back to little-endian before storing it.
*
* Return 0 on success and -errno on error.
*/
-static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
+static int ntfs_write_volume_flags(struct ntfs_volume *vol,
+ const __le16 set_bits, const __le16 clear_bits,
+ const bool skip_if_errors)
{
struct ntfs_inode *ni = NTFS_I(vol->vol_ino);
struct volume_information *vi;
struct ntfs_attr_search_ctx *ctx;
+ u16 flags;
int err;
- ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
- le16_to_cpu(vol->vol_flags), le16_to_cpu(flags));
mutex_lock(&ni->mrec_lock);
- if (vol->vol_flags == flags)
+
+ if (skip_if_errors && NVolErrors(vol))
+ goto done;
+
+ flags = le16_to_cpu(vol->vol_flags);
+ flags |= le16_to_cpu(set_bits) & le16_to_cpu(VOLUME_FLAGS_MASK);
+ flags &= ~(le16_to_cpu(clear_bits) & le16_to_cpu(VOLUME_FLAGS_MASK));
+ ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
+ le16_to_cpu(vol->vol_flags), flags);
+
+ if (le16_to_cpu(vol->vol_flags) == flags)
goto done;
ctx = ntfs_attr_get_search_ctx(ni, NULL);
@@ -393,7 +407,7 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
vi = (struct volume_information *)((u8 *)ctx->attr +
le16_to_cpu(ctx->attr->data.resident.value_offset));
- vol->vol_flags = vi->flags = flags;
+ vol->vol_flags = vi->flags = cpu_to_le16(flags);
mark_mft_record_dirty(ctx->ntfs_ino);
ntfs_attr_put_search_ctx(ctx);
done:
@@ -414,13 +428,14 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
* @flags: flags to set on the volume
*
* Set the bits in @flags in the volume information flags on the volume @vol.
+ * The bits are combined with the current flag state under the lock in
+ * ntfs_write_volume_flags(), so concurrent updates are not lost.
*
* Return 0 on success and -errno on error.
*/
int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
{
- flags &= VOLUME_FLAGS_MASK;
- return ntfs_write_volume_flags(vol, vol->vol_flags | flags);
+ return ntfs_write_volume_flags(vol, flags, 0, false);
}
/*
@@ -429,14 +444,27 @@ int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
* @flags: flags to clear on the volume
*
* Clear the bits in @flags in the volume information flags on the volume @vol.
+ * The bits are combined with the current flag state under the lock in
+ * ntfs_write_volume_flags(), so concurrent updates are not lost.
*
* Return 0 on success and -errno on error.
*/
int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags)
{
- flags &= VOLUME_FLAGS_MASK;
- flags = vol->vol_flags & cpu_to_le16(~le16_to_cpu(flags));
- return ntfs_write_volume_flags(vol, flags);
+ return ntfs_write_volume_flags(vol, 0, flags, false);
+}
+
+/*
+ * ntfs_clear_volume_dirty_if_no_errors - clear dirty bit if no errors exist
+ * @vol: ntfs volume whose dirty bit should be cleared
+ *
+ * Check NVolErrors() and clear VOLUME_IS_DIRTY under the same mrec_lock so
+ * ntfs_sync_fs() cannot clear the dirty bit after a concurrent error has been
+ * recorded.
+ */
+static int ntfs_clear_volume_dirty_if_no_errors(struct ntfs_volume *vol)
+{
+ return ntfs_write_volume_flags(vol, 0, VOLUME_IS_DIRTY, true);
}
int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
@@ -1862,7 +1890,7 @@ static int ntfs_sync_fs(struct super_block *sb, int wait)
return 0;
/* If there are some dirty buffers in the bdev inode */
- if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY)) {
+ if (ntfs_clear_volume_dirty_if_no_errors(vol)) {
ntfs_warning(sb, "Failed to clear dirty bit in volume information flags. Run chkdsk.");
err = -EIO;
}
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/3] ntfs: add helper to record error flag before setting volume dirty bit
2026-09-03 2:19 [PATCH 1/3] ntfs: fix volume flag update races Hongling Zeng
@ 2026-09-03 2:19 ` Hongling Zeng
2026-09-03 2:19 ` [PATCH 3/3] ntfs: convert runtime error paths to mark volume dirty after recording errors Hongling Zeng
2026-09-03 2:21 ` [PATCH 1/3] ntfs: fix volume flag update races Hongling Zeng
2 siblings, 0 replies; 5+ messages in thread
From: Hongling Zeng @ 2026-09-03 2:19 UTC (permalink / raw)
To: linkinjeon, hyc.lee
Cc: ntfs, linux-kernel, zhongling0719, Hongling Zeng, stable
The runtime metadata-corruption paths in fs/ntfs record the error flag
with a bare NVolSetErrors() and separately rely on ntfs_set_volume_flags()
or nothing at all to persist VOLUME_IS_DIRTY. Since the clear side
(ntfs_sync_fs() via ntfs_clear_volume_dirty_if_no_errors()) evaluates
NVolErrors() under ni->mrec_lock, a writer that records the error flag
after the lock has been checked leaves a window in which the volume ends
up persisted as clean despite the recorded error, so chkdsk will not run
on the next mount and corrupted metadata can persist.
Introduce ntfs_mark_volume_dirty_with_error(), which records the error
flag before the locked dirty-bit write: the mutex acquire/release in
ntfs_write_volume_flags() then provides the ordering, as any clear path
running after us can only evaluate NVolErrors() under the lock once the
flag is set.
The helper is only for runtime error paths that can run concurrently
with sync and hold no mrec_lock. Mount and remount paths remain
serialized by sb->s_umount and keep using NVolSetErrors() directly,
since they do not race with ntfs_sync_fs() and may not be allowed to
write the volume: before $Volume is loaded, with a read-only opened
bdev, or on a hibernated volume, which we must not write to at all.
The same applies to the ntfs_attr_lookup() failure paths and the mft
record writeback paths, which hold a caller's mrec_lock and would
self-deadlock the helper on the $Volume inode. The helper also returns
success without doing anything if vol_ino is still NULL during mount, a
state no runtime caller can observe.
Converting the runtime metadata-corruption call sites in mft.c,
lcnalloc.c, bitmap.c and inode.c to use the helper is done in the
next patch.
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
fs/ntfs/ntfs.h | 1 +
fs/ntfs/super.c | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/fs/ntfs/ntfs.h b/fs/ntfs/ntfs.h
index df5a75d506f6..63d7900b6dba 100644
--- a/fs/ntfs/ntfs.h
+++ b/fs/ntfs/ntfs.h
@@ -222,6 +222,7 @@ struct option_t {
extern const struct option_t on_errors_arr[];
int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags);
int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags);
+int ntfs_mark_volume_dirty_with_error(struct ntfs_volume *vol);
int ntfs_write_volume_label(struct ntfs_volume *vol, char *label);
/* From fs/ntfs/mst.c */
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index a7977b95b967..fcf327ac98e5 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -467,6 +467,44 @@ static int ntfs_clear_volume_dirty_if_no_errors(struct ntfs_volume *vol)
return ntfs_write_volume_flags(vol, 0, VOLUME_IS_DIRTY, true);
}
+/*
+ * ntfs_mark_volume_dirty_with_error - record an error and mark volume dirty
+ * @vol: ntfs volume on which an error has been recorded
+ *
+ * To be called when runtime metadata corruption is detected so that chkdsk
+ * runs on the next mount. NVolSetErrors() is called before the dirty bit
+ * is written, and the mutex acquire/release in ntfs_write_volume_flags()
+ * provides the ordering: a concurrent ntfs_clear_volume_dirty_if_no_errors()
+ * can only run under the lock after the error flag is set and will
+ * therefore leave the dirty bit alone.
+ *
+ * Only for runtime error paths that can run concurrently with sync and
+ * hold no mrec_lock. Do not call while holding the mrec_lock of the
+ * $Volume inode itself (the ntfs_attr_lookup() failure paths and the mft
+ * record writeback paths, which may hold it). Mount and remount paths
+ * are serialized by sb->s_umount and cannot race sync; error paths that
+ * run before $Volume is loaded, that may run with a read-only opened
+ * bdev, or on a hibernated volume, which we must not write to at all,
+ * keep calling NVolSetErrors() directly.
+ *
+ * Return 0 on success and -errno on error.
+ */
+int ntfs_mark_volume_dirty_with_error(struct ntfs_volume *vol)
+{
+ /*
+ * vol_ino is NULL while the volume is still being mounted. This is a
+ * runtime-only helper and no runtime caller can see that state, but if
+ * one ever does, there is nothing on disk to update yet, so do nothing
+ * rather than dereference a NULL inode. Mount-time error paths record
+ * the error flag with NVolSetErrors() directly instead.
+ */
+ if (!vol->vol_ino)
+ return 0;
+
+ NVolSetErrors(vol);
+ return ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY);
+}
+
int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
{
struct ntfs_inode *vol_ni = NTFS_I(vol->vol_ino);
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] ntfs: convert runtime error paths to mark volume dirty after recording errors
2026-09-03 2:19 [PATCH 1/3] ntfs: fix volume flag update races Hongling Zeng
2026-09-03 2:19 ` [PATCH 2/3] ntfs: add helper to record error flag before setting volume dirty bit Hongling Zeng
@ 2026-09-03 2:19 ` Hongling Zeng
2026-09-03 2:21 ` [PATCH 1/3] ntfs: fix volume flag update races Hongling Zeng
2 siblings, 0 replies; 5+ messages in thread
From: Hongling Zeng @ 2026-09-03 2:19 UTC (permalink / raw)
To: linkinjeon, hyc.lee
Cc: ntfs, linux-kernel, zhongling0719, Hongling Zeng, stable
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@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] ntfs: fix volume flag update races
2026-09-03 2:19 [PATCH 1/3] ntfs: fix volume flag update races Hongling Zeng
2026-09-03 2:19 ` [PATCH 2/3] ntfs: add helper to record error flag before setting volume dirty bit Hongling Zeng
2026-09-03 2:19 ` [PATCH 3/3] ntfs: convert runtime error paths to mark volume dirty after recording errors Hongling Zeng
@ 2026-09-03 2:21 ` Hongling Zeng
2026-09-03 2:59 ` liubaolin
2 siblings, 1 reply; 5+ messages in thread
From: Hongling Zeng @ 2026-09-03 2:21 UTC (permalink / raw)
To: Hongling Zeng, linkinjeon, hyc.lee; +Cc: ntfs, linux-kernel, stable
Hi,
Sorry for the many versions of this patch.
The ntfs_sync_fs() race you pointed out is fixed in patch 1/3:
NVolErrors() is checked and VOLUME_IS_DIRTY is cleared inside the same
mrec_lock critical section, so every interleaving with a concurrent
error path
leaves the volume dirty on disk.
Patches 2/3 and 3/3 close the writer-side window: the runtime error
paths now record NVolErrors() before taking the mrec_lock to persist
VOLUME_IS_DIRTY, so a clear path running afterwards sees the flag under
the lock
and leaves the dirty bit alone.
Looking forward to your review and feedback.
Thanks,
Hongling
在 2026年09月03日 10:19, Hongling Zeng 写道:
> ntfs_set_volume_flags() and ntfs_clear_volume_flags() both read
> vol->vol_flags outside any lock to compute the new value before handing
> it to ntfs_write_volume_flags(), which only takes ni->mrec_lock around
> the actual write. The read-modify-write is therefore not atomic, and two
> concurrent callers can lose an update: ntfs_sync_fs() may derive a
> "clean" value from vol->vol_flags while a writer concurrently records an
> error and sets VOLUME_IS_DIRTY; the locked write then silently
> overwrites the freshly-set dirty bit. The on-disk volume looks clean
> despite the recorded errors, so chkdsk will not run on the next mount
> and corrupted metadata can persist.
>
> Fix by moving the read-modify-write inside the mrec_lock: pass the bits
> to set and to clear separately, and combine them with the current flag
> state under the lock inside ntfs_write_volume_flags(). The set/clear
> helpers pass only the bits to modify, not the complete flag state. The
> bit manipulation is done on CPU-endian values, and the result is
> converted back to little-endian before storing it. The wrappers keep
> their signatures so callers are unchanged.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
> - Also fix the ntfs_sync_fs() race by checking NVolErrors() and clearing
> VOLUME_IS_DIRTY under ni->mrec_lock.
> - Keep ntfs_set_volume_flags() and ntfs_clear_volume_flags() semantics
> unchanged.
> - Do not tie setting VOLUME_IS_DIRTY to NVolSetErrors() in the generic
> set helper.
> ---
> fs/ntfs/super.c | 62 +++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 45 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index a1813093222b..a7977b95b967 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -353,31 +353,45 @@ void ntfs_handle_error(struct super_block *sb)
> }
>
> /*
> - * ntfs_write_volume_flags - write new flags to the volume information flags
> + * ntfs_write_volume_flags - apply flag changes to the volume information flags
> * @vol: ntfs volume on which to modify the flags
> - * @flags: new flags value for the volume information flags
> + * @set_bits: bits to set in the volume information flags
> + * @clear_bits: bits to clear in the volume information flags
> *
> * Internal function. You probably want to use ntfs_{set,clear}_volume_flags()
> * instead (see below).
> *
> - * Replace the volume information flags on the volume @vol with the value
> - * supplied in @flags. Note, this overwrites the volume information flags, so
> - * make sure to combine the flags you want to modify with the old flags and use
> - * the result when calling ntfs_write_volume_flags().
> + * Combine @set_bits and @clear_bits with the current in-memory flag state and
> + * write the result back. The set/clear helpers pass only the bits to modify,
> + * not the complete flag state. The read-modify-write happens under
> + * ni->mrec_lock so that concurrent set/clear operations cannot lose updates.
> + * All bit manipulation is done on CPU-endian values, and the result is
> + * converted back to little-endian before storing it.
> *
> * Return 0 on success and -errno on error.
> */
> -static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
> +static int ntfs_write_volume_flags(struct ntfs_volume *vol,
> + const __le16 set_bits, const __le16 clear_bits,
> + const bool skip_if_errors)
> {
> struct ntfs_inode *ni = NTFS_I(vol->vol_ino);
> struct volume_information *vi;
> struct ntfs_attr_search_ctx *ctx;
> + u16 flags;
> int err;
>
> - ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
> - le16_to_cpu(vol->vol_flags), le16_to_cpu(flags));
> mutex_lock(&ni->mrec_lock);
> - if (vol->vol_flags == flags)
> +
> + if (skip_if_errors && NVolErrors(vol))
> + goto done;
> +
> + flags = le16_to_cpu(vol->vol_flags);
> + flags |= le16_to_cpu(set_bits) & le16_to_cpu(VOLUME_FLAGS_MASK);
> + flags &= ~(le16_to_cpu(clear_bits) & le16_to_cpu(VOLUME_FLAGS_MASK));
> + ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
> + le16_to_cpu(vol->vol_flags), flags);
> +
> + if (le16_to_cpu(vol->vol_flags) == flags)
> goto done;
>
> ctx = ntfs_attr_get_search_ctx(ni, NULL);
> @@ -393,7 +407,7 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
>
> vi = (struct volume_information *)((u8 *)ctx->attr +
> le16_to_cpu(ctx->attr->data.resident.value_offset));
> - vol->vol_flags = vi->flags = flags;
> + vol->vol_flags = vi->flags = cpu_to_le16(flags);
> mark_mft_record_dirty(ctx->ntfs_ino);
> ntfs_attr_put_search_ctx(ctx);
> done:
> @@ -414,13 +428,14 @@ static int ntfs_write_volume_flags(struct ntfs_volume *vol, const __le16 flags)
> * @flags: flags to set on the volume
> *
> * Set the bits in @flags in the volume information flags on the volume @vol.
> + * The bits are combined with the current flag state under the lock in
> + * ntfs_write_volume_flags(), so concurrent updates are not lost.
> *
> * Return 0 on success and -errno on error.
> */
> int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
> {
> - flags &= VOLUME_FLAGS_MASK;
> - return ntfs_write_volume_flags(vol, vol->vol_flags | flags);
> + return ntfs_write_volume_flags(vol, flags, 0, false);
> }
>
> /*
> @@ -429,14 +444,27 @@ int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
> * @flags: flags to clear on the volume
> *
> * Clear the bits in @flags in the volume information flags on the volume @vol.
> + * The bits are combined with the current flag state under the lock in
> + * ntfs_write_volume_flags(), so concurrent updates are not lost.
> *
> * Return 0 on success and -errno on error.
> */
> int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags)
> {
> - flags &= VOLUME_FLAGS_MASK;
> - flags = vol->vol_flags & cpu_to_le16(~le16_to_cpu(flags));
> - return ntfs_write_volume_flags(vol, flags);
> + return ntfs_write_volume_flags(vol, 0, flags, false);
> +}
> +
> +/*
> + * ntfs_clear_volume_dirty_if_no_errors - clear dirty bit if no errors exist
> + * @vol: ntfs volume whose dirty bit should be cleared
> + *
> + * Check NVolErrors() and clear VOLUME_IS_DIRTY under the same mrec_lock so
> + * ntfs_sync_fs() cannot clear the dirty bit after a concurrent error has been
> + * recorded.
> + */
> +static int ntfs_clear_volume_dirty_if_no_errors(struct ntfs_volume *vol)
> +{
> + return ntfs_write_volume_flags(vol, 0, VOLUME_IS_DIRTY, true);
> }
>
> int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
> @@ -1862,7 +1890,7 @@ static int ntfs_sync_fs(struct super_block *sb, int wait)
> return 0;
>
> /* If there are some dirty buffers in the bdev inode */
> - if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY)) {
> + if (ntfs_clear_volume_dirty_if_no_errors(vol)) {
> ntfs_warning(sb, "Failed to clear dirty bit in volume information flags. Run chkdsk.");
> err = -EIO;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] ntfs: fix volume flag update races
2026-09-03 2:21 ` [PATCH 1/3] ntfs: fix volume flag update races Hongling Zeng
@ 2026-09-03 2:59 ` liubaolin
0 siblings, 0 replies; 5+ messages in thread
From: liubaolin @ 2026-09-03 2:59 UTC (permalink / raw)
To: Hongling Zeng, Hongling Zeng, linkinjeon, hyc.lee
Cc: ntfs, linux-kernel, stable
在 2026/9/3 10:21, Hongling Zeng 写道:
> Hi,
> Sorry for the many versions of this patch.
Hi Hongling,
A small suggestion for operation:
if you want to send a later version patch like v2, when generating
the patch, use "git format patch -- subject prefix='PATCH v2 '-- cover
letter - v2-3" and add "-- subject prefix='PATCH v2'" and "- v2".
This way, others can see at a glance what version of the email your
patch is, making it easier to review.
Thanks，
Baolin
>
> The ntfs_sync_fs() race you pointed out is fixed in patch 1/3:
> NVolErrors() is checked and VOLUME_IS_DIRTY is cleared inside the same
> mrec_lock critical section, so every interleaving with a concurrent
> error path
> leaves the volume dirty on disk.
>
> Patches 2/3 and 3/3 close the writer-side window: the runtime error
> paths now record NVolErrors() before taking the mrec_lock to persist
> VOLUME_IS_DIRTY, so a clear path running afterwards sees the flag under
> the lock
> and leaves the dirty bit alone.
>
> Looking forward to your review and feedback.
>
> Thanks,
> Hongling
>
> 在 2026年09月03日 10:19, Hongling Zeng 写道:
>> ntfs_set_volume_flags() and ntfs_clear_volume_flags() both read
>> vol->vol_flags outside any lock to compute the new value before handing
>> it to ntfs_write_volume_flags(), which only takes ni->mrec_lock around
>> the actual write. The read-modify-write is therefore not atomic, and two
>> concurrent callers can lose an update: ntfs_sync_fs() may derive a
>> "clean" value from vol->vol_flags while a writer concurrently records an
>> error and sets VOLUME_IS_DIRTY; the locked write then silently
>> overwrites the freshly-set dirty bit. The on-disk volume looks clean
>> despite the recorded errors, so chkdsk will not run on the next mount
>> and corrupted metadata can persist.
>>
>> Fix by moving the read-modify-write inside the mrec_lock: pass the bits
>> to set and to clear separately, and combine them with the current flag
>> state under the lock inside ntfs_write_volume_flags(). The set/clear
>> helpers pass only the bits to modify, not the complete flag state. The
>> bit manipulation is done on CPU-endian values, and the result is
>> converted back to little-endian before storing it. The wrappers keep
>> their signatures so callers are unchanged.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>> - Also fix the ntfs_sync_fs() race by checking NVolErrors() and clearing
>> VOLUME_IS_DIRTY under ni->mrec_lock.
>> - Keep ntfs_set_volume_flags() and ntfs_clear_volume_flags() semantics
>> unchanged.
>> - Do not tie setting VOLUME_IS_DIRTY to NVolSetErrors() in the generic
>> set helper.
>> ---
>> fs/ntfs/super.c | 62 +++++++++++++++++++++++++++++++++++--------------
>> 1 file changed, 45 insertions(+), 17 deletions(-)
>>
>> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
>> index a1813093222b..a7977b95b967 100644
>> --- a/fs/ntfs/super.c
>> +++ b/fs/ntfs/super.c
>> @@ -353,31 +353,45 @@ void ntfs_handle_error(struct super_block *sb)
>> }
>> /*
>> - * ntfs_write_volume_flags - write new flags to the volume
>> information flags
>> + * ntfs_write_volume_flags - apply flag changes to the volume
>> information flags
>> * @vol: ntfs volume on which to modify the flags
>> - * @flags: new flags value for the volume information flags
>> + * @set_bits: bits to set in the volume information flags
>> + * @clear_bits: bits to clear in the volume information flags
>> *
>> * Internal function. You probably want to use ntfs_{set,clear}
>> _volume_flags()
>> * instead (see below).
>> *
>> - * Replace the volume information flags on the volume @vol with the
>> value
>> - * supplied in @flags. Note, this overwrites the volume information
>> flags, so
>> - * make sure to combine the flags you want to modify with the old
>> flags and use
>> - * the result when calling ntfs_write_volume_flags().
>> + * Combine @set_bits and @clear_bits with the current in-memory flag
>> state and
>> + * write the result back. The set/clear helpers pass only the bits
>> to modify,
>> + * not the complete flag state. The read-modify-write happens under
>> + * ni->mrec_lock so that concurrent set/clear operations cannot lose
>> updates.
>> + * All bit manipulation is done on CPU-endian values, and the result is
>> + * converted back to little-endian before storing it.
>> *
>> * Return 0 on success and -errno on error.
>> */
>> -static int ntfs_write_volume_flags(struct ntfs_volume *vol, const
>> __le16 flags)
>> +static int ntfs_write_volume_flags(struct ntfs_volume *vol,
>> + const __le16 set_bits, const __le16 clear_bits,
>> + const bool skip_if_errors)
>> {
>> struct ntfs_inode *ni = NTFS_I(vol->vol_ino);
>> struct volume_information *vi;
>> struct ntfs_attr_search_ctx *ctx;
>> + u16 flags;
>> int err;
>> - ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
>> - le16_to_cpu(vol->vol_flags), le16_to_cpu(flags));
>> mutex_lock(&ni->mrec_lock);
>> - if (vol->vol_flags == flags)
>> +
>> + if (skip_if_errors && NVolErrors(vol))
>> + goto done;
>> +
>> + flags = le16_to_cpu(vol->vol_flags);
>> + flags |= le16_to_cpu(set_bits) & le16_to_cpu(VOLUME_FLAGS_MASK);
>> + flags &= ~(le16_to_cpu(clear_bits) &
>> le16_to_cpu(VOLUME_FLAGS_MASK));
>> + ntfs_debug("Entering, old flags = 0x%x, new flags = 0x%x.",
>> + le16_to_cpu(vol->vol_flags), flags);
>> +
>> + if (le16_to_cpu(vol->vol_flags) == flags)
>> goto done;
>> ctx = ntfs_attr_get_search_ctx(ni, NULL);
>> @@ -393,7 +407,7 @@ static int ntfs_write_volume_flags(struct
>> ntfs_volume *vol, const __le16 flags)
>> vi = (struct volume_information *)((u8 *)ctx->attr +
>> le16_to_cpu(ctx->attr->data.resident.value_offset));
>> - vol->vol_flags = vi->flags = flags;
>> + vol->vol_flags = vi->flags = cpu_to_le16(flags);
>> mark_mft_record_dirty(ctx->ntfs_ino);
>> ntfs_attr_put_search_ctx(ctx);
>> done:
>> @@ -414,13 +428,14 @@ static int ntfs_write_volume_flags(struct
>> ntfs_volume *vol, const __le16 flags)
>> * @flags: flags to set on the volume
>> *
>> * Set the bits in @flags in the volume information flags on the
>> volume @vol.
>> + * The bits are combined with the current flag state under the lock in
>> + * ntfs_write_volume_flags(), so concurrent updates are not lost.
>> *
>> * Return 0 on success and -errno on error.
>> */
>> int ntfs_set_volume_flags(struct ntfs_volume *vol, __le16 flags)
>> {
>> - flags &= VOLUME_FLAGS_MASK;
>> - return ntfs_write_volume_flags(vol, vol->vol_flags | flags);
>> + return ntfs_write_volume_flags(vol, flags, 0, false);
>> }
>> /*
>> @@ -429,14 +444,27 @@ int ntfs_set_volume_flags(struct ntfs_volume
>> *vol, __le16 flags)
>> * @flags: flags to clear on the volume
>> *
>> * Clear the bits in @flags in the volume information flags on the
>> volume @vol.
>> + * The bits are combined with the current flag state under the lock in
>> + * ntfs_write_volume_flags(), so concurrent updates are not lost.
>> *
>> * Return 0 on success and -errno on error.
>> */
>> int ntfs_clear_volume_flags(struct ntfs_volume *vol, __le16 flags)
>> {
>> - flags &= VOLUME_FLAGS_MASK;
>> - flags = vol->vol_flags & cpu_to_le16(~le16_to_cpu(flags));
>> - return ntfs_write_volume_flags(vol, flags);
>> + return ntfs_write_volume_flags(vol, 0, flags, false);
>> +}
>> +
>> +/*
>> + * ntfs_clear_volume_dirty_if_no_errors - clear dirty bit if no
>> errors exist
>> + * @vol: ntfs volume whose dirty bit should be cleared
>> + *
>> + * Check NVolErrors() and clear VOLUME_IS_DIRTY under the same
>> mrec_lock so
>> + * ntfs_sync_fs() cannot clear the dirty bit after a concurrent error
>> has been
>> + * recorded.
>> + */
>> +static int ntfs_clear_volume_dirty_if_no_errors(struct ntfs_volume *vol)
>> +{
>> + return ntfs_write_volume_flags(vol, 0, VOLUME_IS_DIRTY, true);
>> }
>> int ntfs_write_volume_label(struct ntfs_volume *vol, char *label)
>> @@ -1862,7 +1890,7 @@ static int ntfs_sync_fs(struct super_block *sb,
>> int wait)
>> return 0;
>> /* If there are some dirty buffers in the bdev inode */
>> - if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY)) {
>> + if (ntfs_clear_volume_dirty_if_no_errors(vol)) {
>> ntfs_warning(sb, "Failed to clear dirty bit in volume
>> information flags. Run chkdsk.");
>> err = -EIO;
>> }
>
Hi Hongling,
Patch 1 fixes the race in ntfs_sync_fs(), but there are two other
sites with the same check-outside-lock-then-clear-inside-lock pattern:
1. super.c:315 (ntfs_reconfigure, remount read-only path)
2. super.c:1755 (ntfs_put_super, umount path)
Both use:
if (!NVolErrors(vol)) { // check
outside lock
if (ntfs_clear_volume_flags(vol, VOLUME_IS_DIRTY)) // clear
inside lock
The same race can occur: if an error thread sets NVolErrors() and the
dirty bit after the check but before the lock acquisition, the clear
operation will overwrite the freshly-set dirty bit.
Although the race window is much narrower for remount/umount, should
these two sites also be converted to use
ntfs_clear_volume_dirty_if_no_errors() for consistency?
Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>
Thanks,
Baolin.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-03 2:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-03 2:19 [PATCH 1/3] ntfs: fix volume flag update races Hongling Zeng
2026-09-03 2:19 ` [PATCH 2/3] ntfs: add helper to record error flag before setting volume dirty bit Hongling Zeng
2026-09-03 2:19 ` [PATCH 3/3] ntfs: convert runtime error paths to mark volume dirty after recording errors Hongling Zeng
2026-09-03 2:21 ` [PATCH 1/3] ntfs: fix volume flag update races Hongling Zeng
2026-09-03 2:59 ` liubaolin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®