linux-kernel - RE: [PATCH] RDMA/siw: Reuse value read using READ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: 
 <BYAPR15MB32080B7AA8255352C1F691A199242@BYAPR15MB3208.namprd15.prod.outlook.com>
Date: Mon, 11 Mar 2024 14:14:09 +0000
From: Bernard Metzler <BMT@...ich.ibm.com>
To: linke li <lilinke99@...com>
CC: Jason Gunthorpe <jgg@...pe.ca>, Leon Romanovsky <leon@...nel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE:  [PATCH] RDMA/siw: Reuse value read using READ_ONCE instead of
 re-reading it



> -----Original Message-----
> From: linke li <lilinke99@...com>
> Sent: Saturday, March 9, 2024 1:27 PM
> Cc: lilinke99@...com; Bernard Metzler <BMT@...ich.ibm.com>; Jason Gunthorpe
> <jgg@...pe.ca>; Leon Romanovsky <leon@...nel.org>; linux-
> rdma@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: [EXTERNAL] [PATCH] RDMA/siw: Reuse value read using READ_ONCE
> instead of re-reading it
> 
> In siw_orqe_start_rx, the orqe's flag in the if condition is read using
> READ_ONCE, checked, and then re-read, voiding all guarantees of the
> checks. Reuse the value that was read by READ_ONCE to ensure the
> consistency of the flags throughout the function.
> 
> Signed-off-by: linke li <lilinke99@...com>
> ---
>  drivers/infiniband/sw/siw/siw_qp_rx.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/siw/siw_qp_rx.c
> b/drivers/infiniband/sw/siw/siw_qp_rx.c
> index ed4fc39718b4..f5f69de56882 100644
> --- a/drivers/infiniband/sw/siw/siw_qp_rx.c
> +++ b/drivers/infiniband/sw/siw/siw_qp_rx.c
> @@ -740,6 +740,7 @@ static int siw_orqe_start_rx(struct siw_qp *qp)
>  {
>  	struct siw_sqe *orqe;
>  	struct siw_wqe *wqe = NULL;
> +	u16 orqe_flags;
> 
>  	if (unlikely(!qp->attrs.orq_size))
>  		return -EPROTO;
> @@ -748,7 +749,8 @@ static int siw_orqe_start_rx(struct siw_qp *qp)
>  	smp_mb();
> 
>  	orqe = orq_get_current(qp);
> -	if (READ_ONCE(orqe->flags) & SIW_WQE_VALID) {
> +	orqe_flags = READ_ONCE(orqe->flags);
> +	if (orqe_flags & SIW_WQE_VALID) {
>  		/* RRESP is a TAGGED RDMAP operation */
>  		wqe = rx_wqe(&qp->rx_tagged);
>  		wqe->sqe.id = orqe->id;
> @@ -756,7 +758,7 @@ static int siw_orqe_start_rx(struct siw_qp *qp)
>  		wqe->sqe.sge[0].laddr = orqe->sge[0].laddr;
>  		wqe->sqe.sge[0].lkey = orqe->sge[0].lkey;
>  		wqe->sqe.sge[0].length = orqe->sge[0].length;
> -		wqe->sqe.flags = orqe->flags;
> +		wqe->sqe.flags = orqe_flags;
>  		wqe->sqe.num_sge = 1;
>  		wqe->bytes = orqe->sge[0].length;
>  		wqe->processed = 0;
> --
> 2.39.3 (Apple Git-146)
> 
> 

The outbound read queue (orq) is a ring buffer with only one
consumer (this code) and one producer (READ.request sending
code). There is no parallel reader and a single writer.

The producer (sender of the READ.request) sets the orq entry
valid and does this only once after completely writing
the entry. It does it under qp->orq_lock.

Only if we find the orq entry valid, its content gets copied
at the beginning of a new READ.response (this code).

The orq entry remains valid to stop the producer from re-using
it until the complete READ.response has been received (may be
multiple fragments). The flag gets cleared under qp->orq_lock
after the complete READ.response has been received, or the
response was invalid.


There is no possibility a valid orq entry gets invalidated
after it has been found valid, so it is safe to copy all its
members. 

Thanks,
Bernard.