[PATCH] isofs: fix inode leak caused by disconnected dentries from exportfs

[PATCH] isofs: fix inode leak caused by disconnected dentries from exportfs

[Deepanshu Kartikey]

When open_by_handle_at() is used with iso9660 filesystems, exportfs
creates disconnected dentries during file handle resolution. If the
operation fails (e.g., with -ESTALE during reconnect_path()), these
dentries remain cached with their associated inodes.

During unmount, shrink_dcache_for_umount() does not fully evict these
disconnected dentries, leaving their inodes with non-zero reference
counts. This triggers the "VFS: Busy inodes after unmount" warning
and causes inode leaks that accumulate across mount/unmount cycles.

The issue occurs because:
1. open_by_handle_at() calls exportfs_decode_fh_raw() to resolve
   file handles
2. For iso9660 with Joliet extensions, this creates disconnected
   dentries for both primary (iso9660) and secondary (Joliet) root
   inodes
3. When path reconnection fails with -ESTALE, the dentries are left
   in DCACHE_DISCONNECTED state
4. shrink_dcache_for_umount() in generic_shutdown_super() does not
   aggressively evict these disconnected dentries
5. The associated inodes (typically root inodes 1792 and 1807)
   remain with i_count=1, triggering the busy inode check

Add explicit shrink_dcache_sb() call in isofs_put_super() to ensure
all cached dentries, including disconnected ones created by exportfs
operations, are released before the superblock is destroyed.

Reported-by: syzbot+1d79ebe5383fc016cf07@syzkaller.appspotmail.com
Tested-by: syzbot+1d79ebe5383fc016cf07@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/isofs/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index 6f0e6b19383c..bee410705442 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -52,6 +52,7 @@ static int isofs_dentry_cmp_ms(const struct dentry *dentry,
 static void isofs_put_super(struct super_block *sb)
 {
 	struct isofs_sb_info *sbi = ISOFS_SB(sb);
+	shrink_dcache_sb(sb);
 
 #ifdef CONFIG_JOLIET
 	unload_nls(sbi->s_nls_iocharset);
-- 
2.43.0

Re: [PATCH] isofs: fix inode leak caused by disconnected dentries from exportfs

[Jan Kara]

On Wed 01-10-25 15:13:10, Deepanshu Kartikey wrote:
> When open_by_handle_at() is used with iso9660 filesystems, exportfs
> creates disconnected dentries during file handle resolution. If the
> operation fails (e.g., with -ESTALE during reconnect_path()), these
> dentries remain cached with their associated inodes.
> 
> During unmount, shrink_dcache_for_umount() does not fully evict these
> disconnected dentries, leaving their inodes with non-zero reference
> counts. This triggers the "VFS: Busy inodes after unmount" warning
> and causes inode leaks that accumulate across mount/unmount cycles.
> 
> The issue occurs because:
> 1. open_by_handle_at() calls exportfs_decode_fh_raw() to resolve
>    file handles
> 2. For iso9660 with Joliet extensions, this creates disconnected
>    dentries for both primary (iso9660) and secondary (Joliet) root
>    inodes
> 3. When path reconnection fails with -ESTALE, the dentries are left
>    in DCACHE_DISCONNECTED state

True, but when reconnection fails, exportfs_decode_fh_raw() calls dput() on
the created dentry and dput() immediately destroys DCACHE_DISCONNECTED
dentries. So I'm not following how these dentries could still survive until
umount(). Can you please explain?

> 4. shrink_dcache_for_umount() in generic_shutdown_super() does not
>    aggressively evict these disconnected dentries
> 5. The associated inodes (typically root inodes 1792 and 1807)
>    remain with i_count=1, triggering the busy inode check
> 
> Add explicit shrink_dcache_sb() call in isofs_put_super() to ensure
> all cached dentries, including disconnected ones created by exportfs
> operations, are released before the superblock is destroyed.

This is almost certainly a wrong way of fixing the problem. First we need
to better understand why DCACHE_DISCONNECTED aren't getting properly
evicted...

								Honza

> 
> Reported-by: syzbot+1d79ebe5383fc016cf07@syzkaller.appspotmail.com
> Tested-by: syzbot+1d79ebe5383fc016cf07@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
>  fs/isofs/inode.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> index 6f0e6b19383c..bee410705442 100644
> --- a/fs/isofs/inode.c
> +++ b/fs/isofs/inode.c
> @@ -52,6 +52,7 @@ static int isofs_dentry_cmp_ms(const struct dentry *dentry,
>  static void isofs_put_super(struct super_block *sb)
>  {
>  	struct isofs_sb_info *sbi = ISOFS_SB(sb);
> +	shrink_dcache_sb(sb);
>  
>  #ifdef CONFIG_JOLIET
>  	unload_nls(sbi->s_nls_iocharset);
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] isofs: fix inode leak caused by disconnected dentries from exportfs

[Deepanshu Kartikey]

Hi Jan,

Thank you for the review. 

You're right - I need to investigate further why these dentries survive until unmount if dput() should destroy DCACHE_DISCONNECTED dentries immediately. 
I'll add more detailed tracing to understand the actual reference counting and dentry state throughout the lifecycle. 
I may have misdiagnosed the root cause even though the fix addresses the symptom

Best Regards
Deepanshu kartikey

Re: [PATCH] isofs: fix inode leak caused by disconnected dentries from exportfs

[Deepanshu Kartikey]

Hi Jan,

Thank you for the review. You're absolutely right - my initial explanation was incorrect. I've done extensive debugging to understand the actual mechanism causing the leak.

Root Cause Analysis
===================

The leak occurs specifically with CONFIG_JOLIET=y through the following sequence:

1. Joliet Root Switching During Mount
--------------------------------------

In isofs_fill_super(), when Joliet extensions are detected:
- Primary root inode 1792 is created with i_count=1, i_nlink=3
- During Joliet switching, iput(inode) is called on inode 1792
- i_count decrements to 0, but generic_drop_inode() returns false (i_nlink=3 > 0)
- Inode 1792 remains cached at i_count=0
- New Joliet root inode 1920 is created and attached to sb->s_root

Debugging output:
  [9.653617] isofs: switching roots, about to iput ino=1792, i_count=1
  [9.653676] isofs: after iput, getting new root
  [9.653880] isofs: old inode after iput ino=1792, i_count=0, i_nlink=3
  [9.654219] isofs: got new root ino=1920, i_count=1

2. open_by_handle_at() Triggers Reconnection
---------------------------------------------

When the system call attempts to resolve a file handle:
- exportfs_decode_fh_raw() calls fh_to_dentry() which returns inode 1856
- The dentry is marked DCACHE_DISCONNECTED
- reconnect_path() is invoked to connect the path to root
- This calls reconnect_one() to walk up the directory tree

3. Reference Accumulation in reconnect_one()
---------------------------------------------

I instrumented reconnect_one() to track dentry reference counts:

  [8.010398] reconnect_one: called for inode 1856
  [8.010735] isofs: __isofs_iget got inode 1792, i_count=1
  [8.011041] After fh_to_parent: d_count=1
  [8.011319] After exportfs_get_name: d_count=2
  [8.011769] After lookup_one_unlocked: d_count=3

The parent dentry (inode 1792) accumulates 3 references:
1. Initial reference from fh_to_parent()
2. Additional reference taken by exportfs_get_name()
3. Another reference taken by lookup_one_unlocked()

Then lookup_one_unlocked() creates a dentry for inode 1807:
  [8.015179] isofs: __isofs_iget got inode 1807, i_count=1
  [8.016169] lookup returns tmp (inode 1807), d_count=1

The code enters the tmp != dentry branch and calls dput(tmp), then goes to 
out_reconnected.

4. Insufficient Cleanup
-----------------------

For inode 1807, I traced through dput():
  [10.083359] fast_dput: lockref_put_return returned 0
  [10.083699] fast_dput: RETAINING dentry for inode 1807, d_flags=0x240043

The dentry refcount goes to 0, but retain_dentry() returns true because of 
the DCACHE_REFERENCED flag (0x40 in 0x240043). The dentry is kept in cache 
with refcount 0, still holding the inode reference.

For inode 1792:
  [10.084125] fast_dput: lockref_put_return returned 2

At out_reconnected, only one dput(parent) is called, decrementing from 3 to 2. 
Two references remain leaked.

5. Unmount Failure
------------------

At unmount time:
- shrink_dcache_for_umount() doesn't evict dentries with positive refcounts (1792)
- Doesn't aggressively evict retained dentries with refcount 0 (1807)
- Both inodes appear as leaked with i_count=1

  [10.155385] LEAKED INODE: ino=1807, i_count=1, i_state=0x0, i_nlink=1
  [10.155604] LEAKED INODE: ino=1792, i_count=1, i_state=0x0, i_nlink=1

Why shrink_dcache_sb() Works
=============================

Calling shrink_dcache_sb() in isofs_put_super() forces eviction of:
- Dentries with extra references that weren't properly released
- Retained dentries sitting in cache at refcount 0

This ensures cleanup happens before the superblock is destroyed.

Open Questions
==============

1. Are exportfs_get_name() and lookup_one_unlocked() supposed to take 
   references to the parent dentry that the caller must release? Should 
   reconnect_one() be calling dput(parent) multiple times, or are these 
   functions leaking references?

2. Is adding shrink_dcache_sb() in put_super() the appropriate fix, or 
   should this be handled in the reconnection error path when 
   reconnect_one() fails?

3. Does this indicate a broader issue with how exportfs handles parent 
   dentry references during failed path reconnections that might affect 
   other filesystems?

I can investigate further into the implementation of exportfs_get_name() 
and lookup_one_unlocked() to understand where exactly the extra references 
are taken if that would be helpful.

Best regards,
Deepanshu Kartikey

Re: [PATCH] isofs: fix inode leak caused by disconnected dentries from exportfs

[Jan Kara]

Hello,

On Thu 02-10-25 01:57:13, Deepanshu Kartikey wrote:
> Thank you for the review. You're absolutely right - my initial
> explanation was incorrect. I've done extensive debugging to understand
> the actual mechanism causing the leak.

Hrm, the text below looks like it was generated by some AI agent :) More
importantly it still doesn't really explain the underlying cause of the
leak - it is fine to use AI for debugging but then one should get his own
understanding of the situation. As I was curious about the root cause, I
did the debugging myself and [1] has the proper analysis of the problem and
the fix.

								Honza

[1] https://lore.kernel.org/all/20251002155506.10755-2-jack@suse.cz/

> 
> Root Cause Analysis
> ===================
> 
> The leak occurs specifically with CONFIG_JOLIET=y through the following sequence:
> 
> 1. Joliet Root Switching During Mount
> --------------------------------------
> 
> In isofs_fill_super(), when Joliet extensions are detected:
> - Primary root inode 1792 is created with i_count=1, i_nlink=3
> - During Joliet switching, iput(inode) is called on inode 1792
> - i_count decrements to 0, but generic_drop_inode() returns false (i_nlink=3 > 0)
> - Inode 1792 remains cached at i_count=0
> - New Joliet root inode 1920 is created and attached to sb->s_root
> 
> Debugging output:
>   [9.653617] isofs: switching roots, about to iput ino=1792, i_count=1
>   [9.653676] isofs: after iput, getting new root
>   [9.653880] isofs: old inode after iput ino=1792, i_count=0, i_nlink=3
>   [9.654219] isofs: got new root ino=1920, i_count=1
> 
> 2. open_by_handle_at() Triggers Reconnection
> ---------------------------------------------
> 
> When the system call attempts to resolve a file handle:
> - exportfs_decode_fh_raw() calls fh_to_dentry() which returns inode 1856
> - The dentry is marked DCACHE_DISCONNECTED
> - reconnect_path() is invoked to connect the path to root
> - This calls reconnect_one() to walk up the directory tree
> 
> 3. Reference Accumulation in reconnect_one()
> ---------------------------------------------
> 
> I instrumented reconnect_one() to track dentry reference counts:
> 
>   [8.010398] reconnect_one: called for inode 1856
>   [8.010735] isofs: __isofs_iget got inode 1792, i_count=1
>   [8.011041] After fh_to_parent: d_count=1
>   [8.011319] After exportfs_get_name: d_count=2
>   [8.011769] After lookup_one_unlocked: d_count=3
> 
> The parent dentry (inode 1792) accumulates 3 references:
> 1. Initial reference from fh_to_parent()
> 2. Additional reference taken by exportfs_get_name()
> 3. Another reference taken by lookup_one_unlocked()
> 
> Then lookup_one_unlocked() creates a dentry for inode 1807:
>   [8.015179] isofs: __isofs_iget got inode 1807, i_count=1
>   [8.016169] lookup returns tmp (inode 1807), d_count=1
> 
> The code enters the tmp != dentry branch and calls dput(tmp), then goes to 
> out_reconnected.
> 
> 4. Insufficient Cleanup
> -----------------------
> 
> For inode 1807, I traced through dput():
>   [10.083359] fast_dput: lockref_put_return returned 0
>   [10.083699] fast_dput: RETAINING dentry for inode 1807, d_flags=0x240043
> 
> The dentry refcount goes to 0, but retain_dentry() returns true because of 
> the DCACHE_REFERENCED flag (0x40 in 0x240043). The dentry is kept in cache 
> with refcount 0, still holding the inode reference.
> 
> For inode 1792:
>   [10.084125] fast_dput: lockref_put_return returned 2
> 
> At out_reconnected, only one dput(parent) is called, decrementing from 3 to 2. 
> Two references remain leaked.
> 
> 5. Unmount Failure
> ------------------
> 
> At unmount time:
> - shrink_dcache_for_umount() doesn't evict dentries with positive refcounts (1792)
> - Doesn't aggressively evict retained dentries with refcount 0 (1807)
> - Both inodes appear as leaked with i_count=1
> 
>   [10.155385] LEAKED INODE: ino=1807, i_count=1, i_state=0x0, i_nlink=1
>   [10.155604] LEAKED INODE: ino=1792, i_count=1, i_state=0x0, i_nlink=1
> 
> Why shrink_dcache_sb() Works
> =============================
> 
> Calling shrink_dcache_sb() in isofs_put_super() forces eviction of:
> - Dentries with extra references that weren't properly released
> - Retained dentries sitting in cache at refcount 0
> 
> This ensures cleanup happens before the superblock is destroyed.
> 
> Open Questions
> ==============
> 
> 1. Are exportfs_get_name() and lookup_one_unlocked() supposed to take 
>    references to the parent dentry that the caller must release? Should 
>    reconnect_one() be calling dput(parent) multiple times, or are these 
>    functions leaking references?
> 
> 2. Is adding shrink_dcache_sb() in put_super() the appropriate fix, or 
>    should this be handled in the reconnection error path when 
>    reconnect_one() fails?
> 
> 3. Does this indicate a broader issue with how exportfs handles parent 
>    dentry references during failed path reconnections that might affect 
>    other filesystems?
> 
> I can investigate further into the implementation of exportfs_get_name() 
> and lookup_one_unlocked() to understand where exactly the extra references 
> are taken if that would be helpful.
> 
> Best regards,
> Deepanshu Kartikey
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] isofs: fix inode leak caused by disconnected dentries from exportfs

[Deepanshu Kartikey]

Thank you for debugging this properly and providing the correct analysis. I clearly missed the actual root cause. 
I'll study your patch to understand what I got wrong.

Best regards,
Deepanshu