santad: Avoid properties in critical path

google · Jul 13, 2016 · 6b0994a · 6b0994a
1 parent 7dd616e
commit 6b0994a
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 34 deletions.
diff --git 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
@@ -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,15 +180,15 @@ - (uint64_t)cacheCount {
 }
 
 - (BOOL)flushCache {
-  return IOConnectCallScalarMethod(self.connection,
+  return IOConnectCallScalarMethod(_connection,
                                    kSantaUserClientClearCache, 0, 0, 0, 0) == KERN_SUCCESS;
 }
 
 - (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
@@ -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
@@ -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];
       }
     }
   }