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 v3] fat: make rename recovery safe after metadata write errors
Date: Wed, 14 Oct 2026 10:23:12 +0200 [thread overview]
Message-ID: <20261014082315.190262-1-krystianmkaniewski@gmail.com> (raw)
In-Reply-To: <87bj90m6dq.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. At an
inode-position failure no dotdot update or source-entry removal has
started, so rereading the affected entry block and restoring the changed
entry slots can still complete the rollback.
Read a retained new-entry buffer again before removing the entry.
Replacement and exchange recovery use inode writeback, which reacquires
the directory-entry buffer. When the second exchange position write
fails, the first has already reached disk, so attempt both restorations
even if retrying the failed block also fails.
Report recovery errors returned by these steps 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 v3:
- Give up the on-disk rollback entirely after a failed dotdot update or
old-entry removal, as suggested by OGAWA Hirofumi, instead of tracking
buffer aliases and reverting what is still possible.
- After a failed inode-position write, read the retained new-entry buffer
again with bh_read() before removing the entry, and write a replaced
target entry back through inode writeback.
- New subject, since rollback is no longer skipped per buffer.
v2: https://lore.kernel.org/all/1f18819d20c92960b8b2d6ccc8bc4506d346150e.1790779353.git.krystianmkaniewski@gmail.com/
fs/fat/namei_msdos.c | 39 ++++++++++----------------
fs/fat/namei_vfat.c | 66 +++++++++++++++++---------------------------
2 files changed, 39 insertions(+), 66 deletions(-)
diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index d46d1a3..0aab2b1 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,26 @@ out:
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) {
+ /* Read the entry block again if a failed write invalidated it. */
+ rollback_err = bh_read(sinfo.bh, 0);
+ if (rollback_err >= 0)
+ rollback_err = fat_remove_entries(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 da3e89c..f10828c 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,25 @@ out:
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) {
+ /* Read the entry block again if a failed write invalidated it. */
+ rollback_err = bh_read(sinfo.bh, 0);
+ if (rollback_err >= 0)
+ rollback_err = fat_remove_entries(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 +1058,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 +1097,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 +1126,17 @@ out:
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
prev parent reply other threads:[~2026-10-14 8:23 UTC|newest]
Thread overview: 8+ 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 ` Krystian Kaniewski [this message]
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=20261014082315.190262-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®