Linux s390 Architecture development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "D. Wythe" <alibuda@linux.alibaba.com>
Cc: "Christian Borntraeger" <borntraeger@linux.ibm.com>,
	linux-s390@vger.kernel.org, "Vasily Gorbik" <gor@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	"Alexander Gordeev" <agordeev@linux.ibm.com>
Subject: Re: [PATCH net-next v2 1/2] net/smc: clear the correct v2 slot and buffer in smc_wr_tx_put_slot()
Date: Sat, 22 Aug 2026 09:17:27 +0000	[thread overview]
Message-ID: <20260822091728.26A961F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260821091702.21458-2-alibuda@linux.alibaba.com>

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 <alibuda@linux.alibaba.com>

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/20260821091702.21458-1-alibuda@linux.alibaba.com?part=1

  reply	other threads:[~2026-08-22  9:17 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21  9:17 [PATCH net-next v2 0/2] net/smc: fix v2 slot clearing and reduce TX slot contention D. Wythe
2026-08-21  9:17 ` [PATCH net-next v2 1/2] net/smc: clear the correct v2 slot and buffer in smc_wr_tx_put_slot() D. Wythe
2026-08-22  9:17   ` sashiko-bot [this message]
2026-08-21  9:17 ` [PATCH net-next v2 2/2] net/smc: reduce TX slot contention with exclusive wait D. Wythe
2026-08-22  9:17   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260822091728.26A961F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=alibuda@linux.alibaba.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox