summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYehyeong Lee <yhlee@isslab.korea.ac.kr>2026-07-27 01:39:31 +0900
committerLeon Romanovsky <leon@kernel.org>2026-07-29 09:07:06 -0400
commit2488b5b4827e5415768afc8daf097e8eb83c98df (patch)
treea5b4b3713d0716696a1b721fe20d18c6665e0213
parent957f92ea4022fb6af4618271615a2a21a7b5bef9 (diff)
downloadlinux-next-2488b5b4827e5415768afc8daf097e8eb83c98df.tar.gz
linux-next-2488b5b4827e5415768afc8daf097e8eb83c98df.zip
IB/isert: reject login PDUs declaring more data than was received
isert_login_recv_done() records how many bytes the HCA actually placed in the login buffer, but nothing compares that against the length the login PDU's BHS declares. isert_rx_login_req() copies min(login_req_len, MAX_KEY_VALUE_PAIRS) bytes into login->req_buf, and the login code then reads the declared length back out of that buffer - for the first PDU in iscsi_target_locate_portal(), payload_length = ntoh24(login_req->dlength); tmpbuf = kmemdup_nul(login->req_buf, payload_length, GFP_KERNEL); and for the ones after it in iscsi_decode_text_input(), reached from iscsi_target_do_login(). login->req_buf is a fixed MAX_KEY_VALUE_PAIRS (8192) byte allocation, so an initiator that declares more than it sends reads off the end of it, before authentication and with the length under its control: 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 which belongs to the cache kmalloc-8k of size 8192 allocated 8192-byte region Falsifying the second login PDU instead reaches the other reader, on the same buffer: BUG: KASAN: slab-out-of-bounds in kmemdup_nul+0x43/0x80 Read of size 8193 at addr ffff888104d10000 by task kworker/1:1/50 Workqueue: isert_login_wq iscsi_target_do_login_rx __asan_memcpy+0x23/0x60 kmemdup_nul+0x43/0x80 iscsi_decode_text_input+0xc6/0x11c0 iscsi_target_do_login+0x261/0x1470 iscsi_target_do_login_rx+0x51d/0x7d0 iscsit over TCP is not exposed: iscsit_get_login_rx() validates the declared length with iscsi_target_check_login_request() and then reads exactly that many bytes off the socket, so the declared length governs how much arrives rather than how much is copied out of an already-filled buffer. isert does not call iscsi_target_check_login_request() at all. Reject a login PDU whose declared DataSegmentLength exceeds what was received, in both paths that reach isert_rx_login_req(): isert_get_login_rx() for the first login PDU and isert_login_recv_done() for the ones after it. dlength <= login_req_len is allowed because the received count can include up to three bytes of iSCSI padding. Once the check is in place the copy out can no longer exceed the copy in: the posted login SGE is ISER_RX_PAYLOAD_SIZE, so login_req_len cannot exceed MAX_KEY_VALUE_PAIRS and the min() in isert_rx_login_req() is login_req_len. Like the existing short-PDU check added by 29e7b925ae6d, the reject in isert_login_recv_done() returns without completing login_req_comp, so a malformed subsequent PDU leaves the login to be torn down by the login timer rather than failing immediately. The first-PDU path returns an error and fails straight away. Reproduced on 7.2.0-rc4 with soft-RoCE (rdma_rxe) under KASAN, using an initiator that sends the real key=value payload while declaring 8193 in the BHS, on the first login PDU and on the second in separate runs. The reported read size tracks the declared value exactly; 16384 and 61440 behave the same. Unpatched 3 of 3 runs report on each of the two paths, patched 0 of 3 on both, run alternately in a single session, and a normal login still completes on the patched build. Fixes: b8d26b3be8b3 ("iser-target: Add iSCSI Extensions for RDMA (iSER) target driver") Suggested-by: Leon Romanovsky <leonro@nvidia.com> Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr> Link: https://patch.msgid.link/20260726163931.971063-3-yhlee@isslab.korea.ac.kr Signed-off-by: Leon Romanovsky <leon@kernel.org>
-rw-r--r--drivers/infiniband/ulp/isert/ib_isert.c25
1 files changed, 24 insertions, 1 deletions
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 9e050d034005..e57545cf337a 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -970,6 +970,21 @@ post_send:
return 0;
}
+static int
+isert_check_login_req(struct isert_conn *isert_conn)
+{
+ struct iscsi_hdr *hdr = isert_get_iscsi_hdr(isert_conn->login_desc);
+ u32 dlength = ntoh24(hdr->dlength);
+
+ if (unlikely(dlength > (u32)isert_conn->login_req_len)) {
+ isert_dbg("login PDU declares %u data bytes but only %d were received\n",
+ dlength, isert_conn->login_req_len);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static void
isert_rx_login_req(struct isert_conn *isert_conn)
{
@@ -1408,8 +1423,12 @@ 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);
@@ -2374,6 +2393,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);