[PATCH] IB/isert: reject PDUs declaring more data than was received

[PATCH] IB/isert: reject PDUs declaring more data than was received

[Yehyeong Lee]

isert_recv_done() hands each received PDU to the opcode handlers without
ever looking at wc->byte_len, the number of bytes the HCA actually placed
in the receive descriptor. The handlers then copy that many bytes - the
data-segment length the initiator declared in the BHS
(ntoh24(hdr->dlength), via the derived unsol_data_len / imm_data_len) -
out of the fixed-size descriptor:

  isert_handle_iscsi_dataout():
        sg_copy_from_buffer(sg_start, sg_nents, isert_get_data(rx_desc),
                            unsol_data_len);
  isert_handle_scsi_cmd():
        sg_copy_from_buffer(cmd->se_cmd.t_data_sg, sg_nents,
                            isert_get_data(rx_desc), imm_data_len);

Because the declared length is never checked against wc->byte_len, an
initiator can declare a data segment larger than the bytes it actually
sent (and larger than the descriptor) and cause an out-of-bounds read of
the receive buffer.

Nothing upstream of isert closes this door:

  - __iscsit_check_dataout_hdr() bounds the inbound payload against
    conn_ops->MaxXmitDataSegmentLength (MXDSL) - a transmit parameter,
    used here for the inbound check.
  - iscsi_set_connection_parameters() sets
    ops->MaxXmitDataSegmentLength = ops->TargetRecvDataSegmentLength;
    and TARGETRECVDATASEGMENTLENGTH is absent from the min()-clamp list in
    iscsi_check_acceptor_state(), so the value the initiator declares is
    adopted verbatim (type range 512..16777215). The initiator effectively
    raises its own ceiling.
  - isert never clamps the negotiated value to its own fixed receive
    descriptor (ISER_RX_SIZE, 9216 bytes), so the target core's bound and
    the descriptor size are unrelated.

The imm_data_len == data_len path is more than an over-read: it aliases
the receive descriptor via sg_set_buf() and passes it to the backend as
the data source for the SCSI WRITE, so an over-declared length causes heap
contents past the descriptor to be written through the backend to the
backing store. The backend is the victim of the oversized scatterlist
isert hands it, not the cause; no read-back of the written bytes was
demonstrated.

Trigger: after login completes (full feature phase), an initiator that has
declared a large TargetRecvDataSegmentLength and a FirstBurstLength that
permits unsolicited/immediate data sends a PDU whose declared data-segment
length exceeds what was received. With KASAN:

  BUG: KASAN: slab-out-of-bounds in sg_copy_buffer+0x150/0x1c0
  Read of size 4096 at addr ffff888109720800 by task kworker/1:0H/25
  Workqueue: ib-comp-wq ib_cq_poll_work
  Call Trace:
   sg_copy_buffer+0x150/0x1c0
   isert_recv_done+0xba6/0x2390
   __ib_process_cq+0xe1/0x390
   ib_cq_poll_work+0x46/0x150

isert_recv_done+0xba6 resolves to isert_handle_iscsi_dataout()
(ib_isert.c:1160), inlined through isert_rx_opcode().

Validate wc->byte_len against the framing in isert_recv_done() before the
PDU reaches any handler, and reinstate the connection if it is short.
Because the test compares without subtracting the header length, it also
rejects PDUs shorter than the iSER and iSCSI headers, which would otherwise
be parsed out of stale descriptor contents. The sibling login handler,
isert_login_recv_done(), already rejects PDUs shorter than ISER_HEADERS_LEN
(commit 29e7b925ae6d ("IB/isert: Reject login PDUs shorter than
ISER_HEADERS_LEN")); the data handlers had no length check at all.

isert reads the data segment from a fixed offset: isert_get_data()
returns the iSER header plus ISER_HEADERS_LEN and makes no adjustment for
an AHS.  The bytes the handlers touch are therefore exactly
[ISER_HEADERS_LEN, ISER_HEADERS_LEN + dlength), and comparing that sum
against wc->byte_len bounds precisely the region that is read.  An AHS
term would only make the test stricter without bounding anything further,
and cannot cause a false reject: a PDU carrying an AHS is longer, not
shorter.

This is a memory-safety fix that verifies the bytes that were actually
received; it does not touch RFC 7145 length negotiation and is not the
MaxXmitDataSegmentLength negotiation redesign raised in the 2017 "[Query]
iSER-Target: QP errors observed on increasing MaxXmitDataSegmentLength"
discussion. That redesign is explicitly out of scope here.

The patched kernel rejects the malformed DataOut PDU and both
immediate-data variants with "PDU declares ... bytes were received" and
continues to pass normal traffic with no regression.

Reproduced with soft-RoCE (rdma_rxe) and a raw rdma_cm/ibv initiator; no
kernel-side test hooks were needed.

Fixes: b8d26b3be8b3 ("iser-target: Add iSCSI Extensions for RDMA (iSER) target driver")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
 drivers/infiniband/ulp/isert/ib_isert.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 1015a51f750a..66435a0a5c7a 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -1333,6 +1333,21 @@ isert_recv_done(struct ib_cq *cq, struct ib_wc *wc)
 	ib_dma_sync_single_for_cpu(ib_dev, rx_desc->dma_addr,
 			ISER_RX_SIZE, DMA_FROM_DEVICE);
 
+	/*
+	 * The data segment length declared in the BHS is attacker controlled
+	 * and is used further down to read that many bytes out of the fixed
+	 * size receive descriptor, so it has to be checked against the number
+	 * of bytes that were actually received. Comparing without subtracting
+	 * also rejects PDUs shorter than the iSER and iSCSI headers, which
+	 * would otherwise be parsed out of stale descriptor contents.
+	 */
+	if (unlikely(wc->byte_len < ISER_HEADERS_LEN + ntoh24(hdr->dlength))) {
+		isert_err("PDU declares %u data bytes but only %u bytes were received\n",
+			  ntoh24(hdr->dlength), wc->byte_len);
+		iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
+		return;
+	}
+
 	isert_dbg("DMA: 0x%llx, iSCSI opcode: 0x%02x, ITT: 0x%08x, flags: 0x%02x dlen: %d\n",
 		 rx_desc->dma_addr, hdr->opcode, hdr->itt, hdr->flags,
 		 (int)(wc->byte_len - ISER_HEADERS_LEN));

base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df
-- 
2.43.0

Re: [PATCH] IB/isert: reject PDUs declaring more data than was received

[Leon Romanovsky]

On Sat, Jul 25, 2026 at 10:18:46AM +0900, Yehyeong Lee wrote:
> isert_recv_done() hands each received PDU to the opcode handlers without
> ever looking at wc->byte_len, the number of bytes the HCA actually placed
> in the receive descriptor. The handlers then copy that many bytes - the
> data-segment length the initiator declared in the BHS
> (ntoh24(hdr->dlength), via the derived unsol_data_len / imm_data_len) -
> out of the fixed-size descriptor:
> 
>   isert_handle_iscsi_dataout():
>         sg_copy_from_buffer(sg_start, sg_nents, isert_get_data(rx_desc),
>                             unsol_data_len);
>   isert_handle_scsi_cmd():
>         sg_copy_from_buffer(cmd->se_cmd.t_data_sg, sg_nents,
>                             isert_get_data(rx_desc), imm_data_len);
> 
> Because the declared length is never checked against wc->byte_len, an
> initiator can declare a data segment larger than the bytes it actually
> sent (and larger than the descriptor) and cause an out-of-bounds read of
> the receive buffer.
> 
> Nothing upstream of isert closes this door:
> 
>   - __iscsit_check_dataout_hdr() bounds the inbound payload against
>     conn_ops->MaxXmitDataSegmentLength (MXDSL) - a transmit parameter,
>     used here for the inbound check.
>   - iscsi_set_connection_parameters() sets
>     ops->MaxXmitDataSegmentLength = ops->TargetRecvDataSegmentLength;
>     and TARGETRECVDATASEGMENTLENGTH is absent from the min()-clamp list in
>     iscsi_check_acceptor_state(), so the value the initiator declares is
>     adopted verbatim (type range 512..16777215). The initiator effectively
>     raises its own ceiling.
>   - isert never clamps the negotiated value to its own fixed receive
>     descriptor (ISER_RX_SIZE, 9216 bytes), so the target core's bound and
>     the descriptor size are unrelated.
> 
> The imm_data_len == data_len path is more than an over-read: it aliases
> the receive descriptor via sg_set_buf() and passes it to the backend as
> the data source for the SCSI WRITE, so an over-declared length causes heap
> contents past the descriptor to be written through the backend to the
> backing store. The backend is the victim of the oversized scatterlist
> isert hands it, not the cause; no read-back of the written bytes was
> demonstrated.
> 
> Trigger: after login completes (full feature phase), an initiator that has
> declared a large TargetRecvDataSegmentLength and a FirstBurstLength that
> permits unsolicited/immediate data sends a PDU whose declared data-segment
> length exceeds what was received. With KASAN:
> 
>   BUG: KASAN: slab-out-of-bounds in sg_copy_buffer+0x150/0x1c0
>   Read of size 4096 at addr ffff888109720800 by task kworker/1:0H/25
>   Workqueue: ib-comp-wq ib_cq_poll_work
>   Call Trace:
>    sg_copy_buffer+0x150/0x1c0
>    isert_recv_done+0xba6/0x2390
>    __ib_process_cq+0xe1/0x390
>    ib_cq_poll_work+0x46/0x150
> 
> isert_recv_done+0xba6 resolves to isert_handle_iscsi_dataout()
> (ib_isert.c:1160), inlined through isert_rx_opcode().
> 
> Validate wc->byte_len against the framing in isert_recv_done() before the
> PDU reaches any handler, and reinstate the connection if it is short.
> Because the test compares without subtracting the header length, it also
> rejects PDUs shorter than the iSER and iSCSI headers, which would otherwise
> be parsed out of stale descriptor contents. The sibling login handler,
> isert_login_recv_done(), already rejects PDUs shorter than ISER_HEADERS_LEN
> (commit 29e7b925ae6d ("IB/isert: Reject login PDUs shorter than
> ISER_HEADERS_LEN")); the data handlers had no length check at all.
> 
> isert reads the data segment from a fixed offset: isert_get_data()
> returns the iSER header plus ISER_HEADERS_LEN and makes no adjustment for
> an AHS.  The bytes the handlers touch are therefore exactly
> [ISER_HEADERS_LEN, ISER_HEADERS_LEN + dlength), and comparing that sum
> against wc->byte_len bounds precisely the region that is read.  An AHS
> term would only make the test stricter without bounding anything further,
> and cannot cause a false reject: a PDU carrying an AHS is longer, not
> shorter.
> 
> This is a memory-safety fix that verifies the bytes that were actually
> received; it does not touch RFC 7145 length negotiation and is not the
> MaxXmitDataSegmentLength negotiation redesign raised in the 2017 "[Query]
> iSER-Target: QP errors observed on increasing MaxXmitDataSegmentLength"
> discussion. That redesign is explicitly out of scope here.
> 
> The patched kernel rejects the malformed DataOut PDU and both
> immediate-data variants with "PDU declares ... bytes were received" and
> continues to pass normal traffic with no regression.
> 
> Reproduced with soft-RoCE (rdma_rxe) and a raw rdma_cm/ibv initiator; no
> kernel-side test hooks were needed.
> 
> Fixes: b8d26b3be8b3 ("iser-target: Add iSCSI Extensions for RDMA (iSER) target driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
> ---
>  drivers/infiniband/ulp/isert/ib_isert.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> index 1015a51f750a..66435a0a5c7a 100644
> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> @@ -1333,6 +1333,21 @@ isert_recv_done(struct ib_cq *cq, struct ib_wc *wc)
>  	ib_dma_sync_single_for_cpu(ib_dev, rx_desc->dma_addr,
>  			ISER_RX_SIZE, DMA_FROM_DEVICE);
>  
> +	/*
> +	 * The data segment length declared in the BHS is attacker controlled
> +	 * and is used further down to read that many bytes out of the fixed
> +	 * size receive descriptor, so it has to be checked against the number
> +	 * of bytes that were actually received. Comparing without subtracting
> +	 * also rejects PDUs shorter than the iSER and iSCSI headers, which
> +	 * would otherwise be parsed out of stale descriptor contents.
> +	 */
> +	if (unlikely(wc->byte_len < ISER_HEADERS_LEN + ntoh24(hdr->dlength))) {
> +		isert_err("PDU declares %u data bytes but only %u bytes were received\n",
> +			  ntoh24(hdr->dlength), wc->byte_len);
> +		iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
> +		return;
> +	}

1. You should review patches generated by AI.
2. There is a need to add similar check to isert_get_login_rx() too.

Something like this:

Thanks
commit ffe5e4081fa535412fdd5a685369441076a4d1e1 (HEAD -> b4/the-iser-login-handler-fails-to-vali, origin/the-iser-login-handler-fails-to-vali-v1)
Author: Leon Romanovsky <leonro@nvidia.com>
Date:   Sat Jul 25 05:40:22 2026 +0300

    IB/isert: Reject login PDUs declaring more data than received

    isert never compares the iSCSI header's 24-bit data length against the
    bytes actually received for a login PDU. A remote initiator can declare far
    more data than it sends, and before target lookup or authentication:

        isert_rx_login_req()
        iscsi_target_locate_portal()
        kmemdup_nul()

    reads ntoh24(dlength) bytes out of the fixed 8192-byte login buffer,
    overrunning it.

    Validate the declared length against the received login_req_len in both the
    initial (isert_get_login_rx()) and subsequent (isert_login_recv_done())
    login paths before any copy. Allow dlength <= login_req_len, since the
    received count may include up to three iSCSI padding bytes.

    Fixes: b8d26b3be8b3 ("iser-target: Add iSCSI Extensions for RDMA (iSER) target driver")
    Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 4691845bf815..02ce4cbe69a1 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -970,6 +970,19 @@ isert_put_login_tx(struct iscsit_conn *conn, struct iscsi_login *login,
        return 0;
 }

+static int
+isert_check_login_req(struct isert_conn *isert_conn)
+{
+       struct iscsi_hdr *hdr =
+               (struct iscsi_hdr *)isert_get_iscsi_hdr(isert_conn->login_desc);
+       u32 dlength = ntoh24(hdr->dlength);
+
+       if (dlength > isert_conn->login_req_len)
+               return -EPROTO;
+
+       return 0;
+}
+
 static void
 isert_rx_login_req(struct isert_conn *isert_conn)
 {
@@ -1393,8 +1406,11 @@ isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc)
        if (isert_conn->conn) {
                struct iscsi_login *login = isert_conn->conn->conn_login;

-               if (login && !login->first_request)
+               if (login && !login->first_request) {
+                       if (isert_check_login_req(isert_conn))
+                               return;
                        isert_rx_login_req(isert_conn);
+               }
        }

        mutex_lock(&isert_conn->mutex);
@@ -2359,6 +2375,10 @@ isert_get_login_rx(struct iscsit_conn *conn, struct iscsi_login *login)
        if (!login->first_request)
                return 0;

+       ret = isert_check_login_req(isert_conn);
+       if (ret)
+               return ret;
+
        isert_rx_login_req(isert_conn);

        isert_info("before login_comp conn: %p\n", conn);

Thanks


> +
>  	isert_dbg("DMA: 0x%llx, iSCSI opcode: 0x%02x, ITT: 0x%08x, flags: 0x%02x dlen: %d\n",
>  		 rx_desc->dma_addr, hdr->opcode, hdr->itt, hdr->flags,
>  		 (int)(wc->byte_len - ISER_HEADERS_LEN));
> 
> base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df
> -- 
> 2.43.0
> 
> 

Re: [PATCH] IB/isert: reject PDUs declaring more data than was received

[Yehyeong Lee]

On Sun, Jul 26, 2026 at 09:46:28AM +0300, Leon Romanovsky wrote:
> 1. You should review patches generated by AI.
> 2. There is a need to add similar check to isert_get_login_rx() too.

You are right on both counts.  My changelog said the login handler
"already rejects PDUs shorter than ISER_HEADERS_LEN"; I confirmed that a
length check existed there and did not confirm that it was the same
check.  It is not - 29e7b925ae6d bounds login_req_len from below, and
nothing bounds the length the BHS declares.

I reproduced it before replying.  An initiator that sends a 105-byte
first login PDU while declaring 8193 in the BHS DataSegmentLength, on
7.2.0-rc4 with KASAN over soft-RoCE:

  BUG: KASAN: slab-out-of-bounds in kmemdup_nul+0x43/0x80
  Read of size 8193 at addr ffff8881056a8000 by task iscsi_np/167
   __asan_memcpy+0x23/0x60
   kmemdup_nul+0x43/0x80
   iscsi_target_locate_portal+0x48d/0x1180
   iscsi_target_login_thread+0x19a9/0x3350
  Allocated by task 167:
   __kmalloc_cache_noprof+0x158/0x370
   iscsi_target_login_thread+0x971/0x3350
  The buggy address belongs to the object at ffff8881056a8000
   which belongs to the cache kmalloc-8k of size 8192
  allocated 8192-byte region

faddr2line puts the allocation at iscsi_login_init_conn(),
iscsi_target_login.c:50 - login->req_buf = kzalloc(MAX_KEY_VALUE_PAIRS) -
and the read at iscsi_target_nego.c:1136, the kmemdup_nul() in
iscsi_target_locate_portal().  The read size tracks the declared value
exactly; 16384 gives "Read of size 16384", and a control run with a
correct DataSegmentLength produces no report.  It is reached before
authentication and the length is attacker-chosen.

On why isert is the one exposed: iscsi_target_check_login_request()
already rejects payload_length > MAX_KEY_VALUE_PAIRS, but it is only
called from iscsit_get_login_rx() and from cxgbit.  isert does not call
it.  iscsit over TCP is safe by construction in any case - it reads
exactly payload_length + padding from the socket, so the declared length
governs how much arrives rather than how much is copied out of an
already-filled buffer.

Your check is sufficient and not merely necessary: the posted login SGE
is ISER_RX_PAYLOAD_SIZE, so login_req_len cannot exceed
MAX_KEY_VALUE_PAIRS and size = min(login_req_len, MAX_KEY_VALUE_PAIRS)
is login_req_len; once dlength <= login_req_len the copy out stays
inside what was copied in.

I will send a v2 two-patch series - the isert_recv_done() check as 1/2
and this one as 2/2 with Suggested-by: Leon Romanovsky
<leonro@nvidia.com> - once the before/after on the login case is run.

Best regards,