Re: [PATCH net 2/2] net/smc: do not reuse the DMB of a nocopy device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] net/smc: UAF of conn->rmb_desc when lgr is terminating in
  smc_conn_free()
--

commit 1d775feb06a42c02b472b41cc9bc4f7bca488d91
Author: Bryam Vargas <hexlabsecurity@proton.me>

net/smc: do not reuse the DMB of a nocopy device

This commit adds smcd_rmb_drop() and calls it from smc_buf_unuse() to
unregister and free the RMB DMB of a nocopy SMC-D device and clear
conn->rmb_desc instead of returning the buffer to the link group pool.

> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index a3164201c1420..88855d9f66173 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1243,6 +1243,22 @@ static void smcd_buf_detach(struct smc_connection *conn)
>  	kfree(buf_desc);
>  }
>  
> +/* With DMB nocopy the peer writes into the RMB through its own
> + * attachment, which can outlive this connection: unregister the DMB
> + * instead of pooling it, so the next connection gets a new one.
> + */
> +static void smcd_rmb_drop(struct smc_connection *conn,
> +			  struct smc_link_group *lgr)
> +{
> +	struct smc_buf_desc *rmb_desc = conn->rmb_desc;
> +
> +	down_write(&lgr->rmbs_lock);
> +	smc_lgr_buf_list_del(lgr, true, rmb_desc);
> +	up_write(&lgr->rmbs_lock);
> +	WRITE_ONCE(conn->rmb_desc, NULL);
> +	smc_buf_free(lgr, true, rmb_desc);
> +}
> +
>  static void smc_buf_unuse(struct smc_connection *conn,
>  			  struct smc_link_group *lgr)
>  {
> @@ -1266,8 +1282,13 @@ static void smc_buf_unuse(struct smc_connection *conn,
>  			smcr_buf_unuse(conn->rmb_desc, true, lgr);
>  		} else {
>  			bufsize += sizeof(struct smcd_cdc_msg);
> -			memzero_explicit(conn->rmb_desc->cpu_addr, bufsize);
> -			WRITE_ONCE(conn->rmb_desc->used, 0);
> +			if (smc_ism_support_dmb_nocopy(lgr->smcd)) {
> +				smcd_rmb_drop(conn, lgr);
> +			} else {
> +				memzero_explicit(conn->rmb_desc->cpu_addr,
> +						 bufsize);
> +				WRITE_ONCE(conn->rmb_desc->used, 0);
> +			}
>  		}
>  		SMC_STAT_RMB_SIZE(smc, is_smcd, true, false, bufsize);
>  	}

[Severity: High]
Can conn->rmb_desc still be used after free if lgr is already terminating
when smc_conn_free() runs?

smcd_rmb_drop() is only called from smc_buf_unuse(), which smc_conn_free()
skips when lgr->list is empty or the connection was already unregistered:

net/smc/smc_core.c:smc_conn_free() {
	...
	if (!smc_conn_lgr_valid(conn))
		goto lgr_put;
	...
	if (!list_empty(&lgr->list))
		smc_buf_unuse(conn, lgr); /* allow buffer reuse */
	smc_lgr_unregister_conn(conn);
	...
}

If a handshake fails after smc_clc_wait_msg() calls
smc_lgr_terminate_sched() (which unlinks lgr->list), smc_conn_abort() ->
smc_conn_free() skips smc_buf_unuse(), so conn->rmb_desc is not cleared to
NULL before __smc_lgr_free() frees the buffer.  If connect() is then
retried on the same socket and fails in smc_buf_create() after
smc_conn_create() attaches to a new link group, smc_conn_free()
dereferences and double-frees the dangling conn->rmb_desc.

Skipping smcd_rmb_drop() on those paths also leaves the ended connection's
nocopy DMB registered in lgr->rmbs until every remaining socket in the
terminating link group is closed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-b4-disp-19244662-v1-0-39b256b70ca7@proton.me?part=2