zfcp: fix use-after-free in request timeout handlers

Message ID 20200813152856.50088-1-maier@linux.ibm.com
State New
Headers show
  • zfcp: fix use-after-free in request timeout handlers
Related show

Commit Message

Steffen Maier Aug. 13, 2020, 3:28 p.m.
Before v4.15 commit 75492a51568b ("s390/scsi: Convert timers to use
timer_setup()"), we intentionally only passed zfcp_adapter as context
argument to zfcp_fsf_request_timeout_handler(). Since we only trigger
adapter recovery, it was unnecessary to sync against races between timeout
and (late) completion.
Likewise, we only passed zfcp_erp_action as context argument to
zfcp_erp_timeout_handler(). Since we only wakeup an ERP action, it was
unnecessary to sync against races between timeout and (late) completion.

Meanwhile the timeout handlers get timer_list as context argument
and do a timer-specific container-of to zfcp_fsf_req which can have
been freed.

Fix it by making sure that any request timeout handlers, that might
just have started before del_timer(), are completed by using
del_timer_sync() instead. This ensures the request free happens

Space time diagram of potential use-after-free:

Basic idea is to have 2 or more pending requests whose timeouts run
out at almost the same time.

req 1 timeout     ERP thread        req 2 timeout
----------------  ----------------  ---------------------------------------
fsf_req = from_timer(fsf_req, t, timer)
adapter = fsf_req->adapter
                    zfcp_fsf_req_complete 1
                    del_timer 1
                    zfcp_fsf_req_free 1
                    zfcp_fsf_req_complete 2
                    del_timer 2
                                    fsf_req = from_timer(fsf_req, t, timer)
                    zfcp_fsf_req_free 2
                                    adapter = fsf_req->adapter
                                              ^^^^^^^ already freed

Suggested-by: Julian Wiedmann <jwi@linux.ibm.com>
Reviewed-by: Julian Wiedmann <jwi@linux.ibm.com>
Fixes: 75492a51568b ("s390/scsi: Convert timers to use timer_setup()")
Cc: <stable@vger.kernel.org> #4.15+
Signed-off-by: Steffen Maier <maier@linux.ibm.com>
 drivers/s390/scsi/zfcp_fsf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index c795f22249d8..140186fe1d1e 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -434,7 +434,7 @@  static void zfcp_fsf_req_complete(struct zfcp_fsf_req *req)
-	del_timer(&req->timer);
+	del_timer_sync(&req->timer);
@@ -867,7 +867,7 @@  static int zfcp_fsf_req_send(struct zfcp_fsf_req *req)
 	req->qdio_req.qdio_outb_usage = atomic_read(&qdio->req_q_free);
 	req->issued = get_tod_clock();
 	if (zfcp_qdio_send(qdio, &req->qdio_req)) {
-		del_timer(&req->timer);
+		del_timer_sync(&req->timer);
 		/* lookup request again, list might have changed */
 		zfcp_reqlist_find_rm(adapter->req_list, req_id);
 		zfcp_erp_adapter_reopen(adapter, 0, "fsrs__1");