Re: [PATCH] ksmbd: only rebind the reopened file's own oplock on durable reconnect

Reviewed-by: ChenXiaoSong <[email protected]>

By the way, your email signature is "Your Name". Did you forget to set 
your name in .gitconfig?

On 7/24/26 07:00, Your Name wrote:
> 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.
> 
> Fixes: f363a0fb134a ("ksmbd: fix app-instance durable supersede session UAF")
> Cc: [email protected]
> Signed-off-by: Aldo Ariel Panzardo <[email protected]>
> ---
>   fs/smb/server/vfs_cache.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
> index d95c405eab11..1e9d13344825 100644
> --- a/fs/smb/server/vfs_cache.c
> +++ b/fs/smb/server/vfs_cache.c
> @@ -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;

-- 
ChenXiaoSong <[email protected]>
Chinese Homepage: https://chenxiaosong.com
English Homepage: https://chenxiaosong.com/en