RE: [PATCH] RDMA/siw: Fix a condition race issue in MPA request processing

From: Bernard Metzler <BMT@zurich.ibm.com>
To: Cheng Xu <chengyou@linux.alibaba.com>,
	"jgg@ziepe.ca" <jgg@ziepe.ca>,
	"leon@kernel.org" <leon@kernel.org>
Cc: "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: RE: [PATCH] RDMA/siw: Fix a condition race issue in MPA request processing
Date: Wed, 4 May 2022 14:42:37 +0000	[thread overview]
Message-ID: <BYAPR15MB2631BAFC4CA45510B933ACCF99C39@BYAPR15MB2631.namprd15.prod.outlook.com> (raw)

> -----Original Message-----
> From: Cheng Xu <chengyou@linux.alibaba.com>
> Sent: Sunday, 24 April 2022 10:01
> To: jgg@ziepe.ca; leon@kernel.org; Bernard Metzler <BMT@zurich.ibm.com>
> Cc: linux-rdma@vger.kernel.org; chengyou@linux.alibaba.com
> Subject: [EXTERNAL] [PATCH] RDMA/siw: Fix a condition race issue in MPA
> request processing
> 
> The calling of siw_cm_upcall and detaching new_cep with its
> listen_cep should be atomistic semantics. Otherwise siw_reject
> may be called in a temporary state, e,g, siw_cm_upcall is called
> but the new_cep->listen_cep has not being cleared.
> 
> This will generate a WARN in dmesg, which reported in:
> INVALID URI REMOVED
> 3A__lore.kernel.org_all_Yliu2ROIh0nLk5l0-
> 40bombadil.infradead.org_&d=DwIDAg&c=jf_iaSHvJObTbx-siA1ZOg&r=2TaYXQ0T-
> r8ZO1PP1alNwU_QJcRRLfmYTAgd3QCvqSc&m=QsPgyfwbTNAi5ZoVzi_NKB-
> L3RbMSdgjD62GPdqPku_yQxeIBYOcBQUcbOLBkb_1&s=pGb_jGW0mrGHjXZCvag7dRdHT0RA
> UaquT15AyeAy4rg&e=
> 
> Reported-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Cheng Xu <chengyou@linux.alibaba.com>
> ---
>  drivers/infiniband/sw/siw/siw_cm.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/siw/siw_cm.c
> b/drivers/infiniband/sw/siw/siw_cm.c
> index 7acdd3c3a599..17f34d584cd9 100644
> --- a/drivers/infiniband/sw/siw/siw_cm.c
> +++ b/drivers/infiniband/sw/siw/siw_cm.c
> @@ -968,14 +968,15 @@ static void siw_accept_newconn(struct siw_cep
> *cep)
> 
>  		siw_cep_set_inuse(new_cep);
>  		rv = siw_proc_mpareq(new_cep);
> -		siw_cep_set_free(new_cep);
> -
>  		if (rv != -EAGAIN) {
>  			siw_cep_put(cep);
>  			new_cep->listen_cep = NULL;
> -			if (rv)
> +			if (rv) {
> +				siw_cep_set_free(new_cep);
>  				goto error;
> +			}
>  		}
> +		siw_cep_set_free(new_cep);
>  	}
>  	return;
> 
> --
> 2.32.0 (Apple Git-132)

While I was so far unable to reproduce it, the patch makes
sense to me. It fixes a potential race condition. Thanks!

Reviewed-by: Bernard Metzler <bmt@zurich.ibm.com>