Re: [PATCH] RDMA/siw: Fix CEP reference race in siw_accept()
Shuangpeng <[email protected]>
| Newsgroups | org.kernel.vger.linux-rdma,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
Hi Bernard, Thanks for your reply. After carefully rechecking the code path, I realized that I had overlooked synchronization mechanisms: the RDMA core serializes application QP destruction with iw_cm_accept(), while peer-side teardown is serialized by siw_cep_set_inuse() and siw_cep_set_free(). Therefore, the concurrent QP-destroy path described in this follow-up patch is not reachable in the current implementation. Please disregard my follow-up patch. I apologize for the confusion. Thanks again for pointing this out. Best regards, Shuangpeng > On Aug 21, 2026, at 11:34, Bernard Metzler <[email protected]> wrote: > > On 16.08.2026 08:13, Shuangpeng Bai wrote: >> siw_accept() associates a CEP with the selected QP while holding >> qp->state_lock. If the transition to RTS fails, the error path releases >> the lock before detaching that association. >> The cleanup drops the QP association reference before clearing qp->cep. >> A concurrent QP destroy can acquire qp->state_lock in between, observe >> the stale pointer, and drop the same association reference again. This >> can free the CEP before siw_accept() releases its remaining reference, >> causing a use-after-free. > > Do we have a stack trace for that use-after-free?> >> Serialize the error-side detachment with qp->state_lock and clear qp->cep >> before dropping the association reference. This ensures that either the >> accept cleanup or QP teardown removes the association, but not both. >> This is a follow-up to commit a93949718259 ("RDMA/siw: Fix >> use-after-free in siw_accept()") and addresses a separate race in the >> same error path. >> Fixes: 6c52fdc244b5 ("rdma/siw: connection management") >> Link: https://patch.msgid.link/[email protected] >> Cc: [email protected] >> Signed-off-by: Shuangpeng Bai <[email protected]> >> --- >> drivers/infiniband/sw/siw/siw_cm.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c >> index 0245b25e7271..da1b953f3fa3 100644 >> --- a/drivers/infiniband/sw/siw/siw_cm.c >> +++ b/drivers/infiniband/sw/siw/siw_cm.c >> @@ -1751,11 +1751,13 @@ int siw_accept(struct iw_cm_id *id, struct iw_cm_conn_param *params) >> cep->state = SIW_EPSTATE_CLOSED; >> siw_free_cm_id(cep); > > What can destroy the QP at this point? > An appication QP destroy shall be serialized with this > accept call by the RDMA core. > > A concurrent connection drop by peer side would affect > QP state, but is serialized at the cep using > siw_cep_set_inuse()/_free() I don't see other cases > which are not serialized...? >> + down_write(&qp->state_lock); > >> if (qp->cep == cep) { >> - siw_cep_put(cep); >> qp->cep = NULL; >> + siw_cep_put(cep); >> } >> cep->qp = NULL; >> + up_write(&qp->state_lock); >> siw_qp_put(qp); >> free_cep: >> siw_cep_set_free_and_put(cep); >> base-commit: a9394971825933074032794a5feee5211509c774 >