Author: trasz
Date: Thu Mar 15 17:36:13 2018
New Revision: 331013
URL: https://svnweb.freebsd.org/changeset/base/331013

Log:
  Fix iSCSI target crash on session reinstation.
  
  The crash scenario goes like this: there's a thread waiting on "reinstate";
  because it doesn't update the timeout counter it gets terminated by the
  callout; at this point the maintenance thread starts the termination routine.
  The first thread finishes waiting, proceeds to icl_conn_handoff(), and drops
  the refcount, which allows the maintenance thread to free its resources.  At
  this point another thread receives a PDU.  Boom.
  
  PR:           222898, 219866
  Reported by:  Eugene M. Zheganin <emz at norma.perm.ru>
  Tested by:    Eugene M. Zheganin <emz at norma.perm.ru>
  Reviewed by:  mav@ (earlier version)
  MFC after:    2 weeks
  Sponsored by: playkey.net

Modified:
  head/sys/cam/ctl/ctl_frontend_iscsi.c
  head/sys/cam/ctl/ctl_frontend_iscsi.h

Modified: head/sys/cam/ctl/ctl_frontend_iscsi.c
==============================================================================
--- head/sys/cam/ctl/ctl_frontend_iscsi.c       Thu Mar 15 17:15:40 2018        
(r331012)
+++ head/sys/cam/ctl/ctl_frontend_iscsi.c       Thu Mar 15 17:36:13 2018        
(r331013)
@@ -1164,11 +1164,11 @@ cfiscsi_maintenance_thread(void *arg)
 
        for (;;) {
                CFISCSI_SESSION_LOCK(cs);
-               if (cs->cs_terminating == false)
+               if (cs->cs_terminating == false || cs->cs_handoff_in_progress)
                        cv_wait(&cs->cs_maintenance_cv, &cs->cs_lock);
                CFISCSI_SESSION_UNLOCK(cs);
 
-               if (cs->cs_terminating) {
+               if (cs->cs_terminating && cs->cs_handoff_in_progress == false) {
 
                        /*
                         * We used to wait up to 30 seconds to deliver queued
@@ -1196,8 +1196,6 @@ static void
 cfiscsi_session_terminate(struct cfiscsi_session *cs)
 {
 
-       if (cs->cs_terminating)
-               return;
        cs->cs_terminating = true;
        cv_signal(&cs->cs_maintenance_cv);
 #ifdef ICL_KERNEL_PROXY
@@ -1268,6 +1266,13 @@ cfiscsi_session_new(struct cfiscsi_softc *softc, const
        cv_init(&cs->cs_login_cv, "cfiscsi_login");
 #endif
 
+       /*
+        * The purpose of this is to avoid racing with session shutdown.
+        * Otherwise we could have the maintenance thread call icl_conn_close()
+        * before we call icl_conn_handoff().
+        */
+       cs->cs_handoff_in_progress = true;
+
        cs->cs_conn = icl_new_conn(offload, false, "cfiscsi", &cs->cs_lock);
        if (cs->cs_conn == NULL) {
                free(cs, M_CFISCSI);
@@ -1378,8 +1383,18 @@ cfiscsi_accept(struct socket *so, struct sockaddr *sa,
        icl_conn_handoff_sock(cs->cs_conn, so);
        cs->cs_initiator_sa = sa;
        cs->cs_portal_id = portal_id;
+       cs->cs_handoff_in_progress = false;
        cs->cs_waiting_for_ctld = true;
        cv_signal(&cfiscsi_softc.accept_cv);
+
+       CFISCSI_SESSION_LOCK(cs);
+       /*
+        * Wake up the maintenance thread if we got scheduled for termination
+        * somewhere between cfiscsi_session_new() and icl_conn_handoff_sock().
+        */
+       if (cs->cs_terminating)
+               cfiscsi_session_terminate(cs);
+       CFISCSI_SESSION_UNLOCK(cs);
 }
 #endif
 
@@ -1559,6 +1574,7 @@ cfiscsi_ioctl_handoff(struct ctl_iscsi *ci)
        mtx_lock(&softc->lock);
        if (ct->ct_online == 0) {
                mtx_unlock(&softc->lock);
+               cs->cs_handoff_in_progress = false;
                cfiscsi_session_terminate(cs);
                cfiscsi_target_release(ct);
                ci->status = CTL_ISCSI_ERROR;
@@ -1569,7 +1585,6 @@ cfiscsi_ioctl_handoff(struct ctl_iscsi *ci)
        cs->cs_target = ct;
        mtx_unlock(&softc->lock);
 
-       refcount_acquire(&cs->cs_outstanding_ctl_pdus);
 restart:
        if (!cs->cs_terminating) {
                mtx_lock(&softc->lock);
@@ -1606,8 +1621,8 @@ restart:
 #endif
                error = icl_conn_handoff(cs->cs_conn, cihp->socket);
                if (error != 0) {
+                       cs->cs_handoff_in_progress = false;
                        cfiscsi_session_terminate(cs);
-                       refcount_release(&cs->cs_outstanding_ctl_pdus);
                        ci->status = CTL_ISCSI_ERROR;
                        snprintf(ci->error_str, sizeof(ci->error_str),
                            "%s: icl_conn_handoff failed with error %d",
@@ -1632,7 +1647,16 @@ restart:
        }
 #endif
 
-       refcount_release(&cs->cs_outstanding_ctl_pdus);
+       CFISCSI_SESSION_LOCK(cs);
+       cs->cs_handoff_in_progress = false;
+
+       /*
+        * Wake up the maintenance thread if we got scheduled for termination.
+        */
+       if (cs->cs_terminating)
+               cfiscsi_session_terminate(cs);
+       CFISCSI_SESSION_UNLOCK(cs);
+
        ci->status = CTL_ISCSI_OK;
 }
 

Modified: head/sys/cam/ctl/ctl_frontend_iscsi.h
==============================================================================
--- head/sys/cam/ctl/ctl_frontend_iscsi.h       Thu Mar 15 17:15:40 2018        
(r331012)
+++ head/sys/cam/ctl/ctl_frontend_iscsi.h       Thu Mar 15 17:36:13 2018        
(r331013)
@@ -85,6 +85,7 @@ struct cfiscsi_session {
        int                             cs_timeout;
        struct cv                       cs_maintenance_cv;
        bool                            cs_terminating;
+       bool                            cs_handoff_in_progress;
        bool                            cs_tasks_aborted;
        int                             cs_max_recv_data_segment_length;
        int                             cs_max_send_data_segment_length;
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to