zfcp fixes for scsi-misc
Subsystems (3)
cover zfcp fixes for scsi-misc
James,
here are some zfcp fixes. They apply on the current version of scsi-misc.
Regards,
Christof Schmitt
PATCH 1/6 zfcp: fix use after free bug.
From: Heiko Carstens <[email protected]>
zfcp_erp_strategy_check_fsfreq() checks if it is safe to access the fsf_req associated with the erp_action that gets passed. To test if it is safe it accesses the fsf_req in order to get its index into the hash list. This is broken since the fsf_req might be freed already and the read index has no meaning. It could lead to memory corruption. Fix this by introducing a new zfcp_reqlist_find_safe() method which just checks if addresses are equal. This is slower, but only gets called in case of error recovery.
---
drivers/s390/scsi/zfcp_def.h | 14 ++++++++++++++ drivers/s390/scsi/zfcp_erp.c | 3 ++- 2 files changed, 16 insertions(+), 1 deletion(-)
drivers/s390/scsi/zfcp_def.h +14 -0
@@ -1123,6 +1123,20 @@ zfcp_reqlist_find(struct zfcp_adapter *areturn NULL ; } +static inline struct zfcp_fsf_req * +zfcp_reqlist_find_safe (struct zfcp_adapter *adapter,struct zfcp_fsf_req *req) +{ +struct zfcp_fsf_req *request; +unsigned int idx; + +for (idx =0 ; idx <REQUEST_LIST_SIZE ; idx++) { +list_for_each_entry (request, &adapter->req_list[idx], list) +if (request == req) +return request; + } +return NULL ; +} +/* * functions needed for reference/usage counting */
drivers/s390/scsi/zfcp_erp.c +2 -1
@@ -846,7 +846,8 @@ zfcp_erp_strategy_check_fsfreq(struct zfif (erp_action->fsf_req) {/* take lock to ensure that request is not deleted meanwhile */ spin_lock (&adapter->req_list_lock); -if (zfcp_reqlist_find (adapter, erp_action->fsf_req->req_id)) { +if (zfcp_reqlist_find_safe (adapter, erp_action->fsf_req) && + erp_action->fsf_req->erp_action == erp_action) {/* fsf_req still exists */ debug_text_event (adapter->erp_dbf,3 ,"a_ca_req" ); debug_event(adapter->erp_dbf, 3, &erp_action->fsf_req,
PATCH 2/6 zfcp: Fix evaluation of port handles in abort handler
In reply to Christof SchmittFrom: Christof Schmitt <[email protected]>
According to the FSF spec, word 0 (bytes 0-3) has the handle specified with the abort command and word 1 (bytes 4-7) has the handle for the command to be aborted. Fix the if statements that try to compare those.
---
drivers/s390/scsi/zfcp_fsf.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/s390/scsi/zfcp_fsf.c +4 -5
@@ -1164,8 +1164,8 @@ zfcp_fsf_abort_fcp_command_handler(struc {int retval = -EINVAL ;struct zfcp_unit *unit; -unsigned char status_qual = - new_fsf_req->qtcb->header.fsf_status_qual.word[0 ]; +union fsf_status_qual *fsf_stat_qual = + &new_fsf_req->qtcb->header.fsf_status_qual;if (new_fsf_req->status &ZFCP_STATUS_FSFREQ_ERROR ) {/* do not set ZFCP_STATUS_FSFREQ_ABORTSUCCEEDED */
@@ -1178,7 +1178,7 @@ zfcp_fsf_abort_fcp_command_handler(strucswitch (new_fsf_req->qtcb->header.fsf_status) {case FSF_PORT_HANDLE_NOT_VALID : -if (status_qual >>4 != status_qual %0xf ) { +if (fsf_stat_qual->word[0 ] != fsf_stat_qual->word[1 ]) {debug_text_event (new_fsf_req->adapter->erp_dbf,3 ,"fsf_s_phand_nv0" );/*
@@ -1207,8 +1207,7 @@ zfcp_fsf_abort_fcp_command_handler(strucbreak ;case FSF_LUN_HANDLE_NOT_VALID : -if (status_qual >>4 != status_qual %0xf ) { -/* 2 */ +if (fsf_stat_qual->word[0 ] != fsf_stat_qual->word[1 ]) {debug_text_event (new_fsf_req->adapter->erp_dbf,3 ,"fsf_s_lhand_nv0" );/*
PATCH 3/6 zfcp: Hold queue lock when checking port/unit handle for abort command
In reply to Christof SchmittFrom: Christof Schmitt <[email protected]>
We need to hold the queue-lock when checking whether we still have a valid unit/port handle for the abort command, i.e whether we can issue this request for this unit/port. If the error recovery is about to close this unit/port, then it competes for the queue-lock. If the close request issued by the error recovery wins, then it is guaranteed that this unit/port has been blocked for other requests.
---
drivers/s390/scsi/zfcp_fsf.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-)
drivers/s390/scsi/zfcp_fsf.c +8 -13
@@ -1116,6 +1116,10 @@ zfcp_fsf_abort_fcp_command(unsigned long goto out; } +if (unlikely (!atomic_test_mask (ZFCP_STATUS_COMMON_UNBLOCKED , + &unit->status))) + goto unit_blocked; + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 ); sbale[0 ].flags |=SBAL_FLAGS0_TYPE_READ ; sbale[1 ].flags |=SBAL_FLAGS_LAST_ENTRY ;
@@ -1131,22 +1135,13 @@ zfcp_fsf_abort_fcp_command(unsigned longzfcp_fsf_start_timer (fsf_req,ZFCP_SCSI_ER_TIMEOUT ); retval =zfcp_fsf_req_send (fsf_req); -if (retval) { -ZFCP_LOG_INFO ("error: Failed to send abort command request " -"on adapter %s, port 0x%016Lx, unit 0x%016Lx\n" , -zfcp_get_busid_by_adapter (adapter), - unit->port->wwpn, unit->fcp_lun); +if (!retval) + goto out; + + unit_blocked:zfcp_fsf_req_free (fsf_req); fsf_req =NULL ; - goto out; - } -ZFCP_LOG_DEBUG ("Abort FCP Command request initiated " -"(adapter%s, port d_id=0x%06x, " -"unit x%016Lx, old_req_id=0x%lx)\n" , -zfcp_get_busid_by_adapter (adapter), - unit->port->d_id, - unit->fcp_lun, old_req_id); out:write_unlock_irqrestore (&adapter->request_queue.queue_lock, lock_flags);return fsf_req;
PATCH 4/6 zfcp: Hold queue lock when checking port handle for ELS command
In reply to Christof SchmittFrom: Christof Schmitt <[email protected]>
We need to hold the queue-lock when checking whether we still have a valid port handle for the ELS command, i.e whether we can issue this request for this port. If the error recovery is about to close this port, then it competes for the queue-lock. If the close request issued by the error recovery wins, then it is guaranteed that this port has been blocked for other requests.
---
drivers/s390/scsi/zfcp_erp.c | 2 +- drivers/s390/scsi/zfcp_fsf.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-)
drivers/s390/scsi/zfcp_erp.c +1 -1
@@ -456,7 +456,7 @@ zfcp_test_link(struct zfcp_port *port)zfcp_port_get (port); retval =zfcp_erp_adisc (port); -if (retval !=0 ) { +if (retval !=0 && retval != -EBUSY ) {zfcp_port_put (port); ZFCP_LOG_NORMAL("reopen needed for port 0x%016Lx " "on adapter %s\n ", port->wwpn,
drivers/s390/scsi/zfcp_fsf.c +7 -0
@@ -1668,6 +1668,12 @@ zfcp_fsf_send_els(struct zfcp_send_els * goto failed_req; } +if (unlikely (!atomic_test_mask (ZFCP_STATUS_COMMON_UNBLOCKED , + &els->port->status))) { + ret = -EBUSY ; + goto port_blocked; + } + sbale =zfcp_qdio_sbale_req (fsf_req, fsf_req->sbal_curr,0 );if (zfcp_use_one_sbal (els->req, els->req_count, els->resp, els->resp_count)){
@@ -1749,6 +1755,7 @@ zfcp_fsf_send_els(struct zfcp_send_els * "0x%06x)\n", zfcp_get_busid_by_adapter(adapter), d_id); goto out; + port_blocked: failed_send:zfcp_fsf_req_free (fsf_req);
PATCH 5/6 zfcp: Hold queue lock when checking port/unit handle for FCP command
In reply to Christof SchmittFrom: Christof Schmitt <[email protected]>
We need to hold the queue-lock when checking whether we still have a valid unit/port handle for the FCP command, i.e whether we can issue this request for this unit/port. If the error recovery is about to close this unit/port, then it competes for the queue-lock. If the close request issued by the error recovery wins, then it is guaranteed that this unit/port has been blocked for other requests.
---
drivers/s390/scsi/zfcp_fsf.c | 7 +++++++ drivers/s390/scsi/zfcp_scsi.c | 8 +++----- 2 files changed, 10 insertions(+), 5 deletions(-)
drivers/s390/scsi/zfcp_fsf.c +7 -0
@@ -3593,6 +3593,12 @@ zfcp_fsf_send_fcp_command_task(struct zf goto failed_req_create; } +if (unlikely (!atomic_test_mask (ZFCP_STATUS_COMMON_UNBLOCKED , + &unit->status))) { + retval = -EBUSY ; + goto unit_blocked; + } +zfcp_unit_get (unit); fsf_req->unit = unit;
@@ -3733,6 +3739,7 @@ zfcp_fsf_send_fcp_command_task(struct zf send_failed: no_fit: failed_scsi_cmnd: + unit_blocked:zfcp_unit_put (unit);zfcp_fsf_req_free (fsf_req); fsf_req =NULL ;
drivers/s390/scsi/zfcp_scsi.c +3 -5
@@ -258,8 +258,9 @@ zfcp_scsi_command_async(struct zfcp_adap goto out; } -if (unlikely ( - !atomic_test_mask (ZFCP_STATUS_COMMON_UNBLOCKED , &unit->status))) { + tmp =zfcp_fsf_send_fcp_command_task (adapter, unit, scpnt, use_timer, +ZFCP_REQ_AUTO_CLEANUP ); +if (unlikely (tmp == -EBUSY )) { ZFCP_LOG_DEBUG("adapter %s not ready or unit 0x%016Lx " "on port 0x%016Lx in recovery\n", zfcp_get_busid_by_unit(unit),
@@ -268,9 +269,6 @@ zfcp_scsi_command_async(struct zfcp_adap goto out; } - tmp =zfcp_fsf_send_fcp_command_task (adapter, unit, scpnt, use_timer, -ZFCP_REQ_AUTO_CLEANUP ); -if (unlikely (tmp <0 )) {ZFCP_LOG_DEBUG ("error: initiation of Send FCP Cmnd failed\n" ); retval =SCSI_MLQUEUE_HOST_BUSY ;
PATCH 6/6 zfcp: Hold queue lock when checking port/unit handle for task management cmd
In reply to Christof SchmittFrom: Christof Schmitt <[email protected]>
We need to hold the queue-lock when checking whether we still have a valid unit/port handle for the task management command, i.e whether we can issue this request for this unit/port. If the error recovery is about to close this unit/port, then it competes for the queue-lock. If the close request issued by the error recovery wins, then it is guaranteed that this unit/port has been blocked for other requests.
---
drivers/s390/scsi/zfcp_fsf.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-)
drivers/s390/scsi/zfcp_fsf.c +9 -17
@@ -3774,6 +3774,10 @@ zfcp_fsf_send_fcp_command_task_managemen goto out; } +if (unlikely (!atomic_test_mask (ZFCP_STATUS_COMMON_UNBLOCKED , + &unit->status))) + goto unit_blocked; +/* * Used to decide on proper handler in the return path, * could be either zfcp_fsf_send_fcp_command_task_handler or
@@ -3807,25 +3811,13 @@ zfcp_fsf_send_fcp_command_task_managemenzfcp_fsf_start_timer (fsf_req,ZFCP_SCSI_ER_TIMEOUT ); retval =zfcp_fsf_req_send (fsf_req); -if (retval) { -ZFCP_LOG_INFO ("error: Could not send an FCP-command (task " -"management) on adapter %s, port 0x%016Lx for " -"unit LUN 0x%016Lx\n" , -zfcp_get_busid_by_adapter (adapter), - unit->port->wwpn, - unit->fcp_lun); -zfcp_fsf_req_free (fsf_req); - fsf_req =NULL ; +if (!retval) goto out; - } -ZFCP_LOG_TRACE ("Send FCP Command (task management function) initiated " -"(adapter %s, port 0x%016Lx, unit 0x%016Lx, " -"tm_flags=0x%x)\n" , -zfcp_get_busid_by_adapter (adapter), - unit->port->wwpn, - unit->fcp_lun, - tm_flags); + unit_blocked: +zfcp_fsf_req_free (fsf_req); + fsf_req =NULL ; + out:write_unlock_irqrestore (&adapter->request_queue.queue_lock, lock_flags);return fsf_req;