From 70e1955487f1ae0d4648fe9bc388568163745c44 Mon Sep 17 00:00:00 2001 From: Lei Xue Date: Mon, 16 Mar 2026 17:22:01 +0800 Subject: [PATCH 1/5] fix: remove dead sessions from target session list When an iSCSI connection closes or a logout occurs, the associated session was never removed from the target's Sessions map, and the TSIH was never released back to the bitmap allocator. This caused session and TSIH leaks over repeated connect/disconnect cycles. Changes: - Add removeConnectionFromSession() to properly clean up session when its last connection is closed - Call session cleanup from handler() on CONN_STATE_CLOSE - Convert iscsiExecLogout to a method and add session cleanup on logout - Release TSIH in UnBindISCSISession to prevent TSIH bitmap exhaustion Fixes #42 Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/port/iscsit/iscsid.go | 9 ++++++--- pkg/port/iscsit/session.go | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/pkg/port/iscsit/iscsid.go b/pkg/port/iscsit/iscsid.go index fb35a89..b0375c6 100644 --- a/pkg/port/iscsit/iscsid.go +++ b/pkg/port/iscsit/iscsid.go @@ -357,6 +357,7 @@ func (s *ISCSITargetDriver) handler(events byte, conn *iscsiConnection) { s.rxHandler(conn) if conn.state == CONN_STATE_CLOSE { log.Warningf("iscsi connection[%d] closed", conn.cid) + s.removeConnectionFromSession(conn) conn.close() IPMutex.Lock() remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0] @@ -373,6 +374,7 @@ func (s *ISCSITargetDriver) handler(events byte, conn *iscsiConnection) { } if conn.state == CONN_STATE_CLOSE { log.Warningf("iscsi connection[%d] closed", conn.cid) + s.removeConnectionFromSession(conn) conn.close() IPMutex.Lock() remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0] @@ -491,7 +493,7 @@ func (s *ISCSITargetDriver) rxHandler(conn *iscsiConnection) { case OpLogoutReq: log.Debug("OpLogoutReq") s.setClientStatus(false) - if err := iscsiExecLogout(conn); err != nil { + if err := s.iscsiExecLogout(conn); err != nil { log.Warningf("set connection to close") conn.state = CONN_STATE_CLOSE } @@ -559,7 +561,7 @@ func (s *ISCSITargetDriver) iscsiExecLogin(conn *iscsiConnection) error { return conn.buildRespPackage(OpLoginResp, nil) } -func iscsiExecLogout(conn *iscsiConnection) error { +func (s *ISCSITargetDriver) iscsiExecLogout(conn *iscsiConnection) error { log.Infof("Logout request received from initiator: %v", conn.conn.RemoteAddr().String()) cmd := conn.req conn.resp = &ISCSICommand{ @@ -573,6 +575,7 @@ func iscsiExecLogout(conn *iscsiConnection) error { } else { conn.resp.ExpCmdSN = conn.session.ExpCmdSN conn.resp.MaxCmdSN = conn.session.ExpCmdSN + conn.session.MaxQueueCommand + s.removeConnectionFromSession(conn) } IPMutex.Lock() remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0] @@ -1018,7 +1021,7 @@ func (s *ISCSITargetDriver) scsiCommandHandler(conn *iscsiConnection) (err error s.setClientStatus(false) conn.txTask = &iscsiTask{conn: conn, cmd: conn.req, tag: conn.req.TaskTag} conn.txIOState = IOSTATE_TX_BHS - iscsiExecLogout(conn) + s.iscsiExecLogout(conn) case OpTextReq: err = fmt.Errorf("Cannot handle yet %s", opCodeMap[conn.req.OpCode]) log.Error(err) diff --git a/pkg/port/iscsit/session.go b/pkg/port/iscsit/session.go index 386593d..faf49ef 100644 --- a/pkg/port/iscsit/session.go +++ b/pkg/port/iscsit/session.go @@ -334,6 +334,27 @@ func (s *ISCSITargetDriver) UnBindISCSISession(sess *ISCSISession) { defer target.SessionsRWMutex.Unlock() delete(target.Sessions, sess.TSIH) scsi.RemoveITNexus(sess.Target.SCSITarget, sess.ITNexus) + s.ReleaseTSIH(sess.TSIH) + log.Infof("session %x unbound from target %s", sess.TSIH, target.SCSITarget.Name) +} + +// removeConnectionFromSession removes a connection from its session. +// If the session has no remaining connections, the session is unbound. +func (s *ISCSITargetDriver) removeConnectionFromSession(conn *iscsiConnection) { + sess := conn.session + if sess == nil { + return + } + + sess.ConnectionsRWMutex.Lock() + delete(sess.Connections, conn.cid) + remaining := len(sess.Connections) + sess.ConnectionsRWMutex.Unlock() + + if remaining == 0 { + s.UnBindISCSISession(sess) + } + conn.session = nil } func (s *ISCSITargetDriver) BindISCSISession(conn *iscsiConnection) error { From b7e5c4a7d22ffb2156cf2ba37775c563c574c1ee Mon Sep 17 00:00:00 2001 From: Lei Xue Date: Tue, 17 Mar 2026 14:11:27 +0800 Subject: [PATCH 2/5] fix: defer session cleanup to connection close, not logout Move session cleanup out of iscsiExecLogout() and keep it only in the CONN_STATE_CLOSE handler. The logout response must be fully sent before the session is removed; cleaning up during logout causes the daemon to hang because subsequent operations reference a nil session. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/port/iscsit/iscsid.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/port/iscsit/iscsid.go b/pkg/port/iscsit/iscsid.go index b0375c6..850a885 100644 --- a/pkg/port/iscsit/iscsid.go +++ b/pkg/port/iscsit/iscsid.go @@ -575,8 +575,9 @@ func (s *ISCSITargetDriver) iscsiExecLogout(conn *iscsiConnection) error { } else { conn.resp.ExpCmdSN = conn.session.ExpCmdSN conn.resp.MaxCmdSN = conn.session.ExpCmdSN + conn.session.MaxQueueCommand - s.removeConnectionFromSession(conn) } + // Session cleanup is deferred to CONN_STATE_CLOSE in handler(), + // because the logout response must be sent before the session is removed. IPMutex.Lock() remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0] if CurrentHostIP == remoteIP { From 8ffe5ec5ce257bd3ade7e77f9e7d1eb3722df836 Mon Sep 17 00:00:00 2001 From: Lei Xue Date: Tue, 17 Mar 2026 14:37:16 +0800 Subject: [PATCH 3/5] fix: prevent nil deref on session reinstatement during cleanup race When a new login arrives while the previous connection's async cleanup is still running, LookupISCSISession finds the stale session but LookupConnection returns nil (old connection already closed). This caused a nil pointer dereference when accessing existConn.session during session reinstatement. Fix by checking existConn != nil before reinstatement. If the old connection is already gone, unbind the stale session and create a fresh one instead. Also add sync.Once to removeConnectionFromSession to prevent concurrent goroutine and main-path cleanup from racing. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/port/iscsit/conn.go | 3 ++- pkg/port/iscsit/session.go | 35 ++++++++++++++++++++++------------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/pkg/port/iscsit/conn.go b/pkg/port/iscsit/conn.go index 19fb165..be6f6a0 100644 --- a/pkg/port/iscsit/conn.go +++ b/pkg/port/iscsit/conn.go @@ -88,7 +88,8 @@ type iscsiConnection struct { rxTask *iscsiTask txTask *iscsiTask - readLock *sync.RWMutex + readLock *sync.RWMutex + cleanupOnce sync.Once } type taskState int diff --git a/pkg/port/iscsit/session.go b/pkg/port/iscsit/session.go index faf49ef..d3bb8ea 100644 --- a/pkg/port/iscsit/session.go +++ b/pkg/port/iscsit/session.go @@ -340,21 +340,24 @@ func (s *ISCSITargetDriver) UnBindISCSISession(sess *ISCSISession) { // removeConnectionFromSession removes a connection from its session. // If the session has no remaining connections, the session is unbound. +// This is safe to call concurrently; cleanup runs at most once per connection. func (s *ISCSITargetDriver) removeConnectionFromSession(conn *iscsiConnection) { - sess := conn.session - if sess == nil { - return - } + conn.cleanupOnce.Do(func() { + sess := conn.session + if sess == nil { + return + } - sess.ConnectionsRWMutex.Lock() - delete(sess.Connections, conn.cid) - remaining := len(sess.Connections) - sess.ConnectionsRWMutex.Unlock() + sess.ConnectionsRWMutex.Lock() + delete(sess.Connections, conn.cid) + remaining := len(sess.Connections) + sess.ConnectionsRWMutex.Unlock() - if remaining == 0 { - s.UnBindISCSISession(sess) - } - conn.session = nil + if remaining == 0 { + s.UnBindISCSISession(sess) + } + conn.session = nil + }) } func (s *ISCSITargetDriver) BindISCSISession(conn *iscsiConnection) error { @@ -433,7 +436,13 @@ func (s *ISCSITargetDriver) BindISCSISession(conn *iscsiConnection) error { if conn.loginParam.tsih == ISCSI_UNSPEC_TSIH { log.Infof("Session Reinstatement initiator name:%v,target name:%v,ISID:0x%x", conn.loginParam.initiator, conn.loginParam.target, conn.loginParam.isid) - newSess, err = s.ReInstatement(existConn.session, conn) + if existConn != nil { + newSess, err = s.ReInstatement(existConn.session, conn) + } else { + // Old connection already closed; unbind the stale session and create new + s.UnBindISCSISession(existSess) + newSess, err = s.NewISCSISession(conn) + } if err != nil { return err } From be3cad5aba3b7163e53d61c6282bb7248b301560 Mon Sep 17 00:00:00 2001 From: Lei Xue Date: Tue, 17 Mar 2026 14:54:08 +0800 Subject: [PATCH 4/5] fix: extract clearHostIP helper and restore IP cleanup on close Restore CurrentHostIP cleanup on connection close (needed for blockMultipleHostLogin feature). Extract to clearHostIP helper to reduce duplication. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/port/iscsit/iscsid.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/pkg/port/iscsit/iscsid.go b/pkg/port/iscsit/iscsid.go index 850a885..c65e444 100644 --- a/pkg/port/iscsit/iscsid.go +++ b/pkg/port/iscsit/iscsid.go @@ -359,12 +359,7 @@ func (s *ISCSITargetDriver) handler(events byte, conn *iscsiConnection) { log.Warningf("iscsi connection[%d] closed", conn.cid) s.removeConnectionFromSession(conn) conn.close() - IPMutex.Lock() - remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0] - if CurrentHostIP == remoteIP { - CurrentHostIP = "" - } - IPMutex.Unlock() + s.clearHostIP(conn) } }() } @@ -376,15 +371,19 @@ func (s *ISCSITargetDriver) handler(events byte, conn *iscsiConnection) { log.Warningf("iscsi connection[%d] closed", conn.cid) s.removeConnectionFromSession(conn) conn.close() - IPMutex.Lock() - remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0] - if CurrentHostIP == remoteIP { - CurrentHostIP = "" - } - IPMutex.Unlock() + s.clearHostIP(conn) } } +func (s *ISCSITargetDriver) clearHostIP(conn *iscsiConnection) { + IPMutex.Lock() + remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0] + if CurrentHostIP == remoteIP { + CurrentHostIP = "" + } + IPMutex.Unlock() +} + func (s *ISCSITargetDriver) rxHandler(conn *iscsiConnection) { var ( hdigest uint = 0 From a7b58b8eb3f07b8d0a0612a53839fbc3b3a0a82a Mon Sep 17 00:00:00 2001 From: Lei Xue Date: Tue, 17 Mar 2026 15:01:22 +0800 Subject: [PATCH 5/5] fix: handle discovery sessions in UnBindISCSISession Discovery sessions have nil Target and nil ITNexus. The cleanup path crashed with nil pointer dereference when trying to remove ITNexus or access Target.Sessions for discovery sessions. Guard against nil Target (just release TSIH) and nil ITNexus. Also add nil safety to clearHostIP helper. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/port/iscsit/iscsid.go | 11 +++++++++-- pkg/port/iscsit/session.go | 9 ++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/port/iscsit/iscsid.go b/pkg/port/iscsit/iscsid.go index c65e444..2022674 100644 --- a/pkg/port/iscsit/iscsid.go +++ b/pkg/port/iscsit/iscsid.go @@ -376,12 +376,19 @@ func (s *ISCSITargetDriver) handler(events byte, conn *iscsiConnection) { } func (s *ISCSITargetDriver) clearHostIP(conn *iscsiConnection) { + if conn.conn == nil { + return + } IPMutex.Lock() - remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0] + defer IPMutex.Unlock() + addr := conn.conn.RemoteAddr() + if addr == nil { + return + } + remoteIP := strings.Split(addr.String(), ":")[0] if CurrentHostIP == remoteIP { CurrentHostIP = "" } - IPMutex.Unlock() } func (s *ISCSITargetDriver) rxHandler(conn *iscsiConnection) { diff --git a/pkg/port/iscsit/session.go b/pkg/port/iscsit/session.go index d3bb8ea..b6c8aa3 100644 --- a/pkg/port/iscsit/session.go +++ b/pkg/port/iscsit/session.go @@ -330,10 +330,17 @@ func (s *ISCSITargetDriver) LookupISCSISession(tgtName string, iniName string, i func (s *ISCSITargetDriver) UnBindISCSISession(sess *ISCSISession) { target := sess.Target + if target == nil { + // Discovery sessions have no target; just release the TSIH. + s.ReleaseTSIH(sess.TSIH) + return + } target.SessionsRWMutex.Lock() defer target.SessionsRWMutex.Unlock() delete(target.Sessions, sess.TSIH) - scsi.RemoveITNexus(sess.Target.SCSITarget, sess.ITNexus) + if sess.ITNexus != nil { + scsi.RemoveITNexus(target.SCSITarget, sess.ITNexus) + } s.ReleaseTSIH(sess.TSIH) log.Infof("session %x unbound from target %s", sess.TSIH, target.SCSITarget.Name) }