Skip to content

Commit

Permalink
cachefiles: fix dentry leak in cachefiles_open_file()
Browse files Browse the repository at this point in the history
A dentry leak may be caused when a lookup cookie and a cull are concurrent:

            P1             |             P2
-----------------------------------------------------------
cachefiles_lookup_cookie
  cachefiles_look_up_object
    lookup_one_positive_unlocked
     // get dentry
                            cachefiles_cull
                              inode->i_flags |= S_KERNEL_FILE;
    cachefiles_open_file
      cachefiles_mark_inode_in_use
        __cachefiles_mark_inode_in_use
          can_use = false
          if (!(inode->i_flags & S_KERNEL_FILE))
            can_use = true
	  return false
        return false
        // Returns an error but doesn't put dentry

After that the following WARNING will be triggered when the backend folder
is umounted:

==================================================================
BUG: Dentry 000000008ad87947{i=7a,n=Dx_1_1.img}  still in use (1) [unmount of ext4 sda]
WARNING: CPU: 4 PID: 359261 at fs/dcache.c:1767 umount_check+0x5d/0x70
CPU: 4 PID: 359261 Comm: umount Not tainted 6.6.0-dirty #25
RIP: 0010:umount_check+0x5d/0x70
Call Trace:
 <TASK>
 d_walk+0xda/0x2b0
 do_one_tree+0x20/0x40
 shrink_dcache_for_umount+0x2c/0x90
 generic_shutdown_super+0x20/0x160
 kill_block_super+0x1a/0x40
 ext4_kill_sb+0x22/0x40
 deactivate_locked_super+0x35/0x80
 cleanup_mnt+0x104/0x160
==================================================================

Whether cachefiles_open_file() returns true or false, the reference count
obtained by lookup_positive_unlocked() in cachefiles_look_up_object()
should be released.

Therefore release that reference count in cachefiles_look_up_object() to
fix the above issue and simplify the code.

Fixes: 1f08c92 ("cachefiles: Implement backing file wrangling")
Cc: [email protected]
Signed-off-by: Baokun Li <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Acked-by: David Howells <[email protected]>
Signed-off-by: Christian Brauner <[email protected]>
  • Loading branch information
LiBaokun96 authored and brauner committed Sep 27, 2024
1 parent 075dbe9 commit da6ef2d
Showing 1 changed file with 3 additions and 4 deletions.
7 changes: 3 additions & 4 deletions fs/cachefiles/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -595,14 +595,12 @@ static bool cachefiles_open_file(struct cachefiles_object *object,
* write and readdir but not lookup or open).
*/
touch_atime(&file->f_path);
dput(dentry);
return true;

check_failed:
fscache_cookie_lookup_negative(object->cookie);
cachefiles_unmark_inode_in_use(object, file);
fput(file);
dput(dentry);
if (ret == -ESTALE)
return cachefiles_create_file(object);
return false;
Expand All @@ -611,7 +609,6 @@ static bool cachefiles_open_file(struct cachefiles_object *object,
fput(file);
error:
cachefiles_do_unmark_inode_in_use(object, d_inode(dentry));
dput(dentry);
return false;
}

Expand Down Expand Up @@ -654,7 +651,9 @@ bool cachefiles_look_up_object(struct cachefiles_object *object)
goto new_file;
}

if (!cachefiles_open_file(object, dentry))
ret = cachefiles_open_file(object, dentry);
dput(dentry);
if (!ret)
return false;

_leave(" = t [%lu]", file_inode(object->file)->i_ino);
Expand Down

0 comments on commit da6ef2d

Please sign in to comment.