From 462c12d9e9529e3cc275f07903fa10aaf4cbbd6d Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Wed, 22 May 2019 16:50:24 -0700 Subject: [PATCH] iscsi: free pdus in hot remove path with no lun The datain handling code path will set the LUN to NULL if it finds a task's LUN has been hotremoved. This could happen before the iscsi hotplug routine actually gets a chance to run. If this happens, one of these tasks doesn't actually get freed, and then will be freed after the lun is closed - causing a segfault in the bdev layer since it may have a bdev_io associated with it. Found by running the iscsi_tgt/fio test after applying the next patch in this series. There's more work needed in this hot remove clean up path - currently we are just freeing a lot of PDUs rather than completing them with error status when a LUN is hot removed. But let's tackle that separately. Signed-off-by: Jim Harris Change-Id: I8d27f0c7a79ae91cb6504e5ff6ffc8e346c9e54c Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/455460 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Shuhei Matsumoto Reviewed-by: Changpeng Liu --- lib/iscsi/conn.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/iscsi/conn.c b/lib/iscsi/conn.c index c2ecd121f3..ae27c159c7 100644 --- a/lib/iscsi/conn.c +++ b/lib/iscsi/conn.c @@ -624,7 +624,13 @@ _iscsi_conn_remove_lun(void *arg1, void *arg2) spdk_clear_all_transfer_task(conn, lun, NULL); TAILQ_FOREACH_SAFE(pdu, &conn->write_pdu_list, tailq, tmp_pdu) { - if (pdu->task && (lun == pdu->task->scsi.lun)) { + /* If the pdu's LUN matches the LUN that was removed, free this + * PDU immediately. If the pdu's LUN is NULL, then we know + * the datain handling code already detected the hot removal, + * so we can free that PDU as well. + */ + if (pdu->task && + (lun == pdu->task->scsi.lun || NULL == pdu->task->scsi.lun)) { TAILQ_REMOVE(&conn->write_pdu_list, pdu, tailq); spdk_iscsi_conn_free_pdu(conn, pdu); }