Re: [PATCH net-next v2 1/2] net/smc: clear the correct v2 slot and buffer in smc_wr_tx_put_slot()

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] smc_wr_tx_wait_no_pending_sends() fails to wait for pending v2 WQEs, leading to premature link teardown and UAF in the CQE handler.
- [High] Blind zeroing of V2 pending slot in CQE error path causes corruption of normal TX slot 0.
--

--- Patch [1]: [PATCH net-next v2 1/2] net/smc: clear the correct v2 slot and buffer in smc_wr_tx_put_slot() ---
commit c70c362b54a94b8914b4d46aec29581eca109cd3
Author: D. Wythe <[email protected]>

net/smc: clear the correct v2 slot and buffer in smc_wr_tx_put_slot()

This patch fixes an issue where smc_wr_tx_put_slot() incorrectly zeroes
the pointer variable itself rather than the intended v2 pending slot and
buffer structures. It achieves this by passing the pointers directly and
using sizeof(*pointer).

> diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
> index 59c92b46945c5..6b5add9229937 100644
> --- a/net/smc/smc_wr.c
> +++ b/net/smc/smc_wr.c
> @@ -288,10 +288,10 @@ int smc_wr_tx_put_slot(struct smc_link *link,
>  	} else if (link->lgr->smc_version == SMC_V2 &&
>  		   pend->idx == link->wr_tx_cnt) {
>  		/* Large v2 buffer */
> -		memset(&link->wr_tx_v2_pend, 0,
> -		       sizeof(link->wr_tx_v2_pend));
> -		memset(&link->lgr->wr_tx_buf_v2, 0,
> -		       sizeof(link->lgr->wr_tx_buf_v2));
> +		memset(link->wr_tx_v2_pend, 0,
> +		       sizeof(*link->wr_tx_v2_pend));

[Severity: High]
This is a pre-existing issue, but does changing this memset from zeroing
the pointer to zeroing the structure transform a previous NULL-pointer
dereference into a silent data corruption regression?

In smc_wr_tx_process_cqe(), if any CQE completes with an error, it
indiscriminately zeroes the link->wr_tx_v2_pend structure.

If a V2 slot operation is concurrently in progress, zeroing its structure
changes its internal idx from link->wr_tx_cnt to 0.

If the concurrent V2 operation then fails to post and calls
smc_wr_tx_put_slot(), pend->idx < link->wr_tx_cnt evaluates to true.

This targets normal slot 0, improperly clearing bit 0 in link->wr_tx_mask
and freeing a slot that might be actively used by another thread.

[Severity: High]
This isn't a bug introduced by this patch, but does the link teardown
process fail to wait for these V2 slots?

smc_wr_tx_wait_no_pending_sends() waits by calling smc_wr_is_tx_pend(),
which only checks if link->wr_tx_mask is empty:

    return !bitmap_empty(link->wr_tx_mask, link->wr_tx_cnt);

However, the V2 TX slot is tracked separately and does not set a bit
in wr_tx_mask. If a V2 WQE is in flight during link teardown, the wait
condition will instantly return and the teardown process frees
link->wr_tx_v2_pend.

When the completion queue event (CQE) for the V2 WQE eventually fires,
smc_wr_tx_process_cqe() dereferences the freed link->wr_tx_v2_pend to
check the wr_id, resulting in a Use-After-Free regression.

> +		memset(link->lgr->wr_tx_buf_v2, 0,
> +		       sizeof(*link->lgr->wr_tx_buf_v2));
>  		return 1;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1