ksmbd: fix app-instance durable supersede session UAF

ksmbd_close_fd_app_instance_id() looks up a prior durable handle by
AppInstanceId and closes it through opinfo->sess->file_table. This is
unsafe after the original session has been torn down. session_fd_check()
preserves reconnectable durable handles in the global table and clears
opinfo->conn/fp->conn, but opinfo->sess can still point to the freed
ksmbd_session.

Use opinfo->conn as the orphan sentinel, but make the check reliable by
serializing it with session_fd_check(). That path clears opinfo->conn
under fp->f_ci->m_lock, so hold the same lock while testing opinfo->conn
and while dereferencing opinfo->sess->file_table. Also avoid closing
through the session file table if the volatile id has already been
unpublished by session teardown.

Durable reconnect must keep the two fields consistent. Rebinding only
opinfo->conn leaves opinfo->sess pointing at the old freed session, so
a later app-instance supersede can pass the conn check and write-lock the
freed session's file table. Clear opinfo->sess when preserving a durable
handle during session teardown, and set it to the reconnecting session
when opinfo->conn is rebound in ksmbd_reopen_durable_fd().

Fixes: 16c30649709d ("ksmbd: handle durable v2 app instance id")
Reported-by: Gil Portnoy <dddhkts1@gmail.com>
Co-developed-by: Gil Portnoy <dddhkts1@gmail.com>
Signed-off-by: Gil Portnoy <dddhkts1@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 73d28942dc0ade6f3613859a0736547edf0be3ee..d95c405eab115f0d12ae9800552b1a19228edd5c 100644 (file)
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -853,19 +853,24 @@ int ksmbd_close_fd_app_instance_id(char *app_instance_id)
                return 0;
 
        opinfo = opinfo_get(fp);
-       if (!opinfo || !opinfo->sess)
+       if (!opinfo)
+               goto out;
+
+       down_read(&fp->f_ci->m_lock);
+       if (!opinfo->conn) {
+               up_read(&fp->f_ci->m_lock);
                goto out;
+       }
 
        ft = &opinfo->sess->file_table;
        write_lock(&ft->lock);
-       if (fp->f_state == FP_INITED) {
-               if (has_file_id(fp->volatile_id)) {
-                       idr_remove(ft->idr, fp->volatile_id);
-                       fp->volatile_id = KSMBD_NO_FID;
-               }
+       if (fp->f_state == FP_INITED && has_file_id(fp->volatile_id)) {
+               idr_remove(ft->idr, fp->volatile_id);
+               fp->volatile_id = KSMBD_NO_FID;
                n_to_drop = ksmbd_mark_fp_closed(fp);
        }
        write_unlock(&ft->lock);
+       up_read(&fp->f_ci->m_lock);
        opinfo_put(opinfo);
        opinfo = NULL;
 
@@ -1562,6 +1567,7 @@ static bool session_fd_check(struct ksmbd_tree_connect *tcon,
                        continue;
                ksmbd_conn_put(op->conn);
                op->conn = NULL;
+               op->sess = NULL;
        }
        up_write(&ci->m_lock);
 
@@ -1717,6 +1723,7 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)
                if (op->conn)
                        continue;
                op->conn = ksmbd_conn_get(fp->conn);
+               op->sess = work->sess;
        }
        up_write(&ci->m_lock);