[PATCH 2/3] ntfs: add helper to record error flag before setting volume dirty bit

[Hongling Zeng]

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@xxxxxxxxxxxxxxx
Signed-off-by: Hongling Zeng <zenghongling@xxxxxxxxxx>
---
 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