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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] DMA use-after-free/unmap during link teardown due to ignored v2 TX pending state.
- [High] TOCTOU data race condition in V2 slot management causes data corruption of outgoing LLC messages due to lockless memset.
--

commit 4dd2538a0566b239179ada14640f3f763de057d5
Author: D. Wythe <alibuda@linux.alibaba.com>

net/smc: clear the correct v2 slot and buffer in smc_wr_tx_put_slot()
    
This commit fixes a bug where pointers were being zeroed instead of the
structures they point to when clearing the v2 pending slot and buffer. By
passing the pointers directly and using sizeof(*pointer), it ensures the
intended structures are actually zeroed.

> diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
> index 97ba46893b17f..def2ab84b0c71 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));
> +		memset(link->lgr->wr_tx_buf_v2, 0,
> +		       sizeof(*link->lgr->wr_tx_buf_v2));

[Severity: High]
This isn't a bug introduced by this patch, but since the memset is now
correctly clearing the struct, could this create a data race with
smc_wr_tx_get_v2_slot()?

The first memset clears the entire smc_wr_tx_pend structure, which includes
the idx field. Once idx becomes 0, a concurrent thread calling
smc_wr_tx_get_v2_slot() might locklessly see that the slot is available
since it checks:

    if (link->wr_tx_v2_pend->idx == link->wr_tx_cnt)
        return -EBUSY;
        
If the concurrent thread grabs the slot and starts writing data into the
payload buffer, the second memset here will then overwrite that newly claimed
buffer with zeroes. Could this cause the hardware to transmit a corrupted
or empty message?

[Severity: High]
This is also a pre-existing issue, but while reviewing the V2 TX pending 
state logic, could there be a DMA use-after-free during link teardown?

During teardown, smc_wr_free_link() waits for pending sends to finish by
calling smc_wr_tx_wait_no_pending_sends(), which relies on:

    static inline bool smc_wr_is_tx_pend(struct smc_link *link)
    {
        ...
        return !bitmap_empty(link->wr_tx_mask, link->wr_tx_cnt);
    }

This only checks the wr_tx_mask bitmap. However, the V2 TX slot does not use
the mask; its pending state is tracked by link->wr_tx_v2_pend->idx. 

If only a V2 TX request is in flight, would smc_wr_is_tx_pend() return false
and allow smc_wr_tx_wait_no_pending_sends() to return early? This could allow
subsequent DMA unmap operations to free memory while the hardware is still
transmitting the V2 message.

>  		return 1;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910104446.41423-1-alibuda@linux.alibaba.com?part=1