Support remounting devices at startup with correct flags

Source/common/SNTCommonEnums.h

@@ -162,6 +162,8 @@ typedef NS_ENUM(NSInteger, SNTDeviceManagerStartupPreferences) {
   SNTDeviceManagerStartupPreferencesNone,
   SNTDeviceManagerStartupPreferencesUnmount,
   SNTDeviceManagerStartupPreferencesForceUnmount,
+  SNTDeviceManagerStartupPreferencesRemount,
+  SNTDeviceManagerStartupPreferencesForceRemount,
 };
 
 #ifdef __cplusplus

Source/common/SNTConfigurator.m

@@ -644,6 +644,10 @@ - (SNTDeviceManagerStartupPreferences)onStartUSBOptions {
     return SNTDeviceManagerStartupPreferencesUnmount;
   } else if ([action isEqualToString:@"forceunmount"]) {
     return SNTDeviceManagerStartupPreferencesForceUnmount;
+  } else if ([action isEqualToString:@"remount"]) {
+    return SNTDeviceManagerStartupPreferencesRemount;
+  } else if ([action isEqualToString:@"forceremount"]) {
+    return SNTDeviceManagerStartupPreferencesForceRemount;
   } else {
     return SNTDeviceManagerStartupPreferencesNone;
   }

Source/santad/EventProviders/DiskArbitrationTestUtil.h

@@ -30,6 +30,7 @@ NS_ASSUME_NONNULL_BEGIN
 @interface MockDADisk : NSObject
 @property(nonatomic) NSDictionary *diskDescription;
 @property(nonatomic, readwrite) NSString *name;
+@property(nonatomic) BOOL wasMounted;
 @property(nonatomic) BOOL wasUnmounted;
 @end
 
@@ -40,14 +41,13 @@ typedef void (^MockDADiskAppearedCallback)(DADiskRef ref);
   NSMutableDictionary<NSString *, MockDADisk *> *insertedDevices;
 @property(nonatomic, readwrite, nonnull)
   NSMutableArray<MockDADiskAppearedCallback> *diskAppearedCallbacks;
-@property(nonatomic) BOOL wasRemounted;
 @property(nonatomic, nullable) dispatch_queue_t sessionQueue;
 
 - (instancetype _Nonnull)init;
 - (void)reset;
 
 // Also triggers DADiskRegisterDiskAppearedCallback
-- (void)insert:(MockDADisk *)ref bsdName:(NSString *)bsdName;
+- (void)insert:(MockDADisk *)ref;
 
 // Retrieve an initialized singleton MockDiskArbitration object
 + (instancetype _Nonnull)mockDiskArbitration;

Source/santad/EventProviders/DiskArbitrationTestUtil.mm

@@ -40,11 +40,14 @@ - (void)reset {
   [self.insertedDevices removeAllObjects];
   [self.diskAppearedCallbacks removeAllObjects];
   self.sessionQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
-  self.wasRemounted = NO;
 }
 
