From ed90d56579c84e1e8e065f9639ac2775925708f9 Mon Sep 17 00:00:00 2001 From: Hou Pu Date: Fri, 6 Nov 2020 17:52:53 +0800 Subject: [PATCH 1/2] iser: fix segmentation fault when async message pdu is received The target sometimes sends a logout request to libiscsi in case it is going down or for some other reason. The opcode of such a request is ISCSI_PDU_ASYNC_MSG. On receiving these kinds of PDU, there is no related pdu on the list of iscsi->waitpdu. Just skip finding them from iscsi->waitpdu. Or segment fault might happen. Also rename nop_target label to no_waitpdu to be more clear. Signed-off-by: Hou Pu --- lib/iser.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/iser.c b/lib/iser.c index a927338..6bf0ce8 100644 --- a/lib/iser.c +++ b/lib/iser.c @@ -1322,7 +1322,10 @@ iser_rcv_completion(struct iser_rx_desc *rx_desc, uint32_t itt = scsi_get_uint32(&in.hdr[16]); if (opcode == ISCSI_PDU_NOP_IN && itt == 0xffffffff) - goto nop_target; + goto no_waitpdu; + + if (opcode == ISCSI_PDU_ASYNC_MSG) + goto no_waitpdu; struct iscsi_pdu *iscsi_pdu; struct iser_pdu *iser_pdu; @@ -1355,7 +1358,7 @@ iser_rcv_completion(struct iser_rx_desc *rx_desc, } } -nop_target: +no_waitpdu: /* decrementing conn->post_recv_buf_count only --after-- freeing the * * task eliminates the need to worry on tasks which are completed in * * parallel to the execution of iser_conn_term. So the code that waits * From 03fa3f627ca7f39d68aca9ecec70c6c914b42b8c Mon Sep 17 00:00:00 2001 From: Hou Pu Date: Fri, 6 Nov 2020 19:47:00 +0800 Subject: [PATCH 2/2] iser: fix segmentation fault when task management pdu is received As iser_pdu->desc->data_dir is not initialised when sending a PDU. The value remains what it was when it was used last time. Thus a PDU could be considered to have data if it previously had and might cause segmentation fault. For example if a pdu is a reset task management task with no data to transfer and the pdu is previously used as a read task. Thus it would cause fault like below: > struct scsi_iovector *iovector_in = &task->iovector_in; 0 0x00007ffff7bcb2d1 in iser_rcv_completion (rx_desc=0x555555b79e48, iser_conn=0x555555b573a0) at iser.c:1349 1 0x00007ffff7bcb53e in iser_handle_wc (wc=0x7fffffffdc00, iser_conn=0x555555b573a0) at iser.c:1426 2 0x00007ffff7bcb685 in cq_event_handler (iser_conn=0x555555b573a0) at iser.c:1468 3 0x00007ffff7bcb81b in cq_handle (iser_conn=0x555555b573a0) at iser.c:1516 4 0x00007ffff7bc8b28 in iscsi_iser_service (iscsi=0x555555b58710, revents=1) at iser.c:118 5 0x00007ffff7bc3862 in iscsi_service (iscsi=0x555555b58710, revents=1) at socket.c:1016 6 0x00007ffff7bc3f6c in event_loop (iscsi=0x555555b58710, state=0x7fffffffe000) at sync.c:71 7 0x00007ffff7bc4605 in iscsi_task_mgmt_sync (iscsi=0x555555b58710, lun=0, function=ISCSI_TM_LUN_RESET, ritt=4294967295, rcmdsn=0) at sync.c:281 8 0x00007ffff7bc46cf in iscsi_task_mgmt_lun_reset_sync (iscsi=0x555555b58710, lun=0) at sync.c:312 9 0x000055555555500d in iscsi_lun_reset_sync (iscsi=0x555555b58710) at iscsiclient_lun_reset.c:34 10 0x0000555555555680 in main (argc=7, argv=0x7fffffffe1c8) at iscsiclient_lun_reset.c:211 Signed-off-by: Hou Pu --- include/iser-private.h | 3 ++- lib/iser.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/iser-private.h b/include/iser-private.h index e167e06..bf30d8a 100644 --- a/include/iser-private.h +++ b/include/iser-private.h @@ -72,7 +72,8 @@ enum desc_type { enum data_dir{ DATA_WRITE = 0, - DATA_READ}; + DATA_READ, + DATA_NONE}; #define SHIFT_4K 12 #define SIZE_4K (1ULL << SHIFT_4K) diff --git a/lib/iser.c b/lib/iser.c index 6bf0ce8..ed3b84f 100644 --- a/lib/iser.c +++ b/lib/iser.c @@ -908,7 +908,8 @@ iser_send_command(struct iser_conn *iser_conn, struct iser_pdu *iser_pdu) iscsi_set_error(iscsi, "error in prepare write cmd\n"); return -1; } - } + } else + iser_pdu->desc->data_dir = DATA_NONE; err = iser_post_send(iser_conn, tx_desc, true); if (err)