From 48b5ecd35d3aac232710cac2afdff3c699563cbb Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Wed, 4 Jan 2017 23:27:34 +0800 Subject: [PATCH 01/21] Bug fix: discovery data source not called. --- Source/User/iscsid/iSCSIDaemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/User/iscsid/iSCSIDaemon.c b/Source/User/iscsid/iSCSIDaemon.c index 72e064f8..6bad6509 100644 --- a/Source/User/iscsid/iSCSIDaemon.c +++ b/Source/User/iscsid/iSCSIDaemon.c @@ -2132,7 +2132,7 @@ int main(void) discoveryContext.info = &discoveryRecords; discoveryContext.perform = iSCSIDProcessDiscoveryData; discoverySource = CFRunLoopSourceCreate(kCFAllocatorDefault,1,&discoveryContext); - CFRunLoopAddSource(CFRunLoopGetMain(),sockSourceRead,kCFRunLoopDefaultMode); + CFRunLoopAddSource(CFRunLoopGetMain(),discoverySource,kCFRunLoopDefaultMode); asl_log(NULL,NULL,ASL_LEVEL_INFO,"daemon started"); From 97f9351616c93ad6d2b1c4fbf9d02ced5ef05cbe Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sun, 5 Mar 2017 15:16:03 +0800 Subject: [PATCH 02/21] Removed stale function getCurrentTask() --- Source/Kernel/iSCSITaskQueue.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Source/Kernel/iSCSITaskQueue.h b/Source/Kernel/iSCSITaskQueue.h index 471dec8b..68310ad8 100644 --- a/Source/Kernel/iSCSITaskQueue.h +++ b/Source/Kernel/iSCSITaskQueue.h @@ -83,10 +83,6 @@ class iSCSITaskQueue : public IOEventSource /*! Removes all tasks from the queue. */ void clearTasksFromQueue(); - /*! Gets the iSCSI task tag of the task that is current being processed. - * @return iSCSI task tag of the current task. */ - UInt32 getCurrentTask(); - protected: /*! Called by the attached work loop to check if there is any processing @@ -109,4 +105,4 @@ class iSCSITaskQueue : public IOEventSource }; -#endif \ No newline at end of file +#endif From 5e16d9dd1bfab363a46b5b89fca53fcb70811dad Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sun, 5 Mar 2017 15:17:37 +0800 Subject: [PATCH 03/21] replace OSSafeRelease with OSSafeReleaseNULL --- Source/Kernel/iSCSIHBAUserClient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Kernel/iSCSIHBAUserClient.cpp b/Source/Kernel/iSCSIHBAUserClient.cpp index 9ad77579..7fd4abd6 100644 --- a/Source/Kernel/iSCSIHBAUserClient.cpp +++ b/Source/Kernel/iSCSIHBAUserClient.cpp @@ -1393,7 +1393,7 @@ IOReturn iSCSIHBAUserClient::GetTargetIQNForSessionId(iSCSIHBAUserClient * targe IOLockUnlock(target->accessLock); - OSSafeRelease(iterator); + OSSafeReleaseNULL(iterator); return retVal; } From 35c9f5a7de172010c77efcad6d5b2cffe394ee0c Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sun, 5 Mar 2017 15:19:46 +0800 Subject: [PATCH 04/21] Removed stale taskQueue references. --- Source/Kernel/iSCSIIOEventSource.cpp | 8 ++------ Source/Kernel/iSCSIIOEventSource.h | 9 --------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/Source/Kernel/iSCSIIOEventSource.cpp b/Source/Kernel/iSCSIIOEventSource.cpp index 16d8925f..0dc843b1 100644 --- a/Source/Kernel/iSCSIIOEventSource.cpp +++ b/Source/Kernel/iSCSIIOEventSource.cpp @@ -51,11 +51,7 @@ bool iSCSIIOEventSource::init(iSCSIVirtualHBA * owner, iSCSIIOEventSource::session = session; iSCSIIOEventSource::connection = connection; - - // Initialize task queue to store parallel SCSI tasks for processing - queue_init(&taskQueue); - taskQueueLock = IOSimpleLockAlloc(); - + return true; } @@ -93,4 +89,4 @@ bool iSCSIIOEventSource::checkForWork() // Tell workloop thread not to call us again until we signal again... return false; -} \ No newline at end of file +} diff --git a/Source/Kernel/iSCSIIOEventSource.h b/Source/Kernel/iSCSIIOEventSource.h index e2a1c410..e79ce56e 100644 --- a/Source/Kernel/iSCSIIOEventSource.h +++ b/Source/Kernel/iSCSIIOEventSource.h @@ -100,15 +100,6 @@ class iSCSIIOEventSource : public IOEventSource /*! The iSCSI connection associated with this event source. */ iSCSIConnection * connection; - queue_head_t taskQueue; - - /*! Flag used to indicate whether the task at the head of the queue is a - * new task that has not yet been processed. */ - bool newTask; - - /*! Mutex lock used to prevent simultaneous access to the iSCSI task queue - * (e.g., simultaneous calls to addTaskToQueue() and removeTaskFromQueue(). */ - IOSimpleLock * taskQueueLock; }; #endif /* defined(__ISCSI_EVENT_SOURCE_H__) */ From 5fa1561dcff70b58a38b85ad88b88943cbd8b090 Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sun, 5 Mar 2017 16:02:29 +0800 Subject: [PATCH 05/21] Removed getCurrentTask() --- Source/Kernel/iSCSITaskQueue.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Source/Kernel/iSCSITaskQueue.cpp b/Source/Kernel/iSCSITaskQueue.cpp index 4ea40cd8..e508c7df 100644 --- a/Source/Kernel/iSCSITaskQueue.cpp +++ b/Source/Kernel/iSCSITaskQueue.cpp @@ -118,12 +118,6 @@ UInt32 iSCSITaskQueue::completeCurrentTask() return taskTag; } -/*! Gets the iSCSI task tag of the task that is current being processed. - * @return iSCSI task tag of the current task. */ -UInt32 getCurrentTask() -{ - return 0; -} bool iSCSITaskQueue::checkForWork() { From 6fa83f396050d72b24f637b9ee272611d7369c20 Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sun, 19 Mar 2017 19:30:12 +0800 Subject: [PATCH 06/21] Fix for mac OS SCSI layer (sense data must be accompanied by kSCSITaskStatus_Good; CHECK_CONDITION is fine if sense data is missing. --- Source/Kernel/iSCSIVirtualHBA.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Source/Kernel/iSCSIVirtualHBA.cpp b/Source/Kernel/iSCSIVirtualHBA.cpp index dc9235d6..41ae5fba 100644 --- a/Source/Kernel/iSCSIVirtualHBA.cpp +++ b/Source/Kernel/iSCSIVirtualHBA.cpp @@ -937,6 +937,7 @@ void iSCSIVirtualHBA::ProcessSCSIResponse(iSCSISession * session, SetRealizedDataTransferCount(parallelTask,(UInt32)GetRequestedDataTransferCount(parallelTask)); // Process sense data if the PDU came with any... + bool senseDataPresent = false; if(length >= senseDataHeaderSize) { // First two bytes of the data segment are the size of the sense data @@ -955,6 +956,8 @@ void iSCSIVirtualHBA::ProcessSCSIResponse(iSCSISession * session, // Incorporate sense data into the task SetAutoSenseData(parallelTask,newSenseData,senseDataLength); + senseDataPresent = true; + DBLog("iscsi: Processed sense data (sid: %d, cid: %d)\n", session->sessionId,connection->cid); } @@ -964,6 +967,13 @@ void iSCSIVirtualHBA::ProcessSCSIResponse(iSCSISession * session, // know that we're done with this task... SCSITaskStatus completionStatus = (SCSITaskStatus)bhs->status; + + // If sense data has been included along with a check condition response, + // the macOS SCSI stack expects that the task status is "GOOD". Otherwise, + // it queries for auto sense data. + if(completionStatus == kSCSITaskStatus_CHECK_CONDITION && senseDataPresent) + completionStatus = kSCSITaskStatus_GOOD; + SCSIServiceResponse serviceResponse; if(bhs->response == kiSCSIPDUSCSICmdCompleted) @@ -978,6 +988,7 @@ void iSCSIVirtualHBA::ProcessSCSIResponse(iSCSISession * session, DBLog("iscsi: Processed SCSI response (sid: %d, cid: %d)\n", session->sessionId,connection->cid); + } void iSCSIVirtualHBA::ProcessDataIn(iSCSISession * session, From 299c7c7e4c1e6d5205f91e9a3b289d11dcad8ee7 Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sun, 19 Mar 2017 19:39:13 +0800 Subject: [PATCH 07/21] Added "iSCSI" connection type to IO Registry --- Source/Kernel/iSCSIVirtualHBA.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Source/Kernel/iSCSIVirtualHBA.cpp b/Source/Kernel/iSCSIVirtualHBA.cpp index 41ae5fba..22d522b0 100644 --- a/Source/Kernel/iSCSIVirtualHBA.cpp +++ b/Source/Kernel/iSCSIVirtualHBA.cpp @@ -156,9 +156,11 @@ bool iSCSIVirtualHBA::InitializeTargetForID(SCSITargetIdentifier targetId) if(targetIQN) { protocolDict->setObject("iSCSI Qualified Name",targetIQN); - device->setProperty(kIOPropertyProtocolCharacteristicsKey,protocolDict); } + protocolDict->setObject(kIOPropertyPhysicalInterconnectTypeKey,OSString::withCString("iSCSI")); + device->setProperty(kIOPropertyProtocolCharacteristicsKey,protocolDict); + protocolDict->release(); } From 0090abda20f81295d5dfe3b68083132291c56c9e Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Mon, 24 Apr 2017 22:37:06 +0800 Subject: [PATCH 08/21] Removed exposed header --- Source/User/iSCSI Framework/iSCSIUtils.c | 1 + Source/User/iSCSI Framework/iSCSIUtils.h | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Source/User/iSCSI Framework/iSCSIUtils.c b/Source/User/iSCSI Framework/iSCSIUtils.c index 403b8fca..fdb6112f 100644 --- a/Source/User/iSCSI Framework/iSCSIUtils.c +++ b/Source/User/iSCSI Framework/iSCSIUtils.c @@ -27,6 +27,7 @@ */ #include "iSCSIUtils.h" +#include /*! Minimum TCP port. */ static int PORT_MIN = 0; diff --git a/Source/User/iSCSI Framework/iSCSIUtils.h b/Source/User/iSCSI Framework/iSCSIUtils.h index c66adcce..6468668b 100644 --- a/Source/User/iSCSI Framework/iSCSIUtils.h +++ b/Source/User/iSCSI Framework/iSCSIUtils.h @@ -33,11 +33,12 @@ #include #include -#include #include #include "iSCSITypes.h" +struct sockaddr_storage; + /*! Verifies whether specified iSCSI qualified name (IQN) is valid per RFC3720. * This function also validates 64-bit EUI names expressed as strings that * start with the "eui" prefix. From 076a478dac0295256583cccfe8ee29f8cb541327 Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Mon, 24 Apr 2017 22:46:30 +0800 Subject: [PATCH 09/21] Fix: allows ion names without a ':' qualifier after the TLD (e.g., iqn.2012-06.com.example is allowed per RFC3720) --- Source/User/iSCSI Framework/iSCSIUtils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/User/iSCSI Framework/iSCSIUtils.c b/Source/User/iSCSI Framework/iSCSIUtils.c index fdb6112f..c6641f93 100644 --- a/Source/User/iSCSI Framework/iSCSIUtils.c +++ b/Source/User/iSCSI Framework/iSCSIUtils.c @@ -44,7 +44,7 @@ Boolean iSCSIUtilsValidateIQN(CFStringRef IQN) { // IEEE regular expression for matching IQN name const char pattern[] = "^iqn[.][0-9]{4}-[0-9]{2}[.][[:alnum:]]{1,}[.]" - "[-A-Za-z0-9.]{1,255}:[-A-Za-z0-9.]{1,255}" + "[-A-Za-z0-9.]{1,255}" "|^eui[.][[:xdigit:]]{16}$"; Boolean validName = false; From 7c6bf6bb4dbc73e58db41e7882bbcb8d387ad396 Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Thu, 27 Apr 2017 22:28:14 +0800 Subject: [PATCH 10/21] FIX: Target name is equal to kiSCSIUnspecifiedTargetIQN for discovery sessions, not NULL; resulting in incorrect session parameters which are sometimes incompatible with certain targets. --- Source/User/iscsid/iSCSISession.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/User/iscsid/iSCSISession.c b/Source/User/iscsid/iSCSISession.c index c82bf600..00cd7a7a 100644 --- a/Source/User/iscsid/iSCSISession.c +++ b/Source/User/iscsid/iSCSISession.c @@ -499,7 +499,8 @@ errno_t iSCSINegotiateSession(iSCSISessionManagerRef managerRef, iSCSINegotiateBuildSWDictCommon(sessCfg,sessCmd); // If target name is specified, this is a normal session; add parameters - if(iSCSITargetGetIQN(target) != NULL) + Boolean discoverySession = CFStringCompare(iSCSITargetGetIQN(target),kiSCSIUnspecifiedTargetIQN,0) == kCFCompareEqualTo; + if(!discoverySession) iSCSINegotiateBuildSWDictNormal(sessCfg,sessCmd); // Add connection parameters From 95ff15c79f5f9ada699371d8dfcde1baa144c39c Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Thu, 27 Apr 2017 22:57:27 +0800 Subject: [PATCH 11/21] FIX: Ignores irrelevant FirstBurstLength parameter when initialR2T is YES and immediateData is NO. --- Source/User/iscsid/iSCSISession.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/Source/User/iscsid/iSCSISession.c b/Source/User/iscsid/iSCSISession.c index 00cd7a7a..444d8fec 100644 --- a/Source/User/iscsid/iSCSISession.c +++ b/Source/User/iscsid/iSCSISession.c @@ -273,6 +273,9 @@ errno_t iSCSINegotiateParseSWDictNormal(iSCSISessionManagerRef managerRef, // Holds target value & comparison result for keys that we'll process CFStringRef targetRsp; + // Holds parameters that are used to process other parameters + Boolean initiatR2T = false, immediatedata = false; + // Get data digest key and compare to requested value if(CFDictionaryGetValueIfPresent(sessRsp,kRFC3720_Key_MaxConnections,(void*)&targetRsp)) { @@ -292,7 +295,7 @@ errno_t iSCSINegotiateParseSWDictNormal(iSCSISessionManagerRef managerRef, if(CFDictionaryGetValueIfPresent(sessRsp,kRFC3720_Key_InitialR2T,(void*)&targetRsp)) { CFStringRef initCmd = CFDictionaryGetValue(sessCmd,kRFC3720_Key_InitialR2T); - Boolean initialR2T = iSCSILVGetOr(initCmd,targetRsp); + initialR2T = iSCSILVGetOr(initCmd,targetRsp); iSCSIHBAInterfaceSetSessionParameter(hbaInterface,sessionId,kiSCSIHBASOInitialR2T, &initialR2T,sizeof(initialR2T)); } @@ -301,7 +304,7 @@ errno_t iSCSINegotiateParseSWDictNormal(iSCSISessionManagerRef managerRef, if(CFDictionaryGetValueIfPresent(sessRsp,kRFC3720_Key_ImmediateData,(void*)&targetRsp)) { CFStringRef initCmd = CFDictionaryGetValue(sessCmd,kRFC3720_Key_ImmediateData); - Boolean immediateData = iSCSILVGetAnd(initCmd,targetRsp); + immediateData = iSCSILVGetAnd(initCmd,targetRsp); iSCSIHBAInterfaceSetSessionParameter(hbaInterface,sessionId,kiSCSIHBASOImmediateData, &immediateData,sizeof(immediateData)); } @@ -336,16 +339,20 @@ errno_t iSCSINegotiateParseSWDictNormal(iSCSISessionManagerRef managerRef, // Grab minimum of first burst length if(CFDictionaryGetValueIfPresent(sessRsp,kRFC3720_Key_FirstBurstLength,(void*)&targetRsp)) { - CFStringRef initCmd = CFDictionaryGetValue(sessCmd,kRFC3720_Key_FirstBurstLength); - UInt32 firstBurstLength = CFStringGetIntValue(targetRsp); + // This parameter is irrelevant when initialR2T = yes and immediateData = no. + if(!initialR2T || immediatedata) { - // Range-check value... - if(iSCSILVRangeInvalid(firstBurstLength,kRFC3720_FirstBurstLength_Min,kRFC3720_FirstBurstLength_Max)) - return ENOTSUP; + CFStringRef initCmd = CFDictionaryGetValue(sessCmd,kRFC3720_Key_FirstBurstLength); + UInt32 firstBurstLength = CFStringGetIntValue(targetRsp); - firstBurstLength = iSCSILVGetMin(initCmd,targetRsp); - iSCSIHBAInterfaceSetSessionParameter(hbaInterface,sessionId,kiSCSIHBASOFirstBurstLength, - &firstBurstLength,sizeof(firstBurstLength)); + // Range-check value... + if(iSCSILVRangeInvalid(firstBurstLength,kRFC3720_FirstBurstLength_Min,kRFC3720_FirstBurstLength_Max)) + return ENOTSUP; + + firstBurstLength = iSCSILVGetMin(initCmd,targetRsp); + iSCSIHBAInterfaceSetSessionParameter(hbaInterface,sessionId,kiSCSIHBASOFirstBurstLength, + &firstBurstLength,sizeof(firstBurstLength)); + } } // Grab minimum of max outstanding R2T From 34fead8938772df8efdd49cfb6dda2646b193bfe Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sun, 21 May 2017 19:49:09 +0800 Subject: [PATCH 12/21] Fix typos --- Source/User/iscsid/iSCSISession.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/User/iscsid/iSCSISession.c b/Source/User/iscsid/iSCSISession.c index 444d8fec..ef06eebe 100644 --- a/Source/User/iscsid/iSCSISession.c +++ b/Source/User/iscsid/iSCSISession.c @@ -274,7 +274,7 @@ errno_t iSCSINegotiateParseSWDictNormal(iSCSISessionManagerRef managerRef, CFStringRef targetRsp; // Holds parameters that are used to process other parameters - Boolean initiatR2T = false, immediatedata = false; + Boolean initialR2T = false, immediateData = false; // Get data digest key and compare to requested value if(CFDictionaryGetValueIfPresent(sessRsp,kRFC3720_Key_MaxConnections,(void*)&targetRsp)) @@ -340,7 +340,7 @@ errno_t iSCSINegotiateParseSWDictNormal(iSCSISessionManagerRef managerRef, if(CFDictionaryGetValueIfPresent(sessRsp,kRFC3720_Key_FirstBurstLength,(void*)&targetRsp)) { // This parameter is irrelevant when initialR2T = yes and immediateData = no. - if(!initialR2T || immediatedata) { + if(!initialR2T || immediateData) { CFStringRef initCmd = CFDictionaryGetValue(sessCmd,kRFC3720_Key_FirstBurstLength); UInt32 firstBurstLength = CFStringGetIntValue(targetRsp); From bdac64a543f1fd2169a3d80aba46dda3ea57036a Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sun, 21 May 2017 23:06:18 +0800 Subject: [PATCH 13/21] Fix potential stability problem (NULL pointer dereference) --- Source/Kernel/iSCSIVirtualHBA.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Kernel/iSCSIVirtualHBA.cpp b/Source/Kernel/iSCSIVirtualHBA.cpp index 22d522b0..52f31ab9 100644 --- a/Source/Kernel/iSCSIVirtualHBA.cpp +++ b/Source/Kernel/iSCSIVirtualHBA.cpp @@ -465,7 +465,7 @@ void iSCSIVirtualHBA::HandleConnectionTimeout(SessionIdentifier sessionId,Connec // Send a notification to the daemon; if the daemon does not respond then // release the session or connection as appropriate - if(client->sendTimeoutMessageNotification(sessionId,connectionId) != kIOReturnSuccess) + if(!client || client->sendTimeoutMessageNotification(sessionId,connectionId) != kIOReturnSuccess) { if(connectionCount > 1) ReleaseConnection(sessionId,connectionId); @@ -1547,7 +1547,7 @@ errno_t iSCSIVirtualHBA::CreateConnection(SessionIdentifier sessionId, goto EVENTSOURCE_ADD_FAILURE; newConn->dataRecvEventSource->disable(); - + // Create a new socket (per RFC3720, only TCP sockets are used. // Domain can be either IPv4 or IPv6. error = sock_socket(portalSockaddr->ss_family, From cca20b6541551f28aa9fd6aa850e31e38817579a Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Mon, 22 May 2017 01:22:41 +0800 Subject: [PATCH 14/21] Generate initiator ID prior to service registration --- Source/Kernel/iSCSIVirtualHBA.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Source/Kernel/iSCSIVirtualHBA.cpp b/Source/Kernel/iSCSIVirtualHBA.cpp index 52f31ab9..ed88a441 100644 --- a/Source/Kernel/iSCSIVirtualHBA.cpp +++ b/Source/Kernel/iSCSIVirtualHBA.cpp @@ -356,12 +356,13 @@ bool iSCSIVirtualHBA::InitializeController() SetHBAProperty(kIOPropertyProductNameKey,OSString::withCString(ISCSI_PRODUCT_NAME)); SetHBAProperty(kIOPropertyProductRevisionLevelKey,OSString::withCString(ISCSI_PRODUCT_REVISION_LEVEL)); + // Generate an initiator id using a random number (per RFC3720) + kInitiatorId = random(); + // Make ourselves discoverable to user clients (we do this last after // everything is initialized). registerService(); - - // Generate an initiator id using a random number (per RFC3720) - kInitiatorId = random(); + // Successfully initialized controller return true; From 10ccdbca1f36ad3caa88033017c87d03c32fb9ba Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sat, 3 Jun 2017 16:43:33 +0800 Subject: [PATCH 15/21] Null checks and memory leak fix for iSCSIDAutoLogin() --- Source/User/iscsid/iSCSIDaemon.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Source/User/iscsid/iSCSIDaemon.c b/Source/User/iscsid/iSCSIDaemon.c index 6bad6509..97d3288b 100644 --- a/Source/User/iscsid/iSCSIDaemon.c +++ b/Source/User/iscsid/iSCSIDaemon.c @@ -1731,13 +1731,17 @@ void iSCSIDAutoLogin() for(CFIndex idx = 0; idx < targetsCount; idx++) { CFStringRef targetIQN = CFArrayGetValueAtIndex(targets,idx); - iSCSITargetRef target = iSCSIPreferencesCopyTarget(preferences,targetIQN); + iSCSITargetRef target = NULL; + + if(!(target = iSCSIPreferencesCopyTarget(preferences,targetIQN))) + continue; // See if this target requires auto-login and process it if(iSCSIPreferencesGetAutoLoginForTarget(preferences,targetIQN)) { - CFArrayRef portals = iSCSIPreferencesCreateArrayOfPortalsForTarget(preferences,targetIQN); - if(!portals) + CFArrayRef portals = NULL; + + if(!(portals = iSCSIPreferencesCreateArrayOfPortalsForTarget(preferences,targetIQN))) continue; CFIndex portalsCount = CFArrayGetCount(portals); @@ -1747,12 +1751,17 @@ void iSCSIDAutoLogin() { CFStringRef portalAddress = CFArrayGetValueAtIndex(portals,portalIdx); iSCSIPortalRef portal = iSCSIPreferencesCopyPortalForTarget(preferences,targetIQN,portalAddress); - iSCSIDQueueLogin(target,portal); + + if(portal) { + iSCSIDQueueLogin(target,portal); + iSCSIPortalRelease(portal); + } } - iSCSITargetRelease(target); CFRelease(portals); } + + iSCSITargetRelease(target); } CFRelease(targets); } From b4f16e10712cafdbbfd33cbbb4a46c78c7dfbf0f Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sat, 3 Jun 2017 16:49:22 +0800 Subject: [PATCH 16/21] iSCSIDQueueLogin retains target and portal object (potential null dereference fix) --- Source/User/iscsid/iSCSIDaemon.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Source/User/iscsid/iSCSIDaemon.c b/Source/User/iscsid/iSCSIDaemon.c index 97d3288b..e13c9c17 100644 --- a/Source/User/iscsid/iSCSIDaemon.c +++ b/Source/User/iscsid/iSCSIDaemon.c @@ -1643,6 +1643,12 @@ void iSCSIDProcessQueuedLogin(SCNetworkReachabilityRef reachabilityTarget, * portal when the network becomes available. */ void iSCSIDQueueLogin(iSCSITargetRef target,iSCSIPortalRef portal) { + if(!target || !portal) + return; + + iSCSITargetRetain(target); + iSCSIPortalRetain(portal); + SCNetworkReachabilityRef reachabilityTarget; SCNetworkReachabilityContext reachabilityContext; From cbfe751a28f201b486056742655c7711ae43c8ef Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sat, 3 Jun 2017 17:11:49 +0800 Subject: [PATCH 17/21] Fix memory leak in timeout handler --- Source/User/iscsid/iSCSISessionManager.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Source/User/iscsid/iSCSISessionManager.c b/Source/User/iscsid/iSCSISessionManager.c index 8b8f4a81..9486dd8a 100644 --- a/Source/User/iscsid/iSCSISessionManager.c +++ b/Source/User/iscsid/iSCSISessionManager.c @@ -66,6 +66,9 @@ struct __iSCSISessionManager // Call user-defined callback function if one exists if(managerRef->callbacks.timeoutCallback) managerRef->callbacks.timeoutCallback(target,portal); + + iSCSITargetRelease(target); + iSCSIPortalRelease(portal); } /*! Called to handle asynchronous events that involve dropped sessions, connections, From cdba0dedf04e3ffd4ba3dccbfd4f12741622048a Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sat, 3 Jun 2017 17:13:01 +0800 Subject: [PATCH 18/21] Fix memory release for target/portal in event handler --- Source/User/iscsid/iSCSIDaemon.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Source/User/iscsid/iSCSIDaemon.c b/Source/User/iscsid/iSCSIDaemon.c index e13c9c17..d6d02236 100644 --- a/Source/User/iscsid/iSCSIDaemon.c +++ b/Source/User/iscsid/iSCSIDaemon.c @@ -1713,10 +1713,6 @@ void iSCSIDSessionTimeoutHandler(iSCSITargetRef target,iSCSIPortalRef portal) // available if(iSCSIPreferencesGetPersistenceForTarget(preferences,iSCSITargetGetIQN(target))) iSCSIDQueueLogin(target,portal); - else { - iSCSITargetRelease(target); - iSCSIPortalRelease(portal); - } } /*! Automatically logs in to targets that were specified for auto-login. From dbaf1c820c0c17ce99c73eb4ba194b196330c809 Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sat, 3 Jun 2017 17:14:31 +0800 Subject: [PATCH 19/21] Fix null check, moved client references together for easy reading --- Source/Kernel/iSCSIVirtualHBA.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Source/Kernel/iSCSIVirtualHBA.cpp b/Source/Kernel/iSCSIVirtualHBA.cpp index ed88a441..46b712f2 100644 --- a/Source/Kernel/iSCSIVirtualHBA.cpp +++ b/Source/Kernel/iSCSIVirtualHBA.cpp @@ -455,8 +455,6 @@ void iSCSIVirtualHBA::HandleConnectionTimeout(SessionIdentifier sessionId,Connec for(ConnectionIdentifier connectionId = 0; connectionId < kiSCSIMaxConnectionsPerSession; connectionId++) if(session->connections[connectionId]) connectionCount++; - - iSCSIHBAUserClient * client = (iSCSIHBAUserClient*)getClient(); // In the future add recovery here... if(connectionCount > 1) @@ -466,14 +464,17 @@ void iSCSIVirtualHBA::HandleConnectionTimeout(SessionIdentifier sessionId,Connec // Send a notification to the daemon; if the daemon does not respond then // release the session or connection as appropriate - if(!client || client->sendTimeoutMessageNotification(sessionId,connectionId) != kIOReturnSuccess) + iSCSIHBAUserClient * client = (iSCSIHBAUserClient*)getClient(); + + if(client) + client->sendTimeoutMessageNotification(sessionId,connectionId) + else { if(connectionCount > 1) ReleaseConnection(sessionId,connectionId); else ReleaseSession(sessionId); } - } SCSIServiceResponse iSCSIVirtualHBA::ProcessParallelTask(SCSIParallelTaskIdentifier parallelTask) From d01d9d01d02aebc19fb1b59fca2420f66b595602 Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sat, 3 Jun 2017 17:21:17 +0800 Subject: [PATCH 20/21] Compile error --- Source/Kernel/iSCSIVirtualHBA.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Kernel/iSCSIVirtualHBA.cpp b/Source/Kernel/iSCSIVirtualHBA.cpp index 46b712f2..19d03b0c 100644 --- a/Source/Kernel/iSCSIVirtualHBA.cpp +++ b/Source/Kernel/iSCSIVirtualHBA.cpp @@ -467,7 +467,7 @@ void iSCSIVirtualHBA::HandleConnectionTimeout(SessionIdentifier sessionId,Connec iSCSIHBAUserClient * client = (iSCSIHBAUserClient*)getClient(); if(client) - client->sendTimeoutMessageNotification(sessionId,connectionId) + client->sendTimeoutMessageNotification(sessionId,connectionId); else { if(connectionCount > 1) From 86209144c14dc3ceeab7415f294b31fab1928ecf Mon Sep 17 00:00:00 2001 From: Nareg Sinenian Date: Sat, 3 Jun 2017 18:45:39 +0800 Subject: [PATCH 21/21] Update scripts for beta6 --- Distribution/package.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Distribution/package.sh b/Distribution/package.sh index e8d91ecd..106692ab 100755 --- a/Distribution/package.sh +++ b/Distribution/package.sh @@ -1,7 +1,7 @@ # Package parameters NAME="iSCSI Initiator for macOS" BUNDLE_ID="com.github.iscsi-osx.iSCSIInitiator" -VERSION="1.0.0-beta5" +VERSION="1.0.0-beta6" # Output of final DMG RELEASE="../Release"