-- (void)insert:(MockDADisk *)ref bsdName:(NSString *)bsdName {
-  self.insertedDevices[bsdName] = ref;
+- (void)insert:(MockDADisk *)ref {
+  if (!ref.diskDescription[@"DAMediaBSDName"]) {
+    [NSException raise:@"Missing DAMediaBSDName"
+                format:@"The MockDADisk is missing the DAMediaBSDName diskDescription key."];
+  }
+  self.insertedDevices[ref.diskDescription[@"DAMediaBSDName"]] = ref;
 
   for (MockDADiskAppearedCallback callback in self.diskAppearedCallbacks) {
     dispatch_sync(self.sessionQueue, ^{
@@ -110,8 +113,13 @@ void DADiskMountWithArguments(DADiskRef _Nonnull disk, CFURLRef __nullable path,
                               DADiskMountOptions options, DADiskMountCallback __nullable callback,
                               void *__nullable context,
                               CFStringRef __nullable arguments[_Nullable]) {
-  MockDiskArbitration *mockDA = [MockDiskArbitration mockDiskArbitration];
-  mockDA.wasRemounted = YES;
+  MockDADisk *mockDisk = (__bridge MockDADisk *)disk;
+  mockDisk.wasMounted = YES;
+
+  if (context) {
+    dispatch_semaphore_t sema = (__bridge dispatch_semaphore_t)context;
+    dispatch_semaphore_signal(sema);
+  }
 }
 
 DADiskRef __nullable DADiskCreateFromBSDName(CFAllocatorRef __nullable allocator,

Source/santad/EventProviders/SNTEndpointSecurityDeviceManager.mm

@@ -47,34 +47,39 @@ - (void)logDiskDisappeared:(NSDictionary *)props;
 
 @property DASessionRef diskArbSession;
 @property(nonatomic, readonly) dispatch_queue_t diskQueue;
-@property dispatch_semaphore_t startupUnmountSema;
+@property dispatch_semaphore_t diskSema;
 
 @end
 
-void diskMountedCallback(DADiskRef disk, DADissenterRef dissenter, void *context) {
+void DiskMountedCallback(DADiskRef disk, DADissenterRef dissenter, void *context) {
   if (dissenter) {
     DAReturn status = DADissenterGetStatus(dissenter);
 
-    NSString *statusString = (NSString *)DADissenterGetStatusString(dissenter);
     IOReturn systemCode = err_get_system(status);
     IOReturn subSystemCode = err_get_sub(status);
     IOReturn errorCode = err_get_code(status);
 
     LOGE(@"SNTEndpointSecurityDeviceManager: dissenter status codes: system: %d, subsystem: %d, "
-         @"err: %d; status: %s",
-         systemCode, subSystemCode, errorCode, [statusString UTF8String]);
+         @"err: %d; status: %@",
+         systemCode, subSystemCode, errorCode,
+         CFBridgingRelease(DADissenterGetStatusString(dissenter)));
+  }
+
+  if (context) {
+    dispatch_semaphore_t sema = (__bridge dispatch_semaphore_t)context;
+    dispatch_semaphore_signal(sema);
   }
 }
 
-void diskAppearedCallback(DADiskRef disk, void *context) {
+void DiskAppearedCallback(DADiskRef disk, void *context) {
   NSDictionary *props = CFBridgingRelease(DADiskCopyDescription(disk));
   if (![props[@"DAVolumeMountable"] boolValue]) return;
   SNTEndpointSecurityDeviceManager *dm = (__bridge SNTEndpointSecurityDeviceManager *)context;
 
   [dm logDiskAppeared:props];
 }
 
-void diskDescriptionChangedCallback(DADiskRef disk, CFArrayRef keys, void *context) {
+void DiskDescriptionChangedCallback(DADiskRef disk, CFArrayRef keys, void *context) {
   NSDictionary *props = CFBridgingRelease(DADiskCopyDescription(disk));
   if (![props[@"DAVolumeMountable"] boolValue]) return;
 
@@ -85,7 +90,7 @@ void diskDescriptionChangedCallback(DADiskRef disk, CFArrayRef keys, void *conte
   }
 }
 
-void diskDisappearedCallback(DADiskRef disk, void *context) {
+void DiskDisappearedCallback(DADiskRef disk, void *context) {
   NSDictionary *props = CFBridgingRelease(DADiskCopyDescription(disk));
   if (![props[@"DAVolumeMountable"] boolValue]) return;
 
@@ -94,6 +99,21 @@ void diskDisappearedCallback(DADiskRef disk, void *context) {
   [dm logDiskDisappeared:props];
 }
 
+void DiskUnmountCallback(DADiskRef disk, DADissenterRef dissenter, void *context) {
+  if (dissenter) {
+    LOGW(@"Unable to unmount device: %@", CFBridgingRelease(DADissenterGetStatusString(dissenter)));
+  } else if (disk) {
+    NSDictionary *diskInfo = CFBridgingRelease(DADiskCopyDescription(disk));
+    LOGI(@"Unmounted device: Model: %@, Vendor: %@, Path: %@",
+         diskInfo[(__bridge NSString *)kDADiskDescriptionDeviceModelKey],
+         diskInfo[(__bridge NSString *)kDADiskDescriptionDeviceVendorKey],
+         diskInfo[(__bridge NSString *)kDADiskDescriptionVolumePathKey]);
+  }
+
+  dispatch_semaphore_t sema = (__bridge dispatch_semaphore_t)context;
+  dispatch_semaphore_signal(sema);
+}
+
 NSArray<NSString *> *maskToMountArgs(uint32_t remountOpts) {
   NSMutableArray<NSString *> *args = [NSMutableArray array];
   if (remountOpts & MNT_RDONLY) [args addObject:@"rdonly"];
@@ -111,43 +131,29 @@ uint32_t mountArgsToMask(NSArray<NSString *> *args) {
   uint32_t flags = 0;
   for (NSString *i in args) {
     NSString *arg = [i lowercaseString];
-    if ([arg isEqualToString:@"rdonly"])
+    if ([arg isEqualToString:@"rdonly"]) {
       flags |= MNT_RDONLY;
-    else if ([arg isEqualToString:@"noexec"])
+    } else if ([arg isEqualToString:@"noexec"]) {
       flags |= MNT_NOEXEC;
-    else if ([arg isEqualToString:@"nosuid"])
+    } else if ([arg isEqualToString:@"nosuid"]) {
       flags |= MNT_NOSUID;
-    else if ([arg isEqualToString:@"nobrowse"])
+    } else if ([arg isEqualToString:@"nobrowse"]) {
       flags |= MNT_DONTBROWSE;
-    else if ([arg isEqualToString:@"noowners"])
+    } else if ([arg isEqualToString:@"noowners"]) {
       flags |= MNT_UNKNOWNPERMISSIONS;
-    else if ([arg isEqualToString:@"nodev"])
+    } else if ([arg isEqualToString:@"nodev"]) {
       flags |= MNT_NODEV;
-    else if ([arg isEqualToString:@"-j"])
+    } else if ([arg isEqualToString:@"-j"]) {
       flags |= MNT_JOURNALED;
-    else if ([arg isEqualToString:@"async"])
+    } else if ([arg isEqualToString:@"async"]) {
       flags |= MNT_ASYNC;
-    else
+    } else {
       LOGE(@"SNTEndpointSecurityDeviceManager: unexpected mount arg: %@", arg);
+    }
   }
   return flags;
 }
 
-void UnmountCallback(DADiskRef disk, DADissenterRef dissenter, void *context) {
-  if (dissenter) {
-    LOGW(@"Unable to unmount device: %@", CFBridgingRelease(DADissenterGetStatusString(dissenter)));
-  } else if (disk) {
-    NSDictionary *diskInfo = CFBridgingRelease(DADiskCopyDescription(disk));
-    LOGI(@"Unmounted device: Model: %@, Vendor: %@, Path: %@",
-         diskInfo[(__bridge NSString *)kDADiskDescriptionDeviceModelKey],
-         diskInfo[(__bridge NSString *)kDADiskDescriptionDeviceVendorKey],
-         diskInfo[(__bridge NSString *)kDADiskDescriptionVolumePathKey]);
-  }
-
-  dispatch_semaphore_t sema = (__bridge dispatch_semaphore_t)context;
-  dispatch_semaphore_signal(sema);
-}
-
 NS_ASSUME_NONNULL_BEGIN
 
 @implementation SNTEndpointSecurityDeviceManager {
@@ -227,9 +233,15 @@ - (BOOL)remountUSBModeContainsFlags:(uint32_t)flags {
   return (flags & requiredFlags) == requiredFlags;
 }
 
+// NB: Remount options are implemented as separate "unmount" and "mount"
+// operations instead of using the "update"/MNT_UPDATE flag. This is because
+// filesystems often don't support many transitions (e.g. RW to RO). Performing
+// the two step process has a higher chance of succeeding.
 - (void)performStartupTasks:(SNTDeviceManagerStartupPreferences)startupPrefs {
   if (!self.blockUSBMount || (startupPrefs != SNTDeviceManagerStartupPreferencesUnmount &&
-                              startupPrefs != SNTDeviceManagerStartupPreferencesForceUnmount)) {
+                              startupPrefs != SNTDeviceManagerStartupPreferencesForceUnmount &&
+                              startupPrefs != SNTDeviceManagerStartupPreferencesRemount &&
+                              startupPrefs != SNTDeviceManagerStartupPreferencesForceRemount)) {
     return;
   }
 
@@ -241,8 +253,7 @@ - (void)performStartupTasks:(SNTDeviceManagerStartupPreferences)startupPrefs {
     return;
   }
 
-  self.startupUnmountSema = dispatch_semaphore_create(0);
-  int numUnmountAttempts = 0;
+  self.diskSema = dispatch_semaphore_create(0);
 
   for (int i = 0; i < numMounts; i++) {
     struct statfs *sfs = &mnts[i];
@@ -267,21 +278,36 @@ - (void)performStartupTasks:(SNTDeviceManagerStartupPreferences)startupPrefs {
     }
 
     DADiskUnmountOptions unmountOptions = kDADiskUnmountOptionDefault;
-    if (startupPrefs == SNTDeviceManagerStartupPreferencesForceUnmount) {
+    if (startupPrefs == SNTDeviceManagerStartupPreferencesForceUnmount ||
+        startupPrefs == SNTDeviceManagerStartupPreferencesForceRemount) {
       unmountOptions = kDADiskUnmountOptionForce;
     }
 
     LOGI(@"Attempting to unmount device: '%s' mounted on '%s'", sfs->f_mntfromname,
          sfs->f_mntonname);
 
-    DADiskUnmount(disk, unmountOptions, UnmountCallback, (__bridge void *)self.startupUnmountSema);
-    numUnmountAttempts++;
-  }
+    DADiskUnmount(disk, unmountOptions, DiskUnmountCallback, (__bridge void *)self.diskSema);
 
-  while (numUnmountAttempts-- > 0) {
-    if (dispatch_semaphore_wait(self.startupUnmountSema,
+    if (dispatch_semaphore_wait(self.diskSema,
                                 dispatch_time(DISPATCH_TIME_NOW, 5 * NSEC_PER_SEC))) {
-      LOGW(@"An unmount attempt took longer than expected. Device may still be mounted.");
+      LOGW(
+        @"Unmounting '%s' mounted on '%s' took longer than expected. Device may still be mounted.",
+        sfs->f_mntfromname, sfs->f_mntonname);
+      continue;
+    }
+
+    if (startupPrefs == SNTDeviceManagerStartupPreferencesRemount ||
+        startupPrefs == SNTDeviceManagerStartupPreferencesForceRemount) {
+      uint32_t newMode = sfs->f_flags | mountArgsToMask(self.remountArgs);
+      LOGI(@"Attempting to mount device again changing flags: 0x%08x --> 0x%08x", sfs->f_flags,
+           newMode);
+
+      [self remount:disk mountMode:newMode semaphore:self.diskSema];
+
+      if (dispatch_semaphore_wait(self.diskSema,
+                                  dispatch_time(DISPATCH_TIME_NOW, 5 * NSEC_PER_SEC))) {
+        LOGW(@"Failed to remount device after unmounting: %s", sfs->f_mntfromname);
+      }
     }
   }
 }
@@ -326,11 +352,11 @@ - (void)handleMessage:(Message &&)esMsg
 }
 
 - (void)enable {
-  DARegisterDiskAppearedCallback(_diskArbSession, NULL, diskAppearedCallback,
+  DARegisterDiskAppearedCallback(_diskArbSession, NULL, DiskAppearedCallback,
                                  (__bridge void *)self);
   DARegisterDiskDescriptionChangedCallback(_diskArbSession, NULL, NULL,
-                                           diskDescriptionChangedCallback, (__bridge void *)self);
-  DARegisterDiskDisappearedCallback(_diskArbSession, NULL, diskDisappearedCallback,
+                                           DiskDescriptionChangedCallback, (__bridge void *)self);
+  DARegisterDiskDisappearedCallback(_diskArbSession, NULL, DiskDisappearedCallback,
                                     (__bridge void *)self);
 
   [super subscribeAndClearCache:{
@@ -385,7 +411,7 @@ - (es_auth_result_t)handleAuthMount:(const Message &)m {
     uint32_t newMode = mountMode | remountOpts;
     LOGI(@"SNTEndpointSecurityDeviceManager: remounting device '%s'->'%s', flags (%u) -> (%u)",
          eventStatFS->f_mntfromname, eventStatFS->f_mntonname, mountMode, newMode);
-    [self remount:disk mountMode:newMode];
+    [self remount:disk mountMode:newMode semaphore:nil];
   }
 
   if (self.deviceBlockCallback) {
@@ -395,14 +421,16 @@ - (es_auth_result_t)handleAuthMount:(const Message &)m {
   return ES_AUTH_RESULT_DENY;
 }
 
-- (void)remount:(DADiskRef)disk mountMode:(uint32_t)remountMask {
+- (void)remount:(DADiskRef)disk
+      mountMode:(uint32_t)remountMask
+      semaphore:(nullable dispatch_semaphore_t)sema {
   NSArray<NSString *> *args = maskToMountArgs(remountMask);
   CFStringRef *argv = (CFStringRef *)calloc(args.count + 1, sizeof(CFStringRef));
   CFArrayGetValues((__bridge CFArrayRef)args, CFRangeMake(0, (CFIndex)args.count),
                    (const void **)argv);
 
-  DADiskMountWithArguments(disk, NULL, kDADiskMountOptionDefault, diskMountedCallback,
-                           (__bridge void *)self, (CFStringRef *)argv);
+  DADiskMountWithArguments(disk, NULL, kDADiskMountOptionDefault, DiskMountedCallback,
+                           (__bridge void *)sema, (CFStringRef *)argv);
 
   free(argv);
 }