[syzbot] [nilfs?] kernel BUG in nilfs_set_link

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 9d89551994a4 Linux 6.13-rc6
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1510b418580000
kernel config: https://syzkaller.appspot.com/x/.config?x=4ef22c4fce5135b4
dashboard link: https://syzkaller.appspot.com/bug?extid=1097e95f134f37d9395c
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

Unfortunately, I don't have any reproducer for this issue yet.

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7feb34a89c2a/non_bootable_disk-9d895519.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/bcfa24563a7a/vmlinux-9d895519.xz
kernel image: https://storage.googleapis.com/syzbot-assets/5bc14c94d0b7/bzImage-9d895519.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+1097e9...@syzkaller.appspotmail.com

loop0: detected capacity change from 0 to 2048
loop0: detected capacity change from 2048 to 0
syz.0.0: attempt to access beyond end of device
loop0: rw=0, sector=84, nr_sectors = 2 limit=0
NILFS (loop0): I/O error reading meta-data file (ino=3, block-offset=0)
NILFS (loop0): error -5 truncating bmap (ino=16)
syz.0.0: attempt to access beyond end of device
loop0: rw=0, sector=100, nr_sectors = 2 limit=0
NILFS (loop0): I/O error reading meta-data file (ino=3, block-offset=226)
syz.0.0: attempt to access beyond end of device
loop0: rw=0, sector=100, nr_sectors = 2 limit=0
NILFS (loop0): I/O error reading meta-data file (ino=3, block-offset=226)
syz.0.0: attempt to access beyond end of device
loop0: rw=0, sector=100, nr_sectors = 2 limit=0
NILFS (loop0): I/O error reading meta-data file (ino=3, block-offset=226)
syz.0.0: attempt to access beyond end of device
loop0: rw=0, sector=100, nr_sectors = 2 limit=0
NILFS (loop0): I/O error reading meta-data file (ino=3, block-offset=226)
syz.0.0: attempt to access beyond end of device
loop0: rw=0, sector=100, nr_sectors = 2 limit=0
NILFS (loop0): I/O error reading meta-data file (ino=3, block-offset=226)
syz.0.0: attempt to access beyond end of device
loop0: rw=0, sector=70, nr_sectors = 2 limit=0
NILFS (loop0): I/O error reading meta-data file (ino=6, block-offset=0)
syz.0.0: attempt to access beyond end of device
loop0: rw=0, sector=70, nr_sectors = 2 limit=0
NILFS (loop0): I/O error reading meta-data file (ino=6, block-offset=0)
NILFS (loop0): mounting fs with errors
Buffer I/O error on dev loop0, logical block 1, lost sync page write
NILFS (loop0): unable to write superblock: err=-5
NILFS (loop0): I/O error reading meta-data file (ino=3, block-offset=2)
------------[ cut here ]------------
kernel BUG at fs/nilfs2/dir.c:413!
Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN NOPTI
CPU: 0 UID: 0 PID: 5317 Comm: syz.0.0 Not tainted 6.13.0-rc6-syzkaller #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:nilfs_set_link+0x3ed/0x3f0 fs/nilfs2/dir.c:413
Code: c6 60 af 46 8c e8 03 b8 6d fe 90 0f 0b e8 eb f7 23 fe 4c 89 ef 48 c7 c6 60 af 46 8c e8 ec b7 6d fe 90 0f 0b e8 d4 f7 23 fe 90 <0f> 0b 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57
RSP: 0018:ffffc9000d1ff8c0 EFLAGS: 00010283
RAX: ffffffff837b8c4c RBX: 00000000fffffffb RCX: 0000000000100000
RDX: ffffc9000e672000 RSI: 0000000000000edb RDI: 0000000000000edc
RBP: ffff88804cb93050 R08: ffffffff837b8a9b R09: 1ffff11008674b79
R10: dffffc0000000000 R11: ffffed1008674b7a R12: dffffc0000000000
R13: ffffea000132e4c0 R14: 0000000000000050 R15: 0000000000000018
FS: 00007f595bb756c0(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ff7ccc87580 CR3: 00000000339c0000 CR4: 0000000000352ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
nilfs_rename+0x6a3/0xb60 fs/nilfs2/namei.c:409
vfs_rename+0xbdb/0xf00 fs/namei.c:5067
do_renameat2+0xd94/0x13f0 fs/namei.c:5224
__do_sys_rename fs/namei.c:5271 [inline]
__se_sys_rename fs/namei.c:5269 [inline]
__x64_sys_rename+0x82/0x90 fs/namei.c:5269
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f595ad85d29
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f595bb75038 EFLAGS: 00000246 ORIG_RAX: 0000000000000052
RAX: ffffffffffffffda RBX: 00007f595af75fa0 RCX: 00007f595ad85d29
RDX: 0000000000000000 RSI: 0000000020000040 RDI: 0000000020000000
RBP: 00007f595ae01b08 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000000 R14: 00007f595af75fa0 R15: 00007ffd8f311a58
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:nilfs_set_link+0x3ed/0x3f0 fs/nilfs2/dir.c:413
Code: c6 60 af 46 8c e8 03 b8 6d fe 90 0f 0b e8 eb f7 23 fe 4c 89 ef 48 c7 c6 60 af 46 8c e8 ec b7 6d fe 90 0f 0b e8 d4 f7 23 fe 90 <0f> 0b 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 55 41 57
RSP: 0018:ffffc9000d1ff8c0 EFLAGS: 00010283
RAX: ffffffff837b8c4c RBX: 00000000fffffffb RCX: 0000000000100000
RDX: ffffc9000e672000 RSI: 0000000000000edb RDI: 0000000000000edc
RBP: ffff88804cb93050 R08: ffffffff837b8a9b R09: 1ffff11008674b79
R10: dffffc0000000000 R11: ffffed1008674b7a R12: dffffc0000000000
R13: ffffea000132e4c0 R14: 0000000000000050 R15: 0000000000000018
FS: 00007f595bb756c0(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f595bb53fe0 CR3: 00000000339c0000 CR4: 0000000000352ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

[Ryusuke Konishi]

Hi Andrew,

please add this series to the queue for the next merge window.

This series fixes BUG_ON check failures reported by syzbot around
rename operations, and a minor behavioral issue where the mtime of a
child directory changes when it is renamed instead of moved.

Thanks,
Ryusuke Konishi

Ryusuke Konishi (2):
nilfs2: handle errors that nilfs_prepare_chunk() may return
nilfs2: do not update mtime of renamed directory that is not moved

fs/nilfs2/dir.c | 13 ++++++++++---
fs/nilfs2/namei.c | 39 +++++++++++++++++++++------------------
fs/nilfs2/nilfs.h | 4 ++--
3 files changed, 33 insertions(+), 23 deletions(-)

--
2.43.0

[Ryusuke Konishi]

The directory manipulation routines nilfs_set_link() and
nilfs_delete_entry() rewrite the directory entry in the folio/page
previously read by nilfs_find_entry(), so error handling is omitted on
the assumption that nilfs_prepare_chunk(), which prepares the buffer
for rewriting, will always succeed for these. And if an error is
returned, it triggers the legacy BUG_ON() checks in each routine.

This assumption is wrong, as proven by syzbot: the buffer layer called
by nilfs_prepare_chunk() may call nilfs_get_block() if necessary,
which may fail due to metadata corruption or other reasons. This has
been there all along, but improved sanity checks and error handling
may have made it more reproducible in fuzzing tests.

Fix this issue by adding missing error paths in nilfs_set_link(),
nilfs_delete_entry(), and their caller nilfs_rename().

Signed-off-by: Ryusuke Konishi <konishi...@gmail.com>
Reported-by: syzbot+32c370...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=32c3706ebf5d95046ea1
Reported-by: syzbot+1097e9...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1097e95f134f37d9395c
Fixes: 2ba466d74ed7 ("nilfs2: directory entry operations")
---
fs/nilfs2/dir.c | 13 ++++++++++---
fs/nilfs2/namei.c | 29 +++++++++++++++--------------
fs/nilfs2/nilfs.h | 4 ++--
3 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 0a3aea6c416b..9b7f8e9655a2 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -400,7 +400,7 @@ int nilfs_inode_by_name(struct inode *dir, const struct qstr *qstr, ino_t *ino)
return 0;
}

-void nilfs_set_link(struct inode *dir, struct nilfs_dir_entry *de,
+int nilfs_set_link(struct inode *dir, struct nilfs_dir_entry *de,
struct folio *folio, struct inode *inode)
{
size_t from = offset_in_folio(folio, de);
@@ -410,11 +410,15 @@ void nilfs_set_link(struct inode *dir, struct nilfs_dir_entry *de,

folio_lock(folio);
err = nilfs_prepare_chunk(folio, from, to);
- BUG_ON(err);
+ if (unlikely(err)) {
+ folio_unlock(folio);
+ return err;
+ }
de->inode = cpu_to_le64(inode->i_ino);
de->file_type = fs_umode_to_ftype(inode->i_mode);
nilfs_commit_chunk(folio, mapping, from, to);
inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
+ return 0;
}

/*
@@ -543,7 +547,10 @@ int nilfs_delete_entry(struct nilfs_dir_entry *dir, struct folio *folio)
from = (char *)pde - kaddr;
folio_lock(folio);
err = nilfs_prepare_chunk(folio, from, to);
- BUG_ON(err);
+ if (unlikely(err)) {
+ folio_unlock(folio);
+ goto out;
+ }
if (pde)
pde->rec_len = nilfs_rec_len_to_disk(to - from);
dir->inode = 0;
diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index 1d836a5540f3..e02fae6757f1 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -406,8 +406,10 @@ static int nilfs_rename(struct mnt_idmap *idmap,
err = PTR_ERR(new_de);
goto out_dir;
}
- nilfs_set_link(new_dir, new_de, new_folio, old_inode);
+ err = nilfs_set_link(new_dir, new_de, new_folio, old_inode);
folio_release_kmap(new_folio, new_de);
+ if (unlikely(err))
+ goto out_dir;
nilfs_mark_inode_dirty(new_dir);
inode_set_ctime_current(new_inode);
if (dir_de)
@@ -430,28 +432,27 @@ static int nilfs_rename(struct mnt_idmap *idmap,
*/
inode_set_ctime_current(old_inode);

- nilfs_delete_entry(old_de, old_folio);
-
- if (dir_de) {
- nilfs_set_link(old_inode, dir_de, dir_folio, new_dir);
- folio_release_kmap(dir_folio, dir_de);
- drop_nlink(old_dir);
+ err = nilfs_delete_entry(old_de, old_folio);
+ if (likely(!err)) {
+ if (dir_de) {
+ err = nilfs_set_link(old_inode, dir_de, dir_folio,
+ new_dir);
+ drop_nlink(old_dir);
+ }
+ nilfs_mark_inode_dirty(old_dir);
}
- folio_release_kmap(old_folio, old_de);
-
- nilfs_mark_inode_dirty(old_dir);
nilfs_mark_inode_dirty(old_inode);

- err = nilfs_transaction_commit(old_dir->i_sb);
- return err;
-
out_dir:
if (dir_de)
folio_release_kmap(dir_folio, dir_de);
out_old:
folio_release_kmap(old_folio, old_de);
out:
- nilfs_transaction_abort(old_dir->i_sb);
+ if (likely(!err))
+ err = nilfs_transaction_commit(old_dir->i_sb);
+ else
+ nilfs_transaction_abort(old_dir->i_sb);
return err;
}

diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index dff241c53fc5..cb6ed54accd7 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -261,8 +261,8 @@ struct nilfs_dir_entry *nilfs_find_entry(struct inode *, const struct qstr *,
int nilfs_delete_entry(struct nilfs_dir_entry *, struct folio *);
int nilfs_empty_dir(struct inode *);
struct nilfs_dir_entry *nilfs_dotdot(struct inode *, struct folio **);
-void nilfs_set_link(struct inode *, struct nilfs_dir_entry *,
- struct folio *, struct inode *);
+int nilfs_set_link(struct inode *dir, struct nilfs_dir_entry *de,
+ struct folio *folio, struct inode *inode);

/* file.c */
extern int nilfs_sync_file(struct file *, loff_t, loff_t, int);
--
2.43.0

[Ryusuke Konishi]

A minor issue with nilfs_rename, originating from an old ext2
implementation, is that the mtime is updated even if the rename target
is a directory and it is renamed within the same directory, rather
than moved to a different directory.

In this case, the child directory being renamed does not change in any
way, so changing its mtime is unnecessary according to the
specification, and can unnecessarily confuse backup tools.

In ext2, this issue was fixed by commit 39fe7557b4d6 ("ext2: Do not
update mtime of a moved directory") and a few subsequent fixes, but it
remained in nilfs2.

Fix this issue by not calling nilfs_set_link(), which rewrites the
inode number of the directory entry that refers to the parent
directory, when the move target is a directory and the source and
destination are the same directory.

Here, the directory to be moved only needs to be read if the inode
number of the parent directory is rewritten with nilfs_set_link, so
also adjust the execution conditions of the preparation work to avoid
unnecessary directory reads.

Signed-off-by: Ryusuke Konishi <konishi...@gmail.com>
---
fs/nilfs2/namei.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index e02fae6757f1..953fbd5f0851 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -370,6 +370,7 @@ static int nilfs_rename(struct mnt_idmap *idmap,
struct folio *old_folio;
struct nilfs_dir_entry *old_de;
struct nilfs_transaction_info ti;
+ bool old_is_dir = S_ISDIR(old_inode->i_mode);
int err;

if (flags & ~RENAME_NOREPLACE)
@@ -385,7 +386,7 @@ static int nilfs_rename(struct mnt_idmap *idmap,
goto out;
}

- if (S_ISDIR(old_inode->i_mode)) {
+ if (old_is_dir && old_dir != new_dir) {
err = -EIO;
dir_de = nilfs_dotdot(old_inode, &dir_folio);
if (!dir_de)
@@ -397,7 +398,7 @@ static int nilfs_rename(struct mnt_idmap *idmap,
struct nilfs_dir_entry *new_de;

err = -ENOTEMPTY;
- if (dir_de && !nilfs_empty_dir(new_inode))
+ if (old_is_dir && !nilfs_empty_dir(new_inode))
goto out_dir;

new_de = nilfs_find_entry(new_dir, &new_dentry->d_name,
@@ -412,7 +413,7 @@ static int nilfs_rename(struct mnt_idmap *idmap,
goto out_dir;
nilfs_mark_inode_dirty(new_dir);
inode_set_ctime_current(new_inode);
- if (dir_de)
+ if (old_is_dir)
drop_nlink(new_inode);
drop_nlink(new_inode);
nilfs_mark_inode_dirty(new_inode);
@@ -420,7 +421,7 @@ static int nilfs_rename(struct mnt_idmap *idmap,
err = nilfs_add_link(new_dentry, old_inode);
if (err)
goto out_dir;
- if (dir_de) {
+ if (old_is_dir) {
inc_nlink(new_dir);
nilfs_mark_inode_dirty(new_dir);
}
@@ -434,9 +435,10 @@ static int nilfs_rename(struct mnt_idmap *idmap,

err = nilfs_delete_entry(old_de, old_folio);
if (likely(!err)) {
- if (dir_de) {
- err = nilfs_set_link(old_inode, dir_de, dir_folio,
- new_dir);
+ if (old_is_dir) {
+ if (old_dir != new_dir)

+ err = nilfs_set_link(old_inode, dir_de,

+ dir_folio, new_dir);
drop_nlink(old_dir);
}
nilfs_mark_inode_dirty(old_dir);
--
2.43.0