ksmbd: only rebind the reopened file's own oplock on durable reconnect
Subsystems (3)
PATCH 1/1 ksmbd: only rebind the reopened file's own oplock on durable reconnect
From: Aldo Ariel Panzardo <[email protected]>
ksmbd_reopen_durable_fd() walks the inode's m_op_list and rebinds every detached oplock to the reconnecting session:
list_for_each_entry_rcu(op, &ci->m_op_list, op_entry,
lockdep_is_held(&ci->m_lock)) {
if (op->conn)
continue;
op->conn = ksmbd_conn_get(fp->conn);
op->sess = work->sess;
}The only key is op->conn == NULL, which every detached durable handle on that inode matches, not just the one owned by fp. When two sessions hold durable handles on the same file and both disconnect, reconnecting one of them adopts the other session's oplock: op->sess is overwritten with the reconnecting session without taking a reference on it, while op->conn pins the connection.
The sibling teardown path, session_fd_check(), keys on the identity of the connection being torn down (op->conn == conn) rather than on shared state, and so does not have this problem.
Once the adopting session is destroyed, ksmbd_session_destroy() frees it while the foreign oplock still points at it. The reader in ksmbd_close_fd_app_instance_id() validates only opinfo->conn, which is still live thanks to the reference taken above, and then dereferences the stale session:
if (!opinfo->conn) {
up_read(&fp->f_ci->m_lock);
goto out;
}
ft = &opinfo->sess->file_table;
write_lock(&ft->lock);BUG: KASAN: slab-use-after-free in _raw_write_lock+0x74/0xd0 Write of size 4 at addr ffff88810a970528 by task kworker/0:0/9 Workqueue: ksmbd-io handle_ksmbd_work Call Trace: _raw_write_lock+0x74/0xd0 ksmbd_close_fd_app_instance_id+0x183/0x410 smb2_open+0x1346/0x4430 handle_ksmbd_work+0x2bb/0x7b0
Reached from an authenticated session against a share with the default durable-handle and oplock configuration: two sessions open the same file with a durable-v2 handle and an RH lease under distinct AppInstanceIds, both log off, one reconnects with DH2C, and a later durable-v2 create carrying the other AppInstanceId walks into the freed session.
Constrain the loop to the oplock owned by the file being reopened.
--- fs/smb/server/vfs_cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/smb/server/vfs_cache.c +1 -1
@@ -1720,7 +1720,7 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)down_write (&ci->m_lock);list_for_each_entry_rcu (op, &ci->m_op_list, op_entry,lockdep_is_held (&ci->m_lock)) { -if (op->conn) +if (op->conn || op->o_fp != fp)continue ; op->conn =ksmbd_conn_get (fp->conn); op->sess = work->sess;
Signature · 1 line
2.43.0
By the way, your email signature is "Your Name". Did you forget to set your name in .gitconfig?
Quoted 75 lines · Your Name
From: Aldo Ariel Panzardo <[email protected]>
ksmbd_reopen_durable_fd()walks the inode'sm_op_listand rebinds every detached oplock to the reconnecting session:list_for_each_entry_rcu(op, &ci->m_op_list, op_entry, lockdep_is_held(&ci->m_lock)) { if (op->conn) continue; op->conn = ksmbd_conn_get(fp->conn); op->sess = work->sess; }The only key is
op->conn== NULL, which every detached durable handle on that inode matches, not just the one owned by fp. When two sessions hold durable handles on the same file and both disconnect, reconnecting one of them adopts the other session's oplock:op->sessis overwritten with the reconnecting session without taking a reference on it, whileop->connpins the connection.The sibling teardown path,
session_fd_check(), keys on the identity of the connection being torn down (op->conn== conn) rather than on shared state, and so does not have this problem.Once the adopting session is destroyed,
ksmbd_session_destroy()frees it while the foreign oplock still points at it. The reader inksmbd_close_fd_app_instance_id()validates onlyopinfo->conn, which is still live thanks to the reference taken above, and then dereferences the stale session:if (!opinfo->conn) { up_read(&fp->f_ci->m_lock); goto out; } ft = &opinfo->sess->file_table; write_lock(&ft->lock);BUG: KASAN: slab-use-after-free in _raw_write_lock+0x74/0xd0 Write of size 4 at addr ffff88810a970528 by task kworker/0:0/9 Workqueue: ksmbd-io handle_ksmbd_work Call Trace: _raw_write_lock+0x74/0xd0 ksmbd_close_fd_app_instance_id+0x183/0x410 smb2_open+0x1346/0x4430 handle_ksmbd_work+0x2bb/0x7b0Reached from an authenticated session against a share with the default durable-handle and oplock configuration: two sessions open the same file with a durable-v2 handle and an RH lease under distinct AppInstanceIds, both log off, one reconnects with DH2C, and a later durable-v2 create carrying the other AppInstanceId walks into the freed session.
Constrain the loop to the oplock owned by the file being reopened.
Fixes f363a0fb134a ("ksmbd: fix app-instance durable supersede session UAF")Signed-off-by Aldo Ariel Panzardo <[email protected]>--- fs/smb/server/vfs_cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/smb/server/vfs_cache.c +1
-1@@ -1720,7 +1720,7 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)down_write (&ci->m_lock);list_for_each_entry_rcu (op, &ci->m_op_list, op_entry,lockdep_is_held (&ci->m_lock)) { -if (op->conn) +if (op->conn || op->o_fp != fp)continue ; op->conn =ksmbd_conn_get (fp->conn); op->sess = work->sess;
Signature · 3 lines
ChenXiaoSong <[email protected]> Chinese Homepage: https://chenxiaosong.com English Homepage: https://chenxiaosong.com/en
Quoted 54 lines · Your Name
From: Aldo Ariel Panzardo <[email protected]>
ksmbd_reopen_durable_fd()walks the inode'sm_op_listand rebinds every detached oplock to the reconnecting session:list_for_each_entry_rcu(op, &ci->m_op_list, op_entry, lockdep_is_held(&ci->m_lock)) { if (op->conn) continue; op->conn = ksmbd_conn_get(fp->conn); op->sess = work->sess; }The only key is
op->conn== NULL, which every detached durable handle on that inode matches, not just the one owned by fp. When two sessions hold durable handles on the same file and both disconnect, reconnecting one of them adopts the other session's oplock:op->sessis overwritten with the reconnecting session without taking a reference on it, whileop->connpins the connection.The sibling teardown path,
session_fd_check(), keys on the identity of the connection being torn down (op->conn== conn) rather than on shared state, and so does not have this problem.Once the adopting session is destroyed,
ksmbd_session_destroy()frees it while the foreign oplock still points at it. The reader inksmbd_close_fd_app_instance_id()validates onlyopinfo->conn, which is still live thanks to the reference taken above, and then dereferences the stale session:if (!opinfo->conn) { up_read(&fp->f_ci->m_lock); goto out; } ft = &opinfo->sess->file_table; write_lock(&ft->lock);BUG: KASAN: slab-use-after-free in _raw_write_lock+0x74/0xd0 Write of size 4 at addr ffff88810a970528 by task kworker/0:0/9 Workqueue: ksmbd-io handle_ksmbd_work Call Trace: _raw_write_lock+0x74/0xd0 ksmbd_close_fd_app_instance_id+0x183/0x410 smb2_open+0x1346/0x4430 handle_ksmbd_work+0x2bb/0x7b0Reached from an authenticated session against a share with the default durable-handle and oplock configuration: two sessions open the same file with a durable-v2 handle and an RH lease under distinct AppInstanceIds, both log off, one reconnects with DH2C, and a later durable-v2 create carrying the other AppInstanceId walks into the freed session.
Quoted 5 lines
Constrain the loop to the oplock owned by the file being reopened.
Fixes f363a0fb134a ("ksmbd: fix app-instance durable supersede session UAF")Signed-off-by Aldo Ariel Panzardo <[email protected]>
Applied it to #ksmbd-for-next-next.
Thanks!