From 6b0994a9909e05d3aca0bac8af9174e72e63db5e Mon Sep 17 00:00:00 2001 From: Russell Hancox Date: Tue, 12 Jul 2016 15:34:40 -0400 Subject: [PATCH] santad: Avoid properties in critical path --- Source/santad/SNTApplication.m | 6 +++--- Source/santad/SNTDriverManager.m | 25 +++++++++++-------------- Source/santad/SNTEventLog.m | 11 +++++------ Source/santad/SNTExecutionController.m | 20 +++++++++----------- 4 files changed, 28 insertions(+), 34 deletions(-) diff --git a/Source/santad/SNTApplication.m b/Source/santad/SNTApplication.m index 024834836..c020d87a8 100644 --- a/Source/santad/SNTApplication.m +++ b/Source/santad/SNTApplication.m @@ -146,7 +146,7 @@ - (void)beginListeningForDecisionRequests { } case ACTION_REQUEST_BINARY: { dispatch_async(exec_queue, ^{ - [self.execController validateBinaryWithMessage:message]; + [_execController validateBinaryWithMessage:message]; }); break; } @@ -177,14 +177,14 @@ - (void)beginListeningForLogRequests { NSRegularExpression *re = [[SNTConfigurator configurator] fileChangesRegex]; NSString *path = @(message.path); if ([re numberOfMatchesInString:path options:0 range:NSMakeRange(0, path.length)]) { - [self.eventLog logFileModification:message]; + [_eventLog logFileModification:message]; } }); break; } case ACTION_NOTIFY_EXEC: { dispatch_async(log_queue, ^{ - [self.eventLog logAllowedExecution:message]; + [_eventLog logAllowedExecution:message]; }); break; } diff --git a/Source/santad/SNTDriverManager.m b/Source/santad/SNTDriverManager.m index 386f33be1..5f0f1cf09 100644 --- a/Source/santad/SNTDriverManager.m +++ b/Source/santad/SNTDriverManager.m @@ -98,13 +98,9 @@ - (void)listenForRequestsOfType:(santa_queuetype_t)type withCallback:(void (^)(santa_message_t))callback { kern_return_t kr; - mach_port_t receivePort = 0; - IODataQueueMemory *queueMemory = NULL; - mach_vm_address_t address = 0; - mach_vm_size_t size = 0; - // Allocate a mach port to receive notifactions from the IODataQueue - if (!(receivePort = IODataQueueAllocateNotificationPort())) { + mach_port_t receivePort = IODataQueueAllocateNotificationPort(); + if (receivePort == MACH_PORT_NULL) { LOGD(@"Failed to allocate notification port"); return; } @@ -118,15 +114,16 @@ - (void)listenForRequestsOfType:(santa_queuetype_t)type } // This will call clientMemoryForType() inside our user client class. - kr = IOConnectMapMemory(self.connection, type, mach_task_self(), - &address, &size, kIOMapAnywhere); + mach_vm_address_t address = 0; + mach_vm_size_t size = 0; + kr = IOConnectMapMemory(self.connection, type, mach_task_self(), &address, &size, kIOMapAnywhere); if (kr != kIOReturnSuccess) { LOGD(@"Failed to map memory for type %d: %d", type, kr); mach_port_destroy(mach_task_self(), receivePort); return; } - queueMemory = (IODataQueueMemory *)address; + IODataQueueMemory *queueMemory = (IODataQueueMemory *)address; do { while (IODataQueueDataAvailable(queueMemory)) { @@ -151,14 +148,14 @@ - (void)listenForRequestsOfType:(santa_queuetype_t)type - (kern_return_t)postToKernelAction:(santa_action_t)action forVnodeID:(uint64_t)vnodeId { switch (action) { case ACTION_RESPOND_ALLOW: - return IOConnectCallScalarMethod(self.connection, + return IOConnectCallScalarMethod(_connection, kSantaUserClientAllowBinary, &vnodeId, 1, 0, 0); case ACTION_RESPOND_DENY: - return IOConnectCallScalarMethod(self.connection, + return IOConnectCallScalarMethod(_connection, kSantaUserClientDenyBinary, &vnodeId, 1, @@ -173,7 +170,7 @@ - (uint64_t)cacheCount { uint32_t input_count = 1; uint64_t cache_count = 0; - IOConnectCallScalarMethod(self.connection, + IOConnectCallScalarMethod(_connection, kSantaUserClientCacheCount, 0, 0, @@ -183,7 +180,7 @@ - (uint64_t)cacheCount { } - (BOOL)flushCache { - return IOConnectCallScalarMethod(self.connection, + return IOConnectCallScalarMethod(_connection, kSantaUserClientClearCache, 0, 0, 0, 0) == KERN_SUCCESS; } @@ -191,7 +188,7 @@ - (santa_action_t)checkCache:(uint64_t)vnodeID { uint32_t input_count = 1; uint64_t vnode_action = 0; - IOConnectCallScalarMethod(self.connection, + IOConnectCallScalarMethod(_connection, kSantaUserClientCheckCache, &vnodeID, 1, diff --git a/Source/santad/SNTEventLog.m b/Source/santad/SNTEventLog.m index bf0290c0e..46b930ce3 100644 --- a/Source/santad/SNTEventLog.m +++ b/Source/santad/SNTEventLog.m @@ -44,8 +44,8 @@ - (instancetype)init { } - (void)saveDecisionDetails:(SNTCachedDecision *)cd { - dispatch_sync(self.detailStoreQueue, ^{ - self.detailStore[@(cd.vnodeId)] = cd; + dispatch_sync(_detailStoreQueue, ^{ + _detailStore[@(cd.vnodeId)] = cd; }); } @@ -109,8 +109,8 @@ - (void)logDeniedExecution:(SNTCachedDecision *)cd withMessage:(santa_message_t) - (void)logAllowedExecution:(santa_message_t)message { __block SNTCachedDecision *cd; - dispatch_sync(self.detailStoreQueue, ^{ - cd = self.detailStore[@(message.vnode_id)]; + dispatch_sync(_detailStoreQueue, ^{ + cd = _detailStore[@(message.vnode_id)]; }); [self logExecution:message withDecision:cd]; } @@ -183,8 +183,7 @@ - (void)logExecution:(santa_message_t)message withDecision:(SNTCachedDecision *) } if (cd.quarantineURL) { - [outLog appendFormat:@"|quarantine_url=%@", - [self sanitizeString:cd.quarantineURL]]; + [outLog appendFormat:@"|quarantine_url=%@", [self sanitizeString:cd.quarantineURL]]; } NSString *user, *group; diff --git a/Source/santad/SNTExecutionController.m b/Source/santad/SNTExecutionController.m index be583f424..efc8ca00d 100644 --- a/Source/santad/SNTExecutionController.m +++ b/Source/santad/SNTExecutionController.m @@ -65,8 +65,7 @@ - (instancetype)initWithDriverManager:(SNTDriverManager *)driverManager _eventLog = eventLog; _uploadBackoff = [NSMutableDictionary dictionaryWithCapacity:128]; - _eventQueue = dispatch_queue_create("com.google.santad.event_upload", - DISPATCH_QUEUE_SERIAL); + _eventQueue = dispatch_queue_create("com.google.santad.event_upload", DISPATCH_QUEUE_SERIAL); // This establishes the XPC connection between libsecurity and syspolicyd. // Not doing this causes a deadlock as establishing this link goes through xpcproxy. @@ -78,7 +77,7 @@ - (instancetype)initWithDriverManager:(SNTDriverManager *)driverManager #pragma mark Binary Validation - (SNTEventState)makeDecision:(SNTCachedDecision *)cd binaryInfo:(SNTFileInfo *)fi { - SNTRule *rule = [self.ruleTable binaryRuleForSHA256:cd.sha256]; + SNTRule *rule = [_ruleTable binaryRuleForSHA256:cd.sha256]; if (rule) { switch (rule.state) { case SNTRuleStateWhitelist: @@ -92,7 +91,7 @@ - (SNTEventState)makeDecision:(SNTCachedDecision *)cd binaryInfo:(SNTFileInfo *) } } - rule = [self.ruleTable certificateRuleForSHA256:cd.certSHA256]; + rule = [_ruleTable certificateRuleForSHA256:cd.certSHA256]; if (rule) { switch (rule.state) { case SNTRuleStateWhitelist: @@ -131,8 +130,7 @@ - (void)validateBinaryWithMessage:(santa_message_t)message { SNTFileInfo *binInfo = [[SNTFileInfo alloc] initWithPath:@(message.path) error:&fileInfoError]; if (!binInfo) { LOGW(@"Failed to read file %@: %@", binInfo.path, fileInfoError.localizedDescription); - [self.driverManager postToKernelAction:ACTION_RESPOND_ALLOW - forVnodeID:message.vnode_id]; + [_driverManager postToKernelAction:ACTION_RESPOND_ALLOW forVnodeID:message.vnode_id]; return; } @@ -153,10 +151,10 @@ - (void)validateBinaryWithMessage:(santa_message_t)message { // Save decision details for logging the execution later. santa_action_t action = [self actionForEventState:cd.decision]; - if (action == ACTION_RESPOND_ALLOW) [self.eventLog saveDecisionDetails:cd]; + if (action == ACTION_RESPOND_ALLOW) [_eventLog saveDecisionDetails:cd]; // Send the decision to the kernel. - [self.driverManager postToKernelAction:action forVnodeID:cd.vnodeId]; + [_driverManager postToKernelAction:action forVnodeID:cd.vnodeId]; // Log to database if necessary. if (cd.decision != SNTEventStateAllowBinary && @@ -199,12 +197,12 @@ - (void)validateBinaryWithMessage:(santa_message_t)message { se.quarantineAgentBundleID = binInfo.quarantineAgentBundleID; dispatch_async(_eventQueue, ^{ - [self.eventTable addStoredEvent:se]; + [_eventTable addStoredEvent:se]; }); // If binary was blocked, do the needful if (action != ACTION_RESPOND_ALLOW) { - [self.eventLog logDeniedExecution:cd withMessage:message]; + [_eventLog logDeniedExecution:cd withMessage:message]; // So the server has something to show the user straight away, initiate an event // upload for the blocked binary rather than waiting for the next sync. @@ -227,7 +225,7 @@ - (void)validateBinaryWithMessage:(santa_message_t)message { } [self printMessage:msg toTTYForPID:message.ppid]; - [self.notifierQueue addEvent:se customMessage:cd.customMsg]; + [_notifierQueue addEvent:se customMessage:cd.customMsg]; } } }