[syzbot] [bfs?] WARNING in bfs_rename

[syzbot] [bfs?] WARNING in bfs_rename

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    8155b4ef3466 Add linux-next specific files for 20241220
git tree:       linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=17d8a818580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=9c90bb7161a56c88
dashboard link: https://syzkaller.appspot.com/bug?extid=80e60df48923e1b7691d
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=167acac4580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1489b0b0580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/98a974fc662d/disk-8155b4ef.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/2dea9b72f624/vmlinux-8155b4ef.xz
kernel image: https://storage.googleapis.com/syzbot-assets/593a42b9eb34/bzImage-8155b4ef.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/8542e0b88fbf/mount_0.gz

Bisection is inconclusive: the issue happens on the oldest tested release.

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=160f5818580000
final oops:     https://syzkaller.appspot.com/x/report.txt?x=150f5818580000
console output: https://syzkaller.appspot.com/x/log.txt?x=110f5818580000

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

------------[ cut here ]------------
WARNING: CPU: 0 PID: 5833 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
Modules linked in:
CPU: 0 UID: 0 PID: 5833 Comm: syz-executor420 Not tainted 6.13.0-rc3-next-20241220-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
Code: bb 70 07 00 00 be 08 00 00 00 e8 87 15 e7 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 4d 97 80 ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
RSP: 0018:ffffc90003e7f950 EFLAGS: 00010293
RAX: ffffffff823e8cd3 RBX: 1ffff1100e6a81f8 RCX: ffff8880347b3c00
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff823e8c53 R09: 1ffffffff203563e
R10: dffffc0000000000 R11: fffffbfff203563f R12: ffff888073540fc0
R13: dffffc0000000000 R14: ffff888073540f78 R15: dffffc0000000000
FS:  00007f7f45f236c0(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005574822450d8 CR3: 0000000029caa000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 inode_dec_link_count include/linux/fs.h:2567 [inline]
 bfs_rename+0x44e/0x530 fs/bfs/dir.c:247
 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:0x7f7f45f6b569
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 17 00 00 90 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 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f7f45f23218 EFLAGS: 00000246 ORIG_RAX: 0000000000000052
RAX: ffffffffffffffda RBX: 00007f7f45ff26a8 RCX: 00007f7f45f6b569
RDX: ffffffffffffffb0 RSI: 0000000020000240 RDI: 00000000200001c0
RBP: 00007f7f45ff26a0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000073 R11: 0000000000000246 R12: 00007f7f45fbf080
R13: 0030656c69662f2e R14: 00007f7f45fbf047 R15: c21418431439518c
 </TASK>


---
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 syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection

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

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

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

[PATCH] bfs: put a inode if link count is 0

[Lizhi Xu]

syzbot reported a warning in drop_nlink. [1]

The reproducer performs the rename operation on the file twice in succession
and changes the file to the same file name.  After the first rename operation,
the number of links in the inode is set to 0. In the second execution, the
same inode is used, resulting in a 0 value warning for i_nlink. 

To avoid this issue, put the target inode before exiting the bfs_rename.

[1]
WARNING: CPU: 0 PID: 5819 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
Modules linked in:
CPU: 0 UID: 0 PID: 5819 Comm: syz-executor232 Not tainted 6.13.0-rc5-syzkaller-00161-g63676eefb7a0 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
Code: bb 70 07 00 00 be 08 00 00 00 e8 37 3b e7 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 8d 5b 83 ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
RSP: 0018:ffffc90003c6f950 EFLAGS: 00010293
RAX: ffffffff821c1843 RBX: 1ffff1100e5181f8 RCX: ffff8880351d9e00
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff821c17c3 R09: 1ffffffff2030ad6
R10: dffffc0000000000 R11: fffffbfff2030ad7 R12: ffff8880728c0fc0
R13: dffffc0000000000 R14: ffff8880728c0f78 R15: dffffc0000000000
FS:  0000555562096380(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000558ecae7b0d8 CR3: 0000000012a66000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 inode_dec_link_count include/linux/fs.h:2521 [inline]
 bfs_rename+0x44e/0x530 fs/bfs/dir.c:247
 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

Reported-by: syzbot+80e60df48923e1b7691d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=80e60df48923e1b7691d
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/bfs/dir.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
index c375e22c4c0c..3d67e7c5b1fa 100644
--- a/fs/bfs/dir.c
+++ b/fs/bfs/dir.c
@@ -251,6 +251,8 @@ static int bfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 
 end_rename:
 	mutex_unlock(&info->bfs_lock);
+	if (new_inode && !new_inode->i_nlink)
+		iput(new_inode);
 	brelse(old_bh);
 	brelse(new_bh);
 	return error;
-- 
2.43.0

Re: [PATCH] bfs: put a inode if link count is 0

[Al Viro]

On Thu, Jan 09, 2025 at 11:49:46AM +0800, Lizhi Xu wrote:
> 
> The reproducer performs the rename operation on the file twice in succession
> and changes the file to the same file name.  After the first rename operation,
> the number of links in the inode is set to 0. In the second execution, the
> same inode is used, resulting in a 0 value warning for i_nlink. 
> 
> To avoid this issue, put the target inode before exiting the bfs_rename.

	This is completely insane - you get an extra drop of in-core inode
refcount, which *will* end up with dangling pointer and memory corruption.
Besides, there is a perfectly legitimate case when you open a file and
rename something on top of it.  It MUST remain open and alive until the
last in-core reference to inode goes away, which must not happen before
close().

	Frankly, if you do not understand why that is wrong, you should not
mess with anything filesystem-related until you learn the area enough.

NAK.

Re: [PATCH] bfs: put a inode if link count is 0

[Lizhi Xu]

On Thu, 9 Jan 2025 06:22:16 +0000, Al Viro wrote:
> > The reproducer performs the rename operation on the file twice in succession
> > and changes the file to the same file name.  After the first rename operation,
> > the number of links in the inode is set to 0. In the second execution, the
> > same inode is used, resulting in a 0 value warning for i_nlink.
> >
> > To avoid this issue, put the target inode before exiting the bfs_rename.
> 
>         This is completely insane - you get an extra drop of in-core inode
> refcount, which *will* end up with dangling pointer and memory corruption.
> Besides, there is a perfectly legitimate case when you open a file and
> rename something on top of it.  It MUST remain open and alive until the
> last in-core reference to inode goes away, which must not happen before
> close().
In the reproducer, changes the file to the same file, the same file name is
"file0", file0 uses mknod to create its inode and sets the i_nlink value to 1.
There is no operation to open file0 in the reproducer. Is this situation also
as you said?

Re: [PATCH] bfs: put a inode if link count is 0

[Al Viro]

On Thu, Jan 09, 2025 at 02:39:39PM +0800, Lizhi Xu wrote:
> On Thu, 9 Jan 2025 06:22:16 +0000, Al Viro wrote:
> > > The reproducer performs the rename operation on the file twice in succession
> > > and changes the file to the same file name.  After the first rename operation,
> > > the number of links in the inode is set to 0. In the second execution, the
> > > same inode is used, resulting in a 0 value warning for i_nlink.
> > >
> > > To avoid this issue, put the target inode before exiting the bfs_rename.
> > 
> >         This is completely insane - you get an extra drop of in-core inode
> > refcount, which *will* end up with dangling pointer and memory corruption.
> > Besides, there is a perfectly legitimate case when you open a file and
> > rename something on top of it.  It MUST remain open and alive until the
> > last in-core reference to inode goes away, which must not happen before
> > close().
> In the reproducer, changes the file to the same file, the same file name is
> "file0", file0 uses mknod to create its inode and sets the i_nlink value to 1.
> There is no operation to open file0 in the reproducer. Is this situation also
> as you said?

I'm not sure I understand your sentence, to be honest.

Your patch is 100% wrong - you must *not*, under any circumstances, have
->rename() drop references to in-core struct inode instances.  It's *always*
wrong; the reference to new_inode in new_dentry->d_inode remains there (as
it ought to) and its contribution to new_inode refcount remains unchanged.
It has nothing to do with ->i_nlink; you are decrementing ->i_count, which
controls the lifetime of in-core struct inode instance.  As soon as that
reaches zero, struct inode instance will be freed.  And destructor of
new_dentry *will* call iput() on its ->d_inode, so you'll end up with
attempt to decrement refcount of already freed memory object.

Again, that has nothing to do with ->i_nlink.  I'm not familiar with
bfs layout, so I can't tell what's going on with the corrupted image syzbot
are messing with just by visual examination.  Is there, by any chance,
a preexisting file0 in the root of that bfs image?  Does mknod() succeed
there?  Because if it doesn't and what you have is a buggered image with
'file0' being there having zero link count, yes, renaming over it would
trigger warnings about detected corrupted filesystem - we are trying to
remove a link (on disk) to something that claims (on disk) to have 0 links
at the time of that operation; something is clearly wrong and deserves
a warning.

Again, that iput() in there is basically introducing random memory
corruption; it might make the warning go away, but it's not a fix.

Re: [PATCH] bfs: put a inode if link count is 0

[Lizhi Xu]

On Thu, 9 Jan 2025 07:32:28 +0000, Al Viro wrote:
> > On Thu, 9 Jan 2025 06:22:16 +0000, Al Viro wrote:
> > > > The reproducer performs the rename operation on the file twice in succession
> > > > and changes the file to the same file name.  After the first rename operation,
> > > > the number of links in the inode is set to 0. In the second execution, the
> > > > same inode is used, resulting in a 0 value warning for i_nlink.
> > > >
> > > > To avoid this issue, put the target inode before exiting the bfs_rename.
> > >
> > >         This is completely insane - you get an extra drop of in-core inode
> > > refcount, which *will* end up with dangling pointer and memory corruption.
> > > Besides, there is a perfectly legitimate case when you open a file and
> > > rename something on top of it.  It MUST remain open and alive until the
> > > last in-core reference to inode goes away, which must not happen before
> > > close().
> > In the reproducer, changes the file to the same file, the same file name is
> > "file0", file0 uses mknod to create its inode and sets the i_nlink value to 1.
> > There is no operation to open file0 in the reproducer. Is this situation also
> > as you said?
> 
> I'm not sure I understand your sentence, to be honest.
> 
> Your patch is 100% wrong - you must *not*, under any circumstances, have
> ->rename() drop references to in-core struct inode instances.  It's *always*
> wrong; the reference to new_inode in new_dentry->d_inode remains there (as
> it ought to) and its contribution to new_inode refcount remains unchanged.
> It has nothing to do with ->i_nlink; you are decrementing ->i_count, which
> controls the lifetime of in-core struct inode instance.  As soon as that
> reaches zero, struct inode instance will be freed.  And destructor of
> new_dentry *will* call iput() on its ->d_inode, so you'll end up with
> attempt to decrement refcount of already freed memory object.
> 
> Again, that has nothing to do with ->i_nlink.  I'm not familiar with
> bfs layout, so I can't tell what's going on with the corrupted image syzbot
> are messing with just by visual examination.  Is there, by any chance,
> a preexisting file0 in the root of that bfs image?  Does mknod() succeed
> there?  Because if it doesn't and what you have is a buggered image with
> 'file0' being there having zero link count, yes, renaming over it would
> trigger warnings about detected corrupted filesystem - we are trying to
> remove a link (on disk) to something that claims (on disk) to have 0 links
> at the time of that operation; something is clearly wrong and deserves
> a warning.
> 
> Again, that iput() in there is basically introducing random memory
> corruption; it might make the warning go away, but it's not a fix.
I also have deep doubts about using iput in rename.
Thanks for your analysis.

Lizhi

Re: [PATCH] bfs: put a inode if link count is 0

[kernel test robot]

Hi Lizhi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brauner-vfs/vfs.all]
[also build test WARNING on linus/master v6.13-rc6 next-20250109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lizhi-Xu/bfs-put-a-inode-if-link-count-is-0/20250109-123423
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/20250109034946.1386748-1-lizhi.xu%40windriver.com
patch subject: [PATCH] bfs: put a inode if link count is 0
config: x86_64-buildonly-randconfig-005-20250110 (https://download.01.org/0day-ci/archive/20250110/202501101459.tVj2wd8p-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250110/202501101459.tVj2wd8p-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501101459.tVj2wd8p-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from fs/bfs/dir.c:12:
   In file included from include/linux/buffer_head.h:12:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/x86/include/asm/cacheflush.h:5:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> fs/bfs/dir.c:225:6: warning: variable 'new_inode' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
     225 |         if (!old_bh || (le16_to_cpu(old_de->ino) != old_inode->i_ino))
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/bfs/dir.c:254:6: note: uninitialized use occurs here
     254 |         if (new_inode && !new_inode->i_nlink)
         |             ^~~~~~~~~
   fs/bfs/dir.c:225:2: note: remove the 'if' if its condition is always false
     225 |         if (!old_bh || (le16_to_cpu(old_de->ino) != old_inode->i_ino))
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     226 |                 goto end_rename;
         |                 ~~~~~~~~~~~~~~~
>> fs/bfs/dir.c:225:6: warning: variable 'new_inode' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
     225 |         if (!old_bh || (le16_to_cpu(old_de->ino) != old_inode->i_ino))
         |             ^~~~~~~
   fs/bfs/dir.c:254:6: note: uninitialized use occurs here
     254 |         if (new_inode && !new_inode->i_nlink)
         |             ^~~~~~~~~
   fs/bfs/dir.c:225:6: note: remove the '||' if its condition is always false
     225 |         if (!old_bh || (le16_to_cpu(old_de->ino) != old_inode->i_ino))
         |             ^~~~~~~~~~
   fs/bfs/dir.c:206:37: note: initialize the variable 'new_inode' to silence this warning
     206 |         struct inode *old_inode, *new_inode;
         |                                            ^
         |                                             = NULL
   6 warnings generated.


vim +225 fs/bfs/dir.c

^1da177e4c3f41 Linus Torvalds    2005-04-16  201  
e18275ae55e07a Christian Brauner 2023-01-13  202  static int bfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
549c7297717c32 Christian Brauner 2021-01-21  203  		      struct dentry *old_dentry, struct inode *new_dir,
549c7297717c32 Christian Brauner 2021-01-21  204  		      struct dentry *new_dentry, unsigned int flags)
^1da177e4c3f41 Linus Torvalds    2005-04-16  205  {
^1da177e4c3f41 Linus Torvalds    2005-04-16  206  	struct inode *old_inode, *new_inode;
^1da177e4c3f41 Linus Torvalds    2005-04-16  207  	struct buffer_head *old_bh, *new_bh;
^1da177e4c3f41 Linus Torvalds    2005-04-16  208  	struct bfs_dirent *old_de, *new_de;
3f165e4cf2af04 Dmitri Vorobiev   2008-07-25  209  	struct bfs_sb_info *info;
^1da177e4c3f41 Linus Torvalds    2005-04-16  210  	int error = -ENOENT;
^1da177e4c3f41 Linus Torvalds    2005-04-16  211  
f03b8ad8d38634 Miklos Szeredi    2016-09-27  212  	if (flags & ~RENAME_NOREPLACE)
f03b8ad8d38634 Miklos Szeredi    2016-09-27  213  		return -EINVAL;
f03b8ad8d38634 Miklos Szeredi    2016-09-27  214  
^1da177e4c3f41 Linus Torvalds    2005-04-16  215  	old_bh = new_bh = NULL;
2b0143b5c986be David Howells     2015-03-17  216  	old_inode = d_inode(old_dentry);
^1da177e4c3f41 Linus Torvalds    2005-04-16  217  	if (S_ISDIR(old_inode->i_mode))
^1da177e4c3f41 Linus Torvalds    2005-04-16  218  		return -EINVAL;
^1da177e4c3f41 Linus Torvalds    2005-04-16  219  
3f165e4cf2af04 Dmitri Vorobiev   2008-07-25  220  	info = BFS_SB(old_inode->i_sb);
3f165e4cf2af04 Dmitri Vorobiev   2008-07-25  221  
3f165e4cf2af04 Dmitri Vorobiev   2008-07-25  222  	mutex_lock(&info->bfs_lock);
33ebdebece6d5d Al Viro           2018-04-30  223  	old_bh = bfs_find_entry(old_dir, &old_dentry->d_name, &old_de);
^1da177e4c3f41 Linus Torvalds    2005-04-16  224  
f433dc56344cb7 Dmitri Vorobiev   2007-11-14 @225  	if (!old_bh || (le16_to_cpu(old_de->ino) != old_inode->i_ino))
^1da177e4c3f41 Linus Torvalds    2005-04-16  226  		goto end_rename;
^1da177e4c3f41 Linus Torvalds    2005-04-16  227  
^1da177e4c3f41 Linus Torvalds    2005-04-16  228  	error = -EPERM;
2b0143b5c986be David Howells     2015-03-17  229  	new_inode = d_inode(new_dentry);
33ebdebece6d5d Al Viro           2018-04-30  230  	new_bh = bfs_find_entry(new_dir, &new_dentry->d_name, &new_de);
^1da177e4c3f41 Linus Torvalds    2005-04-16  231  
^1da177e4c3f41 Linus Torvalds    2005-04-16  232  	if (new_bh && !new_inode) {
^1da177e4c3f41 Linus Torvalds    2005-04-16  233  		brelse(new_bh);
^1da177e4c3f41 Linus Torvalds    2005-04-16  234  		new_bh = NULL;
^1da177e4c3f41 Linus Torvalds    2005-04-16  235  	}
^1da177e4c3f41 Linus Torvalds    2005-04-16  236  	if (!new_bh) {
b455ecd4bb4db1 Al Viro           2018-04-30  237  		error = bfs_add_entry(new_dir, &new_dentry->d_name,
f433dc56344cb7 Dmitri Vorobiev   2007-11-14  238  					old_inode->i_ino);
^1da177e4c3f41 Linus Torvalds    2005-04-16  239  		if (error)
^1da177e4c3f41 Linus Torvalds    2005-04-16  240  			goto end_rename;
^1da177e4c3f41 Linus Torvalds    2005-04-16  241  	}
^1da177e4c3f41 Linus Torvalds    2005-04-16  242  	old_de->ino = 0;
ce17a80c388cd1 Jeff Layton       2023-10-04  243  	inode_set_mtime_to_ts(old_dir, inode_set_ctime_current(old_dir));
^1da177e4c3f41 Linus Torvalds    2005-04-16  244  	mark_inode_dirty(old_dir);
^1da177e4c3f41 Linus Torvalds    2005-04-16  245  	if (new_inode) {
73d9b9d028176b Jeff Layton       2023-07-05  246  		inode_set_ctime_current(new_inode);
9a53c3a783c2fa Dave Hansen       2006-09-30  247  		inode_dec_link_count(new_inode);
^1da177e4c3f41 Linus Torvalds    2005-04-16  248  	}
4427f0c36e22e2 Al Viro           2009-06-08  249  	mark_buffer_dirty_inode(old_bh, old_dir);
^1da177e4c3f41 Linus Torvalds    2005-04-16  250  	error = 0;
^1da177e4c3f41 Linus Torvalds    2005-04-16  251  
^1da177e4c3f41 Linus Torvalds    2005-04-16  252  end_rename:
3f165e4cf2af04 Dmitri Vorobiev   2008-07-25  253  	mutex_unlock(&info->bfs_lock);
56423717549f8d Lizhi Xu          2025-01-09 @254  	if (new_inode && !new_inode->i_nlink)
56423717549f8d Lizhi Xu          2025-01-09  255  		iput(new_inode);
^1da177e4c3f41 Linus Torvalds    2005-04-16  256  	brelse(old_bh);
^1da177e4c3f41 Linus Torvalds    2005-04-16  257  	brelse(new_bh);
^1da177e4c3f41 Linus Torvalds    2005-04-16  258  	return error;
^1da177e4c3f41 Linus Torvalds    2005-04-16  259  }
^1da177e4c3f41 Linus Torvalds    2005-04-16  260  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH V2] bfs: prevent rename a target of link count is 0

[Lizhi Xu]

syzbot reported a warning in drop_nlink. [1]

The reproducer performs the rename operation on the file twice in succession
and changes the file to the same file name.  After the first rename operation,
the number of links in the inode is set to 0. In the second execution, the
same inode is used, resulting in a 0 value warning for i_nlink.

To avoid this issue, if the number of hard links to the target is 0 then
quit renaming.

[1]
WARNING: CPU: 0 PID: 5819 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
Modules linked in:
CPU: 0 UID: 0 PID: 5819 Comm: syz-executor232 Not tainted 6.13.0-rc5-syzkaller-00161-g63676eefb7a0 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
Code: bb 70 07 00 00 be 08 00 00 00 e8 37 3b e7 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 8d 5b 83 ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
RSP: 0018:ffffc90003c6f950 EFLAGS: 00010293
RAX: ffffffff821c1843 RBX: 1ffff1100e5181f8 RCX: ffff8880351d9e00
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff821c17c3 R09: 1ffffffff2030ad6
R10: dffffc0000000000 R11: fffffbfff2030ad7 R12: ffff8880728c0fc0
R13: dffffc0000000000 R14: ffff8880728c0f78 R15: dffffc0000000000
FS:  0000555562096380(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000558ecae7b0d8 CR3: 0000000012a66000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 inode_dec_link_count include/linux/fs.h:2521 [inline]
 bfs_rename+0x44e/0x530 fs/bfs/dir.c:247
 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

Reported-by: syzbot+80e60df48923e1b7691d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=80e60df48923e1b7691d
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
V1 -> V2: if the number of hard links to the target is 0 then quit renaming

 fs/bfs/dir.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
index c375e22c4c0c..c78272a5bc60 100644
--- a/fs/bfs/dir.c
+++ b/fs/bfs/dir.c
@@ -227,6 +227,12 @@ static int bfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 
 	error = -EPERM;
 	new_inode = d_inode(new_dentry);
+	if (new_inode) {
+		if (new_inode->i_nlink < 1) {
+			error = -EUCLEAN;
+			goto end_rename;
+		}
+	}
 	new_bh = bfs_find_entry(new_dir, &new_dentry->d_name, &new_de);
 
 	if (new_bh && !new_inode) {
-- 
2.43.0