[PATCH] RDMA/srpt: widen the RDMA context counters

[Yehyeong Lee]

srpt_get_desc_tbl() bounds the number of indirect data buffer descriptors
by the two counts the initiator put in the SRP_CMD, and both are u8, so it
admits an nbufs of up to 510:

	if (nbufs >
	    (srp_cmd->data_out_desc_cnt + srp_cmd->data_in_desc_cnt)) {
		...
		return -EINVAL;
	}

srpt_alloc_rw_ctxs() then builds that many contexts but counts them in
ioctx->n_rw_ctx and ioctx->n_rdma, which are u8.  At 256 descriptors
n_rw_ctx wraps to zero, and srpt_release_cmd() frees the contexts only
when it is nonzero:

	if (ioctx->n_rw_ctx) {
		srpt_free_rw_ctxs(ch, ioctx);
		ioctx->n_rw_ctx = 0;
	}

A command carrying 256 descriptors therefore leaks every scatterlist
target_alloc_sgl() allocated, every registration rdma_rw_ctx_init() set
up, and the rw_ctxs array as well, since the kfree() for it sits inside
srpt_free_rw_ctxs().  ioctx is a ring entry that the next command on the
same tag overwrites, so the array pointer is lost too.  The descriptor
payload sizes are the initiator's to choose and the command can be
repeated, so this grows without bound.  A count between 257 and 510 leaks
256 contexts and frees the remainder.

Widen both counters so they hold what the guard already admits, rather
than refusing counts the guard accepts.  int suits every existing user:
the two loops that walk the contexts backwards use an int index and stop
at -1, the sq_wr_avail accounting passes the value to atomic_add() and
atomic_sub_return(), which take an int, and the three places that print it
use %d.  The WARN_ON(ioctx->n_rdma <= 0) in srpt_rdma_read_done() becomes
a real signed test instead of a test for zero.  The error unwind path
walks by index and restores both counters, so it was never affected.

There is no splat to quote, because the result is a leak rather than an
access past an object.  A KUnit harness that is not part of this patch
calls srpt_get_desc_tbl() with a fabricated SRP_CMD and counts the
rdma_rw_ctx_init() and rdma_rw_ctx_destroy() calls.  With u8 counters, 256
descriptors build 256 contexts and destroy none, and 510 build 510 and
destroy 254.  With int counters, 1, 128, 255, 256 and 510 descriptors all
build and destroy the same number.

Fixes: a42d985bd5b2 ("ib_srpt: Initial SRP Target merge for v3.3-rc1")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 2 +-
 drivers/infiniband/ulp/srpt/ib_srpt.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 7197d95f2216b..b174d58ca98a3 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -959,7 +959,7 @@ static int srpt_alloc_rw_ctxs(struct srpt_send_ioctx *ioctx,
 	struct srpt_rdma_ch *ch = ioctx->ch;
 	struct scatterlist *prev = NULL;
 	unsigned prev_nents;
-	u8 n_rdma, n_rw_ctx;
+	int n_rdma, n_rw_ctx;
 	int ret, i;
 
 	if (nbufs == 1) {
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 4c46b301eea18..1dce9bcb490f0 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -223,8 +223,8 @@ struct srpt_send_ioctx {
 	struct ib_cqe		rdma_cqe;
 	enum srpt_command_state	state;
 	struct se_cmd		cmd;
-	u8			n_rdma;
-	u8			n_rw_ctx;
+	int			n_rdma;
+	int			n_rw_ctx;
 	bool			queue_status_only;
 	u8			sense_data[TRANSPORT_SENSE_BUFFER];
 };

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.43.0