[PATCH] ksmbd: release ksmbd_inode ref via ksmbd_inode_put on lookup paths

[PATCH] ksmbd: release ksmbd_inode ref via ksmbd_inode_put on lookup paths

[Aleksandr Golovnya]

ksmbd_query_inode_status() and ksmbd_lookup_fd_inode() both take a
reference on a ksmbd_inode via __ksmbd_inode_lookup() (which performs
atomic_inc_not_zero()) and later release it using a bare
atomic_dec(&ci->m_count).  Unlike ksmbd_inode_put(), a bare
atomic_dec() does not check whether the reference count has reached
zero, so if the caller happens to drop the last reference, the
ksmbd_inode is leaked: it stays in the global inode hash table with
m_count == 0, future __ksmbd_inode_lookup() calls reject it via
atomic_inc_not_zero(), and ksmbd_inode_free() is never invoked.

The race is:

    T1: __ksmbd_inode_lookup()    -> atomic_inc_not_zero(): m_count = 2
    T2: ksmbd_inode_put()         -> atomic_dec_and_test():  m_count = 1
                                                            (not freed)
    T1: atomic_dec(&ci->m_count)                          ->  m_count = 0
        return                                            (LEAK)

In ksmbd_lookup_fd_inode() the matched-fp path (which now also uses
ksmbd_inode_put()) cannot currently reach m_count == 0 because the
matched ksmbd_file holds its own reference on ci, but converting it to
the proper API keeps the three call sites consistent and avoids
future regressions if the locking changes.

Because ksmbd_inode_put() may free the ksmbd_inode if this drops the
last reference, the call must happen after up_read(&ci->m_lock) on the
two affected paths in ksmbd_lookup_fd_inode().  On the no-match path
this is a pure reordering; on the matched path ksmbd_fp_get() is
moved above the unlock so that the returned ksmbd_file is pinned
before the inode reference is released.

Signed-off-by: Aleksandr Golovnya <cofedish@gmail.com>
---
 fs/smb/server/vfs_cache.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 5a232d9..4d2d33d 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -217,7 +217,7 @@ int ksmbd_query_inode_status(struct dentry *dentry)
 		ret = KSMBD_INODE_STATUS_OK;
 	up_read(&ci->m_lock);
 
-	atomic_dec(&ci->m_count);
+	ksmbd_inode_put(ci);
 	return ret;
 }
 
@@ -719,14 +719,14 @@ struct ksmbd_file *ksmbd_lookup_fd_inode(struct dentry *dentry)
 	down_read(&ci->m_lock);
 	list_for_each_entry(lfp, &ci->m_fp_list, node) {
 		if (inode == file_inode(lfp->filp)) {
-			atomic_dec(&ci->m_count);
 			lfp = ksmbd_fp_get(lfp);
 			up_read(&ci->m_lock);
+			ksmbd_inode_put(ci);
 			return lfp;
 		}
 	}
-	atomic_dec(&ci->m_count);
 	up_read(&ci->m_lock);
+	ksmbd_inode_put(ci);
 	return NULL;
 }
 
-- 
2.54.0

Re: [PATCH] ksmbd: release ksmbd_inode ref via ksmbd_inode_put on lookup paths

[Namjae Jeon]

On Tue, May 26, 2026 at 3:50 AM Aleksandr Golovnya <cofedish@gmail.com> wrote:
>
> ksmbd_query_inode_status() and ksmbd_lookup_fd_inode() both take a
> reference on a ksmbd_inode via __ksmbd_inode_lookup() (which performs
> atomic_inc_not_zero()) and later release it using a bare
> atomic_dec(&ci->m_count).  Unlike ksmbd_inode_put(), a bare
> atomic_dec() does not check whether the reference count has reached
> zero, so if the caller happens to drop the last reference, the
> ksmbd_inode is leaked: it stays in the global inode hash table with
> m_count == 0, future __ksmbd_inode_lookup() calls reject it via
> atomic_inc_not_zero(), and ksmbd_inode_free() is never invoked.
>
> The race is:
>
>     T1: __ksmbd_inode_lookup()    -> atomic_inc_not_zero(): m_count = 2
>     T2: ksmbd_inode_put()         -> atomic_dec_and_test():  m_count = 1
>                                                             (not freed)
>     T1: atomic_dec(&ci->m_count)                          ->  m_count = 0
>         return                                            (LEAK)
>
> In ksmbd_lookup_fd_inode() the matched-fp path (which now also uses
> ksmbd_inode_put()) cannot currently reach m_count == 0 because the
> matched ksmbd_file holds its own reference on ci, but converting it to
> the proper API keeps the three call sites consistent and avoids
> future regressions if the locking changes.
>
> Because ksmbd_inode_put() may free the ksmbd_inode if this drops the
> last reference, the call must happen after up_read(&ci->m_lock) on the
> two affected paths in ksmbd_lookup_fd_inode().  On the no-match path
> this is a pure reordering; on the matched path ksmbd_fp_get() is
> moved above the unlock so that the returned ksmbd_file is pinned
> before the inode reference is released.
>
> Signed-off-by: Aleksandr Golovnya <cofedish@gmail.com>
Applied it to #ksmbd-for-next-next.
Thanks!