Re: [PATCH net v2 2/2] net/smc: do not dereference an unset send buffer on the SMC-D teardown path

On 08/08/26 12:51 pm, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <[email protected]>
> 
> smc_close_stream_wait() calls smc_tx_prepared_sends() from inside its
> sk_wait_event() condition, and sk_wait_event() evaluates that condition
> once with the socket lock released. smcd_buf_detach() clears
> conn->sndbuf_desc from smc_conn_kill() under lock_sock(), so a link group
> terminating while a socket waits there leaves the helper dereferencing
> NULL, faulting out of close(). SIOCOUTQ reads the field by hand, and
> smc_close_cancel_work() drops the lock across two cancel_*_sync() calls.
> 
> Sample the pointer once in the helper, report nothing prepared while it is
> unset, and bound the ioctl the same way. The receive tasklet dereferences
> the field directly in smc_cdc_msg_recv_action(), not through this helper;
> 1/2 is what keeps it from running that late.
> 
> Fixes: ae2be35cbed2 ("net/smc: {at|de}tach sndbuf to peer DMB if supported")
> Cc: [email protected]
> Signed-off-by: Bryam Vargas <[email protected]>
> ---
>  net/smc/af_smc.c | 3 ++-
>  net/smc/smc_tx.h | 6 +++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 00403175b740..cff910cedbfc 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -3233,7 +3233,8 @@ int smc_ioctl(struct socket *sock, unsigned int cmd,
>  			return -EINVAL;
>  		}
>  		if (smc->sk.sk_state == SMC_INIT ||
> -		    smc->sk.sk_state == SMC_CLOSED)
> +		    smc->sk.sk_state == SMC_CLOSED ||
> +		    !READ_ONCE(smc->conn.sndbuf_desc))
>  			answ = 0;
>  		else
>  			answ = smc->conn.sndbuf_desc->len -
> diff --git a/net/smc/smc_tx.h b/net/smc/smc_tx.h
> index a59f370b8b43..610a945aefd6 100644
> --- a/net/smc/smc_tx.h
> +++ b/net/smc/smc_tx.h
> @@ -20,11 +20,15 @@
>  
>  static inline int smc_tx_prepared_sends(struct smc_connection *conn)
>  {
> +	struct smc_buf_desc *sndbuf_desc = READ_ONCE(conn->sndbuf_desc);
>  	union smc_host_cursor sent, prep;
>  
> +	if (!sndbuf_desc)
> +		return 0;
> +
>  	smc_curs_copy(&sent, &conn->tx_curs_sent, conn);
>  	smc_curs_copy(&prep, &conn->tx_curs_prep, conn);
> -	return smc_curs_diff(conn->sndbuf_desc->len, &sent, &prep);
> +	return smc_curs_diff(sndbuf_desc->len, &sent, &prep);
>  }
>  
>  void smc_tx_pending(struct smc_connection *conn);
> 

Reviewed-by: Sidraya Jayagond <[email protected]>