[PATCH 7.3.y 4/4] ksmbd: fix app-instance non-durable supersede session UAF

[Sasha Levin <sashal@kernel.org>]

From: Ian Bridges <icb@fastmail.org>

[ Upstream commit f728cfee6929057087c303d07d7a6a7c61196b48 ]

ksmbd_close_fd_app_instance_id() supersedes a prior open that has the
same AppInstanceId. It reaches that open's session through opinfo->sess.
This pointer is not refcounted.

Session teardown detaches durable handles before it frees the session.
It does not detach a non-durable open. That open still points at the
session through opinfo->sess after the session is freed. Another session
can supersede the open at the same time. The supersede then takes a
write lock through the stale pointer, in the freed session's file table.

  BUG: KASAN: slab-use-after-free in _raw_write_lock+0x71/0xd0
  Call Trace:
   _raw_write_lock+0x71/0xd0
   ksmbd_close_fd_app_instance_id+0xd9/0x670 [ksmbd]
   smb2_open+0x3e10/0xd910 [ksmbd]
  Freed by task 212:
   ksmbd_session_destroy+0x38d/0x480 [ksmbd]

Fix this by looking the session up by opinfo->session_id. The lookup
returns a reference, as smb2_break_collect_channels() does. Recheck that
opinfo->sess still points at it before using it.

Do the lookup without m_lock held. destroy_previous_session() takes
m_lock under the session table lock, and taking them in the other order
here would deadlock.

Fixes: 16c30649709d ("ksmbd: handle durable v2 app instance id")
Cc: stable@vger.kernel.org # e0386cd6498b: ksmbd: validate AppInstanceId takeover target
Signed-off-by: Ian Bridges <icb@fastmail.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/smb/server/vfs_cache.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index a2051dfb47d91..5f1442c103d81 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -1054,7 +1054,9 @@ int ksmbd_close_fd_app_instance_id(struct ksmbd_work *work,
 				 struct ksmbd_file *fp, bool *closed)
 {
 	struct ksmbd_file_table *ft;
+	struct ksmbd_session *sess;
 	struct oplock_info *opinfo;
+	u64 session_id;
 	int n_to_drop = 0;
 
 	*closed = false;
@@ -1067,10 +1069,35 @@ int ksmbd_close_fd_app_instance_id(struct ksmbd_work *work,
 		up_read(&fp->f_ci->m_lock);
 		goto out;
 	}
+	/*
+	 * opinfo->sess is not refcounted, and m_lock does not keep it alive.
+	 * The owner can be torn down while we hold only a reference to fp.
+	 * Look the owner up by id to take a real reference instead.
+	 */
+	session_id = opinfo->session_id;
+	up_read(&fp->f_ci->m_lock);
 
-	ft = &opinfo->sess->file_table;
+	/* destroy_previous_session() takes m_lock under sessions_table_lock. */
+	sess = ksmbd_session_lookup_slowpath(session_id);
+	if (!sess)
+		goto out;
+
+	down_read(&fp->f_ci->m_lock);
+	if (opinfo->sess != sess) {
+		up_read(&fp->f_ci->m_lock);
+		ksmbd_user_session_put(sess);
+		goto out;
+	}
+	ft = &sess->file_table;
 	write_lock(&ft->lock);
-	if (fp->f_state == FP_INITED && has_file_id(fp->volatile_id) &&
+	/*
+	 * The table may already be detached by destroy_previous_session().
+	 * A new session could also reuse the old id and address and pass the
+	 * check above. Only remove fp if this table still publishes it.
+	 */
+	if (ft->idr && fp->f_state == FP_INITED &&
+	    has_file_id(fp->volatile_id) &&
+	    idr_find(ft->idr, fp->volatile_id) == fp &&
 	    ksmbd_app_instance_id_matches(work, fp)) {
 		idr_remove(ft->idr, fp->volatile_id);
 		fp->volatile_id = KSMBD_NO_FID;
@@ -1078,6 +1105,7 @@ int ksmbd_close_fd_app_instance_id(struct ksmbd_work *work,
 	}
 	write_unlock(&ft->lock);
 	up_read(&fp->f_ci->m_lock);
+	ksmbd_user_session_put(sess);
 	opinfo_put(opinfo);
 	opinfo = NULL;
 
-- 
2.53.0