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

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

[Michael Bommarito]

srp_process_rsp() copies sense data from rsp->data + resp_data_len,
where resp_data_len is the full 32-bit value supplied by the SRP target
and is never checked against the number of bytes actually received
(wc->byte_len). The copy length is bounded to SCSI_SENSE_BUFFERSIZE, so
at most 96 bytes are copied, but the source offset is not bounded.

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 a large resp_data_len. The receive buffer
is allocated at the target-chosen max_ti_iu_len, so the source of the
sense copy lands past the bytes actually received; with resp_data_len
near 0xFFFFFFFF it is gigabytes past the buffer and the read faults.

Copy the sense data only if it has not been truncated, that is, only if
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.

Changes since v1 (all per Bart Van Assche's review):
- Commit message: state that the sense copy is bounded to
  SCSI_SENSE_BUFFERSIZE; the unbounded value is the source offset, not
  the number of bytes copied.
- Describe the dropped case as truncated rather than oversized sense,
  in both the commit message and the log message.
- Guard on the full sense_data_len so the truncated-sense log message is
  accurate; the copy stays bounded to SCSI_SENSE_BUFFERSIZE.
Link to v1: https://lore.kernel.org/all/20260602194619.2272486-1-michael.bommarito@gmail.com/
 drivers/infiniband/ulp/srp/ib_srp.c | 30 +++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index b58868e1cf11c..42eee27e6b2a1 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,27 @@ 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 = be32_to_cpu(rsp->sense_data_len);
+
+			/*
+			 * The sense data starts resp_data_len bytes past the
+			 * response data area; both lengths come from the
+			 * target-controlled response.  Copy the sense data
+			 * only if it has not been truncated, that is, only if
+			 * the full sense region fits within the bytes actually
+			 * received.  Otherwise the copy source would run past
+			 * the receive buffer (sized to the target-chosen
+			 * max_ti_iu_len), reading out of bounds.
+			 */
+			if (sizeof(*rsp) + (u64)resp_len + sense_len <= byte_len)
+				memcpy(scmnd->sense_buffer, rsp->data + resp_len,
+				       min_t(u32, sense_len,
+					     SCSI_SENSE_BUFFERSIZE));
+			else
+				shost_printk(KERN_ERR, target->scsi_host,
+					     "dropping truncated sense data (resp_data_len %u sense_data_len %u, %u bytes received)\n",
+					     resp_len, sense_len, byte_len);
 		}
 
 		if (unlikely(rsp->flags & SRP_RSP_FLAG_DIUNDER))
@@ -2086,7 +2104,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 v2] RDMA/srp: bound SRP_RSP sense copy by the received length

[Bart Van Assche]

On 6/2/26 3:04 PM, Michael Bommarito wrote:
> +			if (sizeof(*rsp) + (u64)resp_len + sense_len <= byte_len)
> +				memcpy(scmnd->sense_buffer, rsp->data + resp_len,
> +				       min_t(u32, sense_len,
> +					     SCSI_SENSE_BUFFERSIZE));
The above min_t() probably can be changed into min(). Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

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

[Jason Gunthorpe]

On Tue, Jun 02, 2026 at 06:04:57PM -0400, Michael Bommarito wrote:
> srp_process_rsp() copies sense data from rsp->data + resp_data_len,
> where resp_data_len is the full 32-bit value supplied by the SRP target
> and is never checked against the number of bytes actually received
> (wc->byte_len). The copy length is bounded to SCSI_SENSE_BUFFERSIZE, so
> at most 96 bytes are copied, but the source offset is not bounded.
> 
> 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 a large resp_data_len. The receive buffer
> is allocated at the target-chosen max_ti_iu_len, so the source of the
> sense copy lands past the bytes actually received; with resp_data_len
> near 0xFFFFFFFF it is gigabytes past the buffer and the read faults.
> 
> Copy the sense data only if it has not been truncated, that is, only if
> 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>
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Applied with Bart's request

In future please be careful, maybe your AI hallucinated the fixes
line, it should be:

Fixes: aef9ec39c47f ("IB: Add SCSI RDMA Protocol (SRP) initiator")

Jason