A failed synchronous FAT metadata write clears BH_Uptodate. Rename
recovery can then modify and dirty the same retained buffer, which
triggers the !buffer_uptodate warning in mark_buffer_dirty().

Stop on-disk rollback after a failed dotdot update or old-entry removal,
where a complete rollback can no longer be guaranteed.

An inode-position write can also fail through another associated metadata
buffer. In that case the retained new-entry buffer remains uptodate, so
remove the added entry directly. Check the buffer state and dirty the
entry while holding the buffer lock. If the entry buffer itself failed
writeback, leave it untouched instead of reading it again and submitting
another write from FAT.

Keep the existing replacement and exchange inode-position recovery. When
recovery starts after an exchange position write fails, attempt both
restorations so a failure of one does not suppress the other.

Report recovery errors as corruption and preserve the original forward
error.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+b0aebd03565f5774f7f8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b0aebd03565f5774f7f8
Signed-off-by: Krystian Kaniewski <krystianmkaniewski@gmail.com>
---
Changes in v4:
- Trust a final block-layer write error. Do not reread or retry the retained
  new-entry buffer from FAT when it is no longer uptodate.
- Remove the added entry directly when its retained buffer remains uptodate.
  Hold the buffer lock across the state check, entry mutation and dirty mark.
- Keep failed dotdot writes and failed old-entry removal on the v3
  no-rollback path.

v3: https://lore.kernel.org/all/20261014082315.190262-1-krystianmkaniewski@gmail.com/

 fs/fat/dir.c         | 27 +++++++++++++++++--
 fs/fat/fat.h         |  2 ++
 fs/fat/namei_msdos.c | 37 +++++++++----------------
 fs/fat/namei_vfat.c  | 64 ++++++++++++++++----------------------------
 4 files changed, 62 insertions(+), 68 deletions(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 35bdb62944a2e..70433361359cb 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1066,7 +1066,9 @@ static int __fat_remove_entries(struct inode *dir, loff_t pos, int nr_slots)
 	return err;
 }
 
-int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
+static int fat_remove_entries_internal(struct inode *dir,
+				       struct fat_slot_info *sinfo,
+				       bool require_uptodate)
 {
 	struct super_block *sb = dir->i_sb;
 	struct msdos_dir_entry *de;
@@ -1079,8 +1081,15 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
 	 */
 	nr_slots = sinfo->nr_slots;
 	de = sinfo->de;
-	sinfo->de = NULL;
 	bh = sinfo->bh;
+	if (require_uptodate) {
+		lock_buffer(bh);
+		if (!buffer_uptodate(bh)) {
+			unlock_buffer(bh);
+			return -EIO;
+		}
+	}
+	sinfo->de = NULL;
 	sinfo->bh = NULL;
 	while (nr_slots && de >= (struct msdos_dir_entry *)bh->b_data) {
 		de->name[0] = DELETED_FLAG;
@@ -1088,6 +1097,8 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
 		nr_slots--;
 	}
 	mmb_mark_buffer_dirty(bh, &MSDOS_I(dir)->i_metadata_bhs);
+	if (require_uptodate)
+		unlock_buffer(bh);
 	if (IS_DIRSYNC(dir))
 		err = sync_dirty_buffer(bh);
 	brelse(bh);
@@ -1116,8 +1127,20 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
 
 	return 0;
 }
+
+int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
+{
+	return fat_remove_entries_internal(dir, sinfo, false);
+}
 EXPORT_SYMBOL_GPL(fat_remove_entries);
 
+int fat_remove_entries_if_uptodate(struct inode *dir,
+				   struct fat_slot_info *sinfo)
+{
+	return fat_remove_entries_internal(dir, sinfo, true);
+}
+EXPORT_SYMBOL_GPL(fat_remove_entries_if_uptodate);
+
 static int fat_zeroed_cluster(struct inode *dir, sector_t blknr, int nr_used,
 			      struct buffer_head **bhs, int nr_bhs)
 {
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 61338413d9f3e..003d311ae5523 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -343,6 +343,8 @@ extern int fat_alloc_new_dir(struct inode *dir, struct timespec64 *ts);
 extern int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
 			   struct fat_slot_info *sinfo);
 extern int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo);
+int fat_remove_entries_if_uptodate(struct inode *dir,
+				   struct fat_slot_info *sinfo);
 
 /* fat/fatent.c */
 struct fat_entry {
diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index d46d1a3851f25..6c833ed1affb3 100644
--- a/fs/fat/namei_msdos.c
+++ b/fs/fat/namei_msdos.c
@@ -441,7 +441,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 	struct fat_slot_info old_sinfo, sinfo;
 	struct timespec64 ts;
 	loff_t new_i_pos;
-	int err, old_attrs, is_dir, update_dotdot, corrupt = 0;
+	int err, old_attrs, is_dir, update_dotdot, rollback_err = 0;
 
 	old_sinfo.bh = sinfo.bh = dotdot_bh = NULL;
 	old_inode = d_inode(old_dentry);
@@ -533,7 +533,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 		if (IS_DIRSYNC(new_dir)) {
 			err = sync_dirty_buffer(dotdot_bh);
 			if (err)
-				goto error_dotdot;
+				goto error_no_rollback;
 		}
 		drop_nlink(old_dir);
 		if (!new_inode)
@@ -543,7 +543,7 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 	err = fat_remove_entries(old_dir, &old_sinfo);	/* and releases bh */
 	old_sinfo.bh = NULL;
 	if (err)
-		goto error_dotdot;
+		goto error_no_rollback;
 	inode_inc_iversion(old_dir);
 	fat_truncate_time(old_dir, &ts, FAT_UPDATE_CMTIME);
 	mark_inode_dirty(old_dir);
@@ -561,37 +561,24 @@ static int do_msdos_rename(struct inode *old_dir, unsigned char *old_name,
 	brelse(old_sinfo.bh);
 	return err;
 
-error_dotdot:
-	/* data cluster is shared, serious corruption */
-	corrupt = 1;
-
-	if (update_dotdot) {
-		fat_set_start(dotdot_de, MSDOS_I(old_dir)->i_logstart);
-		mmb_mark_buffer_dirty(dotdot_bh,
-				      &MSDOS_I(old_inode)->i_metadata_bhs);
-		corrupt |= sync_dirty_buffer(dotdot_bh);
-	}
+error_no_rollback:
+	/* Do not continue an on-disk rollback that cannot be completed. */
+	rollback_err = err;
 error_inode:
 	fat_detach(old_inode);
 	fat_attach(old_inode, old_sinfo.i_pos);
 	MSDOS_I(old_inode)->i_attrs = old_attrs;
 	if (new_inode) {
 		fat_attach(new_inode, new_i_pos);
-		if (corrupt) {
+		if (!rollback_err) {
 			mark_inode_dirty(new_inode);
-			corrupt |= sync_inode_metadata(new_inode, 1);
+			rollback_err = sync_inode_metadata(new_inode, 1);
 		}
-	} else {
-		/*
-		 * If new entry was not sharing the data cluster, it
-		 * shouldn't be serious corruption.
-		 */
-		int err2 = fat_remove_entries(new_dir, &sinfo);
-		if (corrupt)
-			corrupt |= err2;
-		sinfo.bh = NULL;
+	} else if (!rollback_err) {
+		/* Do not retry a write error reported for this buffer. */
+		rollback_err = fat_remove_entries_if_uptodate(new_dir, &sinfo);
 	}
-	if (corrupt < 0) {
+	if (rollback_err < 0) {
 		fat_fs_error(new_dir->i_sb,
 			     "%s: Filesystem corrupted (i_pos %lld)",
 			     __func__, sinfo.i_pos);
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index da3e89c0b16ac..898f98e4459d3 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -938,7 +938,7 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 	struct fat_slot_info old_sinfo, sinfo;
 	struct timespec64 ts;
 	loff_t new_i_pos;
-	int err, is_dir, corrupt = 0;
+	int err, is_dir, rollback_err = 0;
 	struct super_block *sb = old_dir->i_sb;
 
 	old_sinfo.bh = sinfo.bh = dotdot_bh = NULL;
@@ -984,7 +984,7 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 		err = vfat_update_dotdot_de(new_dir, old_inode, dotdot_bh,
 					    dotdot_de);
 		if (err)
-			goto error_dotdot;
+			goto error_no_rollback;
 		drop_nlink(old_dir);
 		if (!new_inode)
  			inc_nlink(new_dir);
@@ -993,7 +993,7 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 	err = fat_remove_entries(old_dir, &old_sinfo);	/* and releases bh */
 	old_sinfo.bh = NULL;
 	if (err)
-		goto error_dotdot;
+		goto error_no_rollback;
 	vfat_update_dir_metadata(old_dir, &ts);
 
 	if (new_inode) {
@@ -1009,34 +1009,23 @@ static int vfat_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 	return err;
 
-error_dotdot:
-	/* data cluster is shared, serious corruption */
-	corrupt = 1;
-
-	if (dotdot_de) {
-		corrupt |= vfat_update_dotdot_de(old_dir, old_inode, dotdot_bh,
-						 dotdot_de);
-	}
+error_no_rollback:
+	/* Do not continue an on-disk rollback that cannot be completed. */
+	rollback_err = err;
 error_inode:
 	fat_detach(old_inode);
 	fat_attach(old_inode, old_sinfo.i_pos);
 	if (new_inode) {
 		fat_attach(new_inode, new_i_pos);
-		if (corrupt) {
+		if (!rollback_err) {
 			mark_inode_dirty(new_inode);
-			corrupt |= sync_inode_metadata(new_inode, 1);
+			rollback_err = sync_inode_metadata(new_inode, 1);
 		}
-	} else {
-		/*
-		 * If new entry was not sharing the data cluster, it
-		 * shouldn't be serious corruption.
-		 */
-		int err2 = fat_remove_entries(new_dir, &sinfo);
-		if (corrupt)
-			corrupt |= err2;
-		sinfo.bh = NULL;
+	} else if (!rollback_err) {
+		/* Do not retry a write error reported for this buffer. */
+		rollback_err = fat_remove_entries_if_uptodate(new_dir, &sinfo);
 	}
-	if (corrupt < 0) {
+	if (rollback_err < 0) {
 		fat_fs_error(new_dir->i_sb,
 			     "%s: Filesystem corrupted (i_pos %lld)",
 			     __func__, new_i_pos);
@@ -1067,7 +1056,7 @@ static int vfat_rename_exchange(struct inode *old_dir, struct dentry *old_dentry
 	struct inode *old_inode, *new_inode;
 	struct timespec64 ts = current_time(old_dir);
 	loff_t old_i_pos, new_i_pos;
-	int err, corrupt = 0;
+	int err, rollback_err = 0;
 	struct super_block *sb = old_dir->i_sb;
 
 	old_inode = d_inode(old_dentry);
@@ -1106,13 +1095,13 @@ static int vfat_rename_exchange(struct inode *old_dir, struct dentry *old_dentry
 		err = vfat_update_dotdot_de(new_dir, old_inode, old_dotdot_bh,
 					    old_dotdot_de);
 		if (err)
-			goto error_old_dotdot;
+			goto error_no_rollback;
 	}
 	if (new_dotdot_de) {
 		err = vfat_update_dotdot_de(old_dir, new_inode, new_dotdot_bh,
 					    new_dotdot_de);
 		if (err)
-			goto error_new_dotdot;
+			goto error_no_rollback;
 	}
 
 	/* if cross directory and only one is a directory, adjust nlink */
@@ -1135,24 +1124,17 @@ static int vfat_rename_exchange(struct inode *old_dir, struct dentry *old_dentry
 
 	return err;
 
-error_new_dotdot:
-	if (new_dotdot_de) {
-		corrupt |= vfat_update_dotdot_de(new_dir, new_inode,
-						 new_dotdot_bh, new_dotdot_de);
-	}
-
-error_old_dotdot:
-	if (old_dotdot_de) {
-		corrupt |= vfat_update_dotdot_de(old_dir, old_inode,
-						 old_dotdot_bh, old_dotdot_de);
-	}
-
+error_no_rollback:
+	/* Do not start a partial on-disk rollback after a failed ".." write. */
+	rollback_err = err;
 error_exchange:
 	vfat_exchange_ipos(old_inode, new_inode, new_i_pos, old_i_pos);
-	corrupt |= vfat_sync_ipos(new_dir, new_inode);
-	corrupt |= vfat_sync_ipos(old_dir, old_inode);
+	if (!rollback_err) {
+		rollback_err |= vfat_sync_ipos(new_dir, new_inode);
+		rollback_err |= vfat_sync_ipos(old_dir, old_inode);
+	}
 
-	if (corrupt < 0) {
+	if (rollback_err < 0) {
 		fat_fs_error(new_dir->i_sb,
 			     "%s: Filesystem corrupted (i_pos %lld, %lld)",
 			     __func__, old_i_pos, new_i_pos);
-- 
2.53.0