[PATCH v5 2/3] RDMA/srpt: reject zero-length buffer descriptors

[Yehyeong Lee]

srpt_alloc_rw_ctxs() takes each buffer descriptor length straight from the
wire and hands it to target_alloc_sgl(), which has no length check of its
own.

For the single descriptor of a DIRECT request, and for the last descriptor
of an INDIRECT table, chainable is false, so a zero length makes
sgl_alloc_order() compute nalloc zero. kmalloc_objs() answers that with
ZERO_SIZE_PTR, which is not NULL, so the allocation looks like it
succeeded and sg_init_table(sgl, 0) runs. That reaches sg_init_marker(),
which does sg_mark_end(&sgl[nents - 1]) with nents unsigned, so the index
is 4294967295 rather than minus one and the read modify write of page_link
lands far outside any allocation.

  Oops: general protection fault, probably for non-canonical address 0xdffffc03fffffffe: 0000 [#1] KASAN PTI
  KASAN: probably user-memory-access in range [0x0000001ffffffff0-0x0000001ffffffff7]
  CPU: 0 UID: 0 PID: 43 Comm: kunit_try_catch Tainted: G                 N   7.3.0-12378-g6fba331225f5-dirty #1 PREEMPT(lazy)
  Tainted: [N]=TEST
  RIP: 0010:sgl_alloc_order+0xb6/0x370
  Code: ef 01 4c 89 e2 49 89 c4 31 f6 49 c1 e7 05 48 89 c7 e8 ae e0 cb ff 4d 01 e7 48 b8 00 00 00 00 00 fc ff df 4c 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 6b 02 00 00 49 bc 00 00 00 00 00 fc ff df 49 8b
  RSP: 0000:ffff888002d4fa38 EFLAGS: 00010202
  RAX: dffffc0000000000 RBX: 0000000000001000 RCX: 0000000000000000
  RDX: 00000003fffffffe RSI: 0000000000000000 RDI: 0000000000000010
  RBP: 0000000000000000 R08: ffff888002b158a0 R09: 0000000000000000
  R10: 0000000000000010 R11: 0000000000000037 R12: 0000000000000010
  R13: ffff888002d6803c R14: 0000000000000000 R15: 0000001ffffffff0
  FS:  0000000000000000(0000) GS:0000000000000000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: ffff88802f1c8000 CR3: 000000002e458000 CR4: 00000000000006f0
  Call Trace:
   <TASK>
   target_alloc_sgl+0x2b/0x70
   srpt_alloc_rw_ctxs+0x2ef/0x870
   srpt_test_parse+0x114/0x580
   ? __pfx_srpt_test_parse+0x10/0x10
   ? srpt_test_ctx_init_direct+0x17d/0x280
   srpt_direct_len0_test+0xa4/0x280
   ? __pfx_srpt_direct_len0_test+0x10/0x10
   ? __set_cpus_allowed_ptr+0x181/0x240
   ? __pfx_read_tsc+0x10/0x10
   ? ktime_get_ts64+0x192/0x250
   kunit_try_run_case+0x1ca/0x4b0
   ? __pfx_kunit_try_run_case+0x10/0x10
   ? kthread_affine_node+0x24b/0x370
   ? __pfx_kthread_affine_node+0x10/0x10
   ? __pfx_kunit_try_run_case+0x10/0x10
   ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10
   kunit_generic_run_threadfn_adapter+0x7b/0xe0
   kthread+0x36e/0x500
   ? __pfx_kthread+0x10/0x10
   ret_from_fork+0x34d/0x620
   ? __pfx_ret_from_fork+0x10/0x10
   ? __switch_to+0x22/0xdd0
   ? __pfx_kthread+0x10/0x10
   ret_from_fork_asm+0x19/0x30
   </TASK>

The KUnit harness that forces this is not part of this patch.

A zero length descriptor elsewhere in a table is chainable, so one entry
is allocated and that write does not happen, but the entry count comes
back zero and the mapping in rdma_rw_ctx_init() then refuses it.

The target core never hands the same allocator a zero length. Its own call
in transport_generic_new_cmd() sits behind a cmd->data_length test.

Reject a descriptor whose length is zero. The existing unwind label covers
the first iteration, where its loop body does not run because nothing has
been allocated yet.

Fixes: b99f8e4d7bcd ("IB/srpt: convert to the generic RDMA READ/WRITE API")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 40d796840befc..dfcecbe3a47c7 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -979,6 +979,12 @@ static int srpt_alloc_rw_ctxs(struct srpt_send_ioctx *ioctx,
 		u32 size = be32_to_cpu(db->len);
 		u32 rkey = be32_to_cpu(db->key);
 
+		if (size == 0) {
+			pr_err("received SRP_CMD buffer descriptor with zero length\n");
+			ret = -EINVAL;
+			goto unwind;
+		}
+
 		ret = target_alloc_sgl(&ctx->sg, &ctx->nents, size, false,
 				i < nbufs - 1);
 		if (ret)
-- 
2.43.0