[PATCH] RDMA/srp: bound SRP_RSP sense copy by the received length

[PATCH] RDMA/srp: bound SRP_RSP sense copy by the received length

[Michael Bommarito]

srp_process_rsp() in drivers/infiniband/ulp/srp/ib_srp.c copies sense
data with the source pointer rsp->data + be32_to_cpu(rsp->resp_data_len),
where resp_data_len is the full 32-bit value supplied by the SRP target
and is never checked against the size of the receive buffer or the
number of bytes actually received (wc->byte_len).

A malicious or compromised SRP target on the InfiniBand/RoCE fabric that
the initiator has logged into can return an SRP_RSP with
SRP_RSP_FLAG_SNSVALID set and resp_data_len set to a large value such as
0xFFFFFFFF. The receive buffer is allocated at the target-chosen
max_ti_iu_len, so the copy source lands far past the allocation.

The memcpy then reads out of bounds of the kzalloc'd receive IU; with
resp_data_len near 0xFFFFFFFF the source is multiple gigabytes past the
buffer and faults.

Pass wc->byte_len into srp_process_rsp() and copy the sense data only
when the response header, the response data, and the sense region fit
within the bytes actually received; otherwise drop the sense and log.
The in-tree iSER and NVMe-RDMA receive paths already bound their parse
by wc->byte_len; this brings ib_srp into line with them.

Fixes: aef9ec39c40c ("IB: Add SCSI RDMA Protocol (SRP) initiator")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---

Impact: a malicious or compromised SRP target the initiator has logged
into can trigger an out-of-bounds read in srp_process_rsp() by returning
an SRP_RSP with SRP_RSP_FLAG_SNSVALID and a large resp_data_len, faulting
the initiator's SRP completion path.

 drivers/infiniband/ulp/srp/ib_srp.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index b58868e1cf11c..199d46fb12146 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1932,7 +1932,8 @@ static int srp_post_recv(struct srp_rdma_ch *ch, struct srp_iu *iu)
 	return ib_post_recv(ch->qp, &wr, NULL);
 }
 
-static void srp_process_rsp(struct srp_rdma_ch *ch, struct srp_rsp *rsp)
+static void srp_process_rsp(struct srp_rdma_ch *ch, struct srp_rsp *rsp,
+			    u32 byte_len)
 {
 	struct srp_target_port *target = ch->target;
 	struct srp_request *req;
@@ -1973,10 +1974,26 @@ static void srp_process_rsp(struct srp_rdma_ch *ch, struct srp_rsp *rsp)
 		scmnd->result = rsp->status;
 
 		if (rsp->flags & SRP_RSP_FLAG_SNSVALID) {
-			memcpy(scmnd->sense_buffer, rsp->data +
-			       be32_to_cpu(rsp->resp_data_len),
-			       min_t(int, be32_to_cpu(rsp->sense_data_len),
-				     SCSI_SENSE_BUFFERSIZE));
+			u32 resp_len = be32_to_cpu(rsp->resp_data_len);
+			u32 sense_len = min_t(u32,
+					      be32_to_cpu(rsp->sense_data_len),
+					      SCSI_SENSE_BUFFERSIZE);
+
+			/*
+			 * The sense data starts resp_data_len bytes past the
+			 * response data area.  Both lengths are taken from the
+			 * target controlled response, so reject a response
+			 * whose sense region would run past the bytes actually
+			 * received rather than reading out of bounds of the
+			 * receive buffer.
+			 */
+			if (sizeof(*rsp) + (u64)resp_len + sense_len <= byte_len)
+				memcpy(scmnd->sense_buffer,
+				       rsp->data + resp_len, sense_len);
+			else
+				shost_printk(KERN_ERR, target->scsi_host,
+					     "dropping oversized sense (resp_data_len %u sense_data_len %u) in %u-byte RSP\n",
+					     resp_len, sense_len, byte_len);
 		}
 
 		if (unlikely(rsp->flags & SRP_RSP_FLAG_DIUNDER))
@@ -2086,7 +2103,7 @@ static void srp_recv_done(struct ib_cq *cq, struct ib_wc *wc)
 
 	switch (opcode) {
 	case SRP_RSP:
-		srp_process_rsp(ch, iu->buf);
+		srp_process_rsp(ch, iu->buf, wc->byte_len);
 		break;
 
 	case SRP_CRED_REQ:
-- 
2.53.0

Re: [PATCH] RDMA/srp: bound SRP_RSP sense copy by the received length

[Bart Van Assche]

On 6/2/26 12:46 PM, Michael Bommarito wrote:
> A malicious or compromised SRP target on the InfiniBand/RoCE fabric that
> the initiator has logged into can return an SRP_RSP with
> SRP_RSP_FLAG_SNSVALID set and resp_data_len set to a large value such as
> 0xFFFFFFFF. The receive buffer is allocated at the target-chosen
> max_ti_iu_len, so the copy source lands far past the allocation.
> 
> The memcpy then reads out of bounds of the kzalloc'd receive IU; with
> resp_data_len near 0xFFFFFFFF the source is multiple gigabytes past the
> buffer and faults.

The above is misleading because it does not mention that the SRP
initiator copies at most SCSI_SENSE_BUFFERSIZE bytes sense data.

> Pass wc->byte_len into srp_process_rsp() and copy the sense data only
> when the response header, the response data, and the sense region fit
> within the bytes actually received; otherwise drop the sense and log.
> The in-tree iSER and NVMe-RDMA receive paths already bound their parse
> by wc->byte_len; this brings ib_srp into line with them.

This sounds weird. I'd write this as follows: "... copy only if the
sense data has not been truncated".

> +			else
> +				shost_printk(KERN_ERR, target->scsi_host,
> +					     "dropping oversized sense (resp_data_len %u sense_data_len %u) in %u-byte RSP\n",
> +					     resp_len, sense_len, byte_len);
No, in this case the sense data is not oversized but has been truncated.

Thanks,

Bart.