⬢ DragonFlyBSD Kernel Audit
DF-2462 / fix.diff
← back to finding ↓ download raw
--- a/sys/dev/disk/iscsi/initiator/iscsivar.h
+++ b/sys/dev/disk/iscsi/initiator/iscsivar.h
@@ -137,6 +137,12 @@
      int		targetid;
 //     int		cid;		// Connection ID
 //     int		tsih;		// target session identifier handle
+     /*
+      | in-flight users of this session (ic_action, per-session sysctl
+      | handlers); guarded by isc_softc.lock.  ism_stop() drains this
+      | to zero before destroying the session.
+      */
+     int		refs;
      sn_t       	sn;             // sequence number stuff;
      int		cws;		// current window size
 
@@ -249,6 +255,12 @@
 int	ism_start(isc_session_t *sp);
 void	ism_stop(isc_session_t *sp);
 
+/* the driver softc (was static in iscsi.c) */
+extern struct isc_softc	isc;
+
+isc_session_t	*isc_sess_lookup(int sid);	/* takes a reference */
+void	isc_sess_rele(isc_session_t *sp);	/* drops a reference */
+
 int	scsi_encap(struct cam_sim *sim, union ccb *ccb);
 int	scsi_decap(isc_session_t *sp, pduq_t *opq, pduq_t *pq);
 void	iscsi_r2t(isc_session_t *sp, pduq_t *opq, pduq_t *pq);
--- a/sys/dev/disk/iscsi/initiator/iscsi.c
+++ b/sys/dev/disk/iscsi/initiator/iscsi.c
@@ -63,7 +63,7 @@
 
 static char *iscsi_driver_version = "2.1.0";
 
-static struct isc_softc isc;
+struct isc_softc	isc;
 
 MALLOC_DEFINE(M_ISCSI, "iSCSI", "iSCSI driver");
 
--- a/sys/dev/disk/iscsi/initiator/isc_sm.c
+++ b/sys/dev/disk/iscsi/initiator/isc_sm.c
@@ -626,6 +626,35 @@
 }
 #endif
 
+/*
+ | Look up a session by sid and take a reference, so the caller may
+ | use sp while ism_stop() runs concurrently: ism_stop() clears
+ | sc->sessions[sid] under sc->lock before tearing the session down,
+ | and drains sp->refs to zero before freeing it.
+ */
+isc_session_t *
+isc_sess_lookup(int sid)
+{
+     isc_session_t	*sp;
+
+     if(sid < 0 || sid >= MAX_SESSIONS)
+	  return NULL;
+     lockmgr(&isc.lock, LK_EXCLUSIVE);
+     if((sp = isc.sessions[sid]) != NULL)
+	  sp->refs++;
+     lockmgr(&isc.lock, LK_RELEASE);
+     return sp;
+}
+
+void
+isc_sess_rele(isc_session_t *sp)
+{
+     lockmgr(&sp->isc->lock, LK_EXCLUSIVE);
+     if(--sp->refs == 0)
+	  wakeup(sp);
+     lockmgr(&sp->isc->lock, LK_RELEASE);
+}
+
 static int
 isc_dump_stats(SYSCTL_HANDLER_ARGS)
 {
@@ -634,7 +663,13 @@
      char	buf[1024], *bp;
      int 	error, n;
 
-     sp = (isc_session_t *)arg1;
+     /*
+      | arg1 is the sid, NOT a raw session pointer: the sysctl OID can
+      | race (or outlive) the session, so resolve it under the lock.
+      */
+     sp = isc_sess_lookup((int)(intptr_t)arg1);
+     if(sp == NULL)
+	  return ENODEV;
      sc = sp->isc;
 
      bp = buf;
@@ -649,29 +684,40 @@
      ksnprintf(bp, n, " cws=%d cmd=%x exp=%x max=%x stat=%x itt=%x",
 		  sp->cws, sp->sn.cmd, sp->sn.expCmd, sp->sn.maxCmd, sp->sn.stat, sp->sn.itt);
      error = SYSCTL_OUT(req, buf, strlen(buf));
+     isc_sess_rele(sp);
      return error;
 }
 
 static int
 isc_sysctl_targetName(SYSCTL_HANDLER_ARGS)
 {
+     isc_session_t	*sp;
      char	buf[128], **cp;
      int 	error;
 
-     cp = (char **)arg1;
+     sp = isc_sess_lookup((int)(intptr_t)arg1);
+     if(sp == NULL)
+	  return ENODEV;
+     cp = (char **)&sp->opt.targetName;
      ksnprintf(buf, sizeof(buf), "%s", *cp);
      error = SYSCTL_OUT(req, buf, strlen(buf));
+     isc_sess_rele(sp);
      return error;
 }
 static int
 isc_sysctl_targetAddress(SYSCTL_HANDLER_ARGS)
 {
+     isc_session_t	*sp;
      char	buf[128], **cp;
      int 	error;
 
-     cp = (char **)arg1;
+     sp = isc_sess_lookup((int)(intptr_t)arg1);
+     if(sp == NULL)
+	  return ENODEV;
+     cp = (char **)&sp->opt.targetAddress;
      ksnprintf(buf, sizeof(buf), "%s", *cp);
      error = SYSCTL_OUT(req, buf, strlen(buf));
+     isc_sess_rele(sp);
      return error;
 }
 static void
@@ -693,7 +739,7 @@
 		     OID_AUTO,
 		     "targetname",
 		     CTLFLAG_RD,
-		     (void *)&sp->opt.targetName, 0,
+		     (void *)(intptr_t)sp->sid, 0,
 		     isc_sysctl_targetName, "A", "target name");
 
      SYSCTL_ADD_PROC(&sp->clist,
@@ -701,7 +747,7 @@
 		     OID_AUTO,
 		     "targeaddress",
 		     CTLFLAG_RD,
-		     (void *)&sp->opt.targetAddress, 0,
+		     (void *)(intptr_t)sp->sid, 0,
 		     isc_sysctl_targetAddress, "A", "target address");
 
      SYSCTL_ADD_PROC(&sp->clist,
@@ -709,7 +755,7 @@
 		     OID_AUTO,
 		     "stats",
 		     CTLFLAG_RD,
-		     (void *)sp, 0,
+		     (void *)(intptr_t)sp->sid, 0,
 		     isc_dump_stats, "A", "statistics");
 
      SYSCTL_ADD_INT(&sp->clist,
@@ -727,6 +773,16 @@
      cdev_t	dev;
 
      debug_called(8);
+     /*
+      | Detach from the lookup table FIRST, under sc->lock, so no new
+      | ic_action() or per-session sysctl reader can find the session
+      | while it is torn down (they look it up under the same lock and
+      | take a reference, or see NULL and fail cleanly).
+      */
+     lockmgr(&sc->lock, LK_EXCLUSIVE);
+     sc->sessions[sp->sid] = NULL;
+     lockmgr(&sc->lock, LK_RELEASE);
+
      sdebug(2, "terminating");
      /*
       | first stop the receiver
@@ -756,6 +812,18 @@
      sc->nsess--;
      lockmgr(&sc->lock, LK_RELEASE);
 
+     /*
+      | Wait for in-flight ic_action()/sysctl holders to drop their
+      | references before the session is destroyed.
+      */
+     lockmgr(&sc->lock, LK_EXCLUSIVE);
+     while(sp->refs != 0) {
+	  lockmgr(&sc->lock, LK_RELEASE);
+	  tsleep(sp, 0, "isc-drn", 1);
+	  lockmgr(&sc->lock, LK_EXCLUSIVE);
+     }
+     lockmgr(&sc->lock, LK_RELEASE);
+
      dev = sp->dev;
      sp->dev = NULL;
 
@@ -768,12 +836,16 @@
      mtx_uninit(&sp->snd_mtx);
      mtx_uninit(&sp->io_mtx);
 
-     i_freeopt(&sp->opt);
-     sc->sessions[sp->sid] = NULL;
-
+     /*
+      | Deregister the sysctl OIDs BEFORE freeing what they resolve
+      | through; with the drain above no handler can still be running
+      | and no new one can start.
+      */
      if(sysctl_ctx_free(&sp->clist))
 	  xdebug("sysctl_ctx_free failed");
 
+     i_freeopt(&sp->opt);
+
      kfree(sp, M_ISCSI);
 }
 
--- a/sys/dev/disk/iscsi/initiator/isc_cam.c
+++ b/sys/dev/disk/iscsi/initiator/isc_cam.c
@@ -207,8 +207,20 @@
 
      debug_called(8);
 
-     if((ccb_h->target_id != CAM_TARGET_WILDCARD) && (ccb_h->target_id < MAX_SESSIONS))
+     if((ccb_h->target_id != CAM_TARGET_WILDCARD) && (ccb_h->target_id < MAX_SESSIONS)) {
+	  /*
+	   | Take a reference under the lock so ism_stop() cannot free
+	   | the session while we use it; released before returning.
+	   | (The XPT_SCSI_IO queue path keeps the pre-existing pdu/CCB
+	   | lifetime, which ism_stop's xmitter stop + i_pdu_flush
+	   | already bound.)
+	   */
+	  lockmgr(&isp->lock, LK_EXCLUSIVE);
 	  sp = isp->sessions[ccb_h->target_id];
+	  if(sp != NULL)
+	       sp->refs++;
+	  lockmgr(&isp->lock, LK_RELEASE);
+     }
      else
 	  sp = NULL;
 
@@ -304,6 +316,8 @@
 	  ccb_h->status = CAM_REQ_INVALID;
 	  break;
      }
+     if(sp != NULL)
+	  isc_sess_rele(sp);
      xpt_done(ccb);
      return;
 }