From: Krystian Kaniewski <krystianmkaniewski@gmail.com>
To: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
syzbot+b0aebd03565f5774f7f8@syzkaller.appspotmail.com
Subject: [PATCH v4] fat: make rename recovery safe after metadata write errors
Date: Mon, 19 Oct 2026 16:51:45 +0200 [thread overview]
Message-ID: <20261019145146.539233-1-krystianmkaniewski@gmail.com> (raw)
In-Reply-To: <878q3uhv4h.fsf@mail.parknet.co.jp>
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
next prev parent reply other threads:[~2026-10-19 14:51 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-14 16:38 [syzbot] [exfat?] WARNING in vfat_update_dotdot_de syzbot
2026-09-29 9:00 ` [PATCH] fat: validate dotdot buffers in VFAT and MSDOS rename and rollback Krystian Kaniewski
2026-09-30 8:02 ` OGAWA Hirofumi
2026-09-30 12:17 ` Krystian Kaniewski
2026-10-02 13:43 ` [PATCH v2] fat: skip rename rollback on buffers with failed I/O Krystian Kaniewski
2026-10-11 18:36 ` OGAWA Hirofumi
2026-10-14 8:21 ` Krystian Kaniewski
2026-10-14 8:23 ` [PATCH v3] fat: make rename recovery safe after metadata write errors Krystian Kaniewski
2026-10-15 18:54 ` OGAWA Hirofumi
2026-10-16 13:30 ` Krystian Kaniewski
2026-10-16 13:50 ` OGAWA Hirofumi
2026-10-16 14:01 ` Krystian Kaniewski
2026-10-16 15:23 ` OGAWA Hirofumi
2026-10-19 8:21 ` Krystian Kaniewski
2026-10-19 9:55 ` OGAWA Hirofumi
2026-10-19 14:51 ` Krystian Kaniewski [this message]
2026-10-19 16:15 ` [PATCH v4] " OGAWA Hirofumi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261019145146.539233-1-krystianmkaniewski@gmail.com \
--to=krystianmkaniewski@gmail.com \
--cc=hirofumi@mail.parknet.co.jp \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=syzbot+b0aebd03565f5774f7f8@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®