Re: [PATCH net v6 1/3] net/smc: bound the wire-controlled producer cursor to the RMB

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] smc: NULL pointer deref in smcr_cdc_msg_to_host() via token guessing race
--

commit 841c4fb2c2d0123708a65170d68d2745b02a7ce8
Author: Bryam Vargas <hexlabsecurity@proton.me>

net/smc: bound the wire-controlled producer cursor to the RMB

This commit bounds the wire-controlled producer cursor to the receive
buffer length in smcr_cdc_msg_to_host() and smcd_cdc_msg_to_host(). This
prevents a peer from advertising a producer cursor past the buffer bounds
and disclosing adjacent kernel memory during urgent data path processing.

> diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
> --- a/net/smc/smc_cdc.h
> +++ b/net/smc/smc_cdc.h
[ ... ]
> @@ -246,8 +256,13 @@ static inline void smcr_cdc_msg_to_host(struct smc_host_cdc_msg *local,
>  	local->len = peer->len;
>  	local->seqno = ntohs(peer->seqno);
>  	local->token = ntohl(peer->token);
> -	smc_cdc_cursor_to_host(&local->prod, &peer->prod, conn);
> -	smc_cdc_cursor_to_host(&local->cons, &peer->cons, conn);
> +	/* bound the wire-controlled producer cursor to our RMB (used as a raw
> +	 * index by the urgent path); leave the consumer cursor unbounded -- it
> +	 * indexes the peer's RMB and is bounded by peer_rmbe_size.
> +	 */
> +	smc_cdc_cursor_to_host(&local->prod, &peer->prod, conn,
> +			       conn->rmb_desc->len);

[Severity: High]
Can this lead to a NULL pointer dereference?

There appears to be a race window where conn->rmb_desc can be NULL when this
code is executed. In af_smc.c, smc_connect_rdma() registers the connection
via smc_conn_create() before the send buffer and RMB are created via
smc_buf_create():

af_smc.c:smc_connect_rdma() {
    ...
    reason_code = smc_conn_create(smc, ini);
    ...
    if (smc_buf_create(smc, false)) {
    ...
}

During smc_conn_create(), smc_lgr_register_conn() is called, which makes the
connection globally visible with a highly predictable token:

smc_core.c:smc_lgr_register_conn() {
    ...
    while (!conn->alert_token_local) {
        conn->alert_token_local = atomic_inc_return(&nexttoken);
        if (smc_lgr_find_conn(conn->alert_token_local, conn->lgr))
            conn->alert_token_local = 0;
    }
    smc_lgr_add_alert_token(conn);
    ...
}

If an unauthenticated remote peer on the same RoCE network guesses the token
and sends a CDC message over the active link before smc_buf_create() allocates
the rmb_desc, smc_cdc_rx_handler() will process the message and call
smcr_cdc_msg_to_host(). This would cause a kernel panic when trying to access
conn->rmb_desc->len.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-b4-disp-f7cd3d96-v6-0-fba616c0920c@proton.me?part=1