[PATCH for-next] RDMA/rxe: Fix "Replace mr by rkey in responder resources"

[PATCH for-next] RDMA/rxe: Fix "Replace mr by rkey in responder resources"

[Bob Pearson]

The referenced commit generates a reference counting error if
the the rkey has the same index but the wrong key. In this
case the reference taken by rxe_pool_get_index() is not dropped.

Drop the reference if the keys don't match in rxe_recheck_mr().
Check that the mw and mr are still valid.

Fixes: 8a1a0be894da0 ("RDMA/rxe: Replace mr by rkey in responder resources")
Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 16fc7ea1298d..1d95fab606da 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -680,6 +680,11 @@ static struct resp_res *rxe_prepare_read_res(struct rxe_qp *qp,
  * It is assumed that the access permissions if originally good
  * are OK and the mappings to be unchanged.
  *
+ * TODO: If someone reregisters an MR to change its size or
+ * access permissions during the processing of an RDMA read
+ * we should kill the responder resource and complete the
+ * operation with an error.
+ *
  * Return: mr on success else NULL
  */
 static struct rxe_mr *rxe_recheck_mr(struct rxe_qp *qp, u32 rkey)
@@ -690,23 +695,27 @@ static struct rxe_mr *rxe_recheck_mr(struct rxe_qp *qp, u32 rkey)
 
 	if (rkey_is_mw(rkey)) {
 		mw = rxe_pool_get_index(&rxe->mw_pool, rkey >> 8);
-		if (!mw || mw->rkey != rkey)
+		if (!mw)
 			return NULL;
 
-		if (mw->state != RXE_MW_STATE_VALID) {
+		mr = mw->mr;
+		if (mw->rkey != rkey || mw->state != RXE_MW_STATE_VALID ||
+		    !mr || mr->state != RXE_MR_STATE_VALID) {
 			rxe_put(mw);
 			return NULL;
 		}
 
-		mr = mw->mr;
+		rxe_get(mr);
 		rxe_put(mw);
-	} else {
-		mr = rxe_pool_get_index(&rxe->mr_pool, rkey >> 8);
-		if (!mr || mr->rkey != rkey)
-			return NULL;
+
+		return mr;
 	}
 
-	if (mr->state != RXE_MR_STATE_VALID) {
+	mr = rxe_pool_get_index(&rxe->mr_pool, rkey >> 8);
+	if (!mr)
+		return NULL;
+
+	if (mr->rkey != rkey || mr->state != RXE_MR_STATE_VALID) {
 		rxe_put(mr);
 		return NULL;
 	}
-- 
2.32.0

Re: [PATCH for-next] RDMA/rxe: Fix "Replace mr by rkey in responder resources"

[Jason Gunthorpe]

On Sun, Apr 10, 2022 at 10:06:48PM -0500, Bob Pearson wrote:
> The referenced commit generates a reference counting error if
> the the rkey has the same index but the wrong key. In this
> case the reference taken by rxe_pool_get_index() is not dropped.
> 
> Drop the reference if the keys don't match in rxe_recheck_mr().
> Check that the mw and mr are still valid.
> 
> Fixes: 8a1a0be894da0 ("RDMA/rxe: Replace mr by rkey in responder resources")
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_resp.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)

Applied to for-rc, thanks

Jason