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

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
<[email protected]> - once the before/after on the login case is run.

Best regards,