NACK: [SRU][J][PATCH 1/1] ksmbd: fix use-after-free in __smb2_lease_break_noti()

[Andrei Gherzan]

On 26/09/10 12:35AM, Cengiz Can via kernel-team wrote:
> From: Namjae Jeon <linkinjeon at kernel.org>
> 
> Move tcp_transport free to ksmbd_conn_free. If ksmbd connection is
> referenced when ksmbd server thread terminates, It will not be freed,
> but conn->tcp_transport is freed. __smb2_lease_break_noti can be performed
> asynchronously when the connection is disconnected. __smb2_lease_break_noti
> calls ksmbd_conn_write, which can cause use-after-free
> when conn->ksmbd_transport is already freed.
> 
> Cc: stable at vger.kernel.org
> Reported-by: Norbert Szetei <norbert at doyensec.com>
> Tested-by: Norbert Szetei <norbert at doyensec.com>
> Signed-off-by: Namjae Jeon <linkinjeon at kernel.org>
> Signed-off-by: Steve French <stfrench at microsoft.com>
> (backported from commit 21a4e47578d44c6b37c4fc4aba8ed7cc8dbb13de)
> [bot_kybele: Tree already carries a refactor (ee426bfb/b1f1e806-style) where the
>  refcnt-zero final conn release lives in ksmbd_conn_put(), not in
>  ksmbd_conn_free(); so ksmbd_conn_free() keeps calling ksmbd_conn_put() (plus
>  this tree's extra kfree(conn->mechToken)) and the upstream
>  ksmbd_free_transport(conn->transport) was placed in ksmbd_conn_put()'s
>  atomic_dec_and_test block (before kfree(conn)), the true analogue of upstream's
>  ksmbd_conn_free() hunk. transport_tcp.h lacks...]
> CVE-2025-37777
> Assisted-by: kybele:claude-opus-4.8
> Signed-off-by: Cengiz Can <cengiz.can at canonical.com>
> ---
>  fs/ksmbd/connection.c    | 11 ++++++-----
>  fs/ksmbd/transport_tcp.c | 14 +++++++++-----
>  fs/ksmbd/transport_tcp.h |  1 +
>  3 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
> index 76afa733351a..215248528ee0 100644
> --- a/fs/ksmbd/connection.c
> +++ b/fs/ksmbd/connection.c
> @@ -42,11 +42,11 @@ struct ksmbd_conn *ksmbd_conn_get(struct ksmbd_conn *conn)
>   * ksmbd_conn_put() - drop a reference and, if it was the last, perform the
>   * final, once-per-struct release of a ksmbd_conn.
>   *
> - * The transport is not released here: free_transport() releases it after
> - * calling ksmbd_conn_free().  The final release therefore only pairs
> - * ida_destroy() with kfree(), reaches no sleeping teardown and is safe
> - * from any context, including the RCU callback that frees an opinfo
> - * holding a connection reference.
> + * The transport is released here rather than in free_transport() so that it
> + * lives as long as the connection it belongs to.  __smb2_lease_break_noti()
> + * can run asynchronously after the connection is disconnected and dereference
> + * conn->transport via ksmbd_conn_write(); freeing the transport only on the
> + * final connection put avoids that use-after-free.
>   */
>  void ksmbd_conn_put(struct ksmbd_conn *conn)
>  {
> @@ -54,6 +54,7 @@ void ksmbd_conn_put(struct ksmbd_conn *conn)
>  		return;
>  
>  	if (atomic_dec_and_test(&conn->refcnt)) {
> +		ksmbd_free_transport(conn->transport);

This introduces a double free bug. Upstream has a known bug fixed in commit
a89f5fae998b, which carries Fixes: 21a4e47578d4. It calls out the cause:

  free_transport function for tcp connection can be called from smbdirect. It
  will cause kernel oops.

This is particularly important for Jammy as CONFIG_SMB_SERVER_SMBDIRECT is
enabled for all supported arches.

Another issue is that this is now reachable from an RCU softirq callback, where
it may not sleep.

oplock.c:202   call_rcu(&opinfo->rcu_head, opinfo_free_rcu);                                             
oplock.c:130   opinfo_free_rcu()  →                                                                      
oplock.c:122   free_opinfo()      →                                                                      
oplock.c:126   ksmbd_conn_put(opinfo->conn);

>  		ida_destroy(&conn->async_ida);
>  		kfree(conn);
>  	}
> diff --git a/fs/ksmbd/transport_tcp.c b/fs/ksmbd/transport_tcp.c
> index 28dc86dfb030..5ab7fb43cac5 100644
> --- a/fs/ksmbd/transport_tcp.c
> +++ b/fs/ksmbd/transport_tcp.c
> @@ -94,17 +94,21 @@ static struct tcp_transport *alloc_transport(struct socket *client_sk)
>  	return t;
>  }
>  
> -static void free_transport(struct tcp_transport *t)
> +void ksmbd_free_transport(struct ksmbd_transport *kt)
>  {
> -	kernel_sock_shutdown(t->sock, SHUT_RDWR);
> -	sock_release(t->sock);
> -	t->sock = NULL;
> +	struct tcp_transport *t = TCP_TRANS(kt);
>  
> -	ksmbd_conn_free(KSMBD_TRANS(t)->conn);
> +	sock_release(t->sock);
>  	kfree(t->iov);
>  	kfree(t);
>  }
>  
> +static void free_transport(struct tcp_transport *t)
> +{
> +	kernel_sock_shutdown(t->sock, SHUT_RDWR);
> +	ksmbd_conn_free(KSMBD_TRANS(t)->conn);
> +}
> +
>  /**
>   * kvec_array_init() - initialize a IO vector segment
>   * @new:	IO vector to be initialized
> diff --git a/fs/ksmbd/transport_tcp.h b/fs/ksmbd/transport_tcp.h
> index e338bebe322f..5925ec5df475 100644
> --- a/fs/ksmbd/transport_tcp.h
> +++ b/fs/ksmbd/transport_tcp.h
> @@ -7,6 +7,7 @@
>  #define __KSMBD_TRANSPORT_TCP_H__
>  
>  int ksmbd_tcp_set_interfaces(char *ifc_list, int ifc_list_sz);
> +void ksmbd_free_transport(struct ksmbd_transport *kt);
>  int ksmbd_tcp_init(void);
>  void ksmbd_tcp_destroy(void);

-- 
Andrei Gherzan
gpg: rsa4096/D4D94F67AD0E9640
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20260915/0964d2ae/attachment.sig>