Re: [PATCH 07/26] cachefiles: Fix excess dput() after end_removing()

David Howells <[email protected]> writes:

> When cachefiles_cull() calls cachefiles_bury_object(), the latter eats the
> former's ref on the victim dentry that it obtained from
> cachefiles_lookup_for_cull().  However, commit 7bb1eb45e43c left the dput
> of the victim in place, resulting in occasional:
>
>   WARNING: fs/dcache.c:829 at dput.part.0+0xf5/0x110, CPU#7: cachefilesd/11831
>   cachefiles_cull+0x8c/0xe0 [cachefiles]
>   cachefiles_daemon_cull+0xcd/0x120 [cachefiles]
>   cachefiles_daemon_write+0x14e/0x1d0 [cachefiles]
>   vfs_write+0xc3/0x480
>   ...
>
> reports.
>
> Actually, it's worse than that: cachefiles_bury_object() eats the ref it was
> given - and then may continue to the now-unref'd dentry it if it turns out to
> be a directory.  So simply removing the aberrant dput() is not sufficient.
>
> Fix this by making cachefiles_bury_object() retain the ref itself around
> end_removing() if it needs to keep it and then drop the ref before returning.
>
> Fixes: bd6ede8a06e8 ("VFS/nfsd/cachefiles/ovl: introduce start_removing() and end_removing()")
> Reported-by: Marc Dionne <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: NeilBrown <[email protected]>
> cc: Paulo Alcantara <[email protected]>
> cc: [email protected]
> cc: [email protected]
> cc: [email protected]
> ---
>  fs/cachefiles/namei.c | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)

Acked-by: Paulo Alcantara (Red Hat) <[email protected]>