From 8ffe5ec5ce257bd3ade7e77f9e7d1eb3722df836 Mon Sep 17 00:00:00 2001 From: Lei Xue Date: Tue, 17 Mar 2026 14:37:16 +0800 Subject: [PATCH] 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 }