From 5f26356079f6f97ea0e2a40b053ec0c257e07e66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Tue, 11 Jun 2024 20:53:58 +0200 Subject: [PATCH] [MOUNTMGR] Rectify "NoAutoMount" usage (#7030) This "NoAutoMount" member was not consistently used. Sometimes it was used correctly, some other times it was used as "not NoAutoMount" i.e. "AutoMount" enabled. Fix this consistently throughout the source, and fix also some comments. --- drivers/storage/mountmgr/database.c | 10 +++--- drivers/storage/mountmgr/device.c | 48 ++++++++++++++--------------- drivers/storage/mountmgr/mntmgr.h | 8 +---- drivers/storage/mountmgr/mountmgr.c | 33 +++++++++----------- 4 files changed, 44 insertions(+), 55 deletions(-) diff --git a/drivers/storage/mountmgr/database.c b/drivers/storage/mountmgr/database.c index 118d37d1cfd..9801d708ba0 100644 --- a/drivers/storage/mountmgr/database.c +++ b/drivers/storage/mountmgr/database.c @@ -1641,12 +1641,12 @@ ReconcileThisDatabaseWithMaster(IN PDEVICE_EXTENSION DeviceExtension, WorkItem->DeviceInformation = DeviceInformation; QueueWorkItem(DeviceExtension, WorkItem, &(WorkItem->DeviceExtension)); - /* If there's no automount, and automatic letters - * all volumes to find those online and notify there presence - */ + /* If the worker thread isn't started yet, automatic drive letter is + * enabled but automount disabled, manually set mounted volumes online. + * Otherwise, they will be set online during database reconciliation. */ if (DeviceExtension->WorkerThreadStatus == 0 && - DeviceExtension->AutomaticDriveLetter == 1 && - DeviceExtension->NoAutoMount == FALSE) + DeviceExtension->AutomaticDriveLetter && + DeviceExtension->NoAutoMount) { OnlineMountedVolumes(DeviceExtension, DeviceInformation); } diff --git a/drivers/storage/mountmgr/device.c b/drivers/storage/mountmgr/device.c index 4bf053b62d3..ca8ed5c1395 100644 --- a/drivers/storage/mountmgr/device.c +++ b/drivers/storage/mountmgr/device.c @@ -94,7 +94,6 @@ MountmgrWriteNoAutoMount(IN PDEVICE_EXTENSION DeviceExtension) REG_DWORD, &Value, sizeof(Value)); - } /* @@ -115,16 +114,17 @@ MountMgrSetAutoMount(IN PDEVICE_EXTENSION DeviceExtension, return STATUS_INVALID_PARAMETER; } - /* Only change if there's a real difference */ + /* Change the state only if there is a real difference + * with the user-provided NewState (normalized) */ SetState = (PMOUNTMGR_SET_AUTO_MOUNT)Irp->AssociatedIrp.SystemBuffer; - if (SetState->NewState == !DeviceExtension->NoAutoMount) + if ((SetState->NewState != Enabled) == DeviceExtension->NoAutoMount) { Irp->IoStatus.Information = 0; return STATUS_SUCCESS; } - /* Set new state; ! on purpose */ - DeviceExtension->NoAutoMount = !SetState->NewState; + /* Set new state */ + DeviceExtension->NoAutoMount = (SetState->NewState != Enabled); Irp->IoStatus.Information = 0; return MountmgrWriteNoAutoMount(DeviceExtension); } @@ -480,7 +480,7 @@ MountMgrNextDriveLetterWorker(IN PDEVICE_EXTENSION DeviceExtension, return STATUS_OBJECT_NAME_NOT_FOUND; } - /* Now, mark we have assigned a letter (assumption) */ + /* Now, assume we will assign a letter */ DeviceInformation->LetterAssigned = DriveLetterInfo->DriveLetterWasAssigned = TRUE; @@ -501,39 +501,35 @@ MountMgrNextDriveLetterWorker(IN PDEVICE_EXTENSION DeviceExtension, NextEntry = NextEntry->Flink; } - /* If we didn't find a drive letter online - * ensure this is not a no drive entry - * by querying GPT attributes & database - */ + /* If we didn't find a drive letter online, ensure this is not + * a no-drive entry by querying GPT attributes & database */ if (NextEntry == &(DeviceInformation->SymbolicLinksListHead)) { if (!GptDriveLetter || HasNoDriveLetterEntry(DeviceInformation->UniqueId)) { DriveLetterInfo->DriveLetterWasAssigned = FALSE; DriveLetterInfo->CurrentDriveLetter = 0; - goto Release; } } - /* No, ensure that the device is not automounted nor removable */ - if (!DeviceExtension->NoAutoMount && !Removable) + /* If automount is disabled, and the device is not removable + * but needs a drive letter, don't assign one and bail out */ + if (DeviceExtension->NoAutoMount && !Removable) { if (DriveLetterInfo->DriveLetterWasAssigned) { DriveLetterInfo->DriveLetterWasAssigned = FALSE; DriveLetterInfo->CurrentDriveLetter = 0; - goto Release; } } + /* Stop now if we don't need to assign the drive a letter */ if (!DriveLetterInfo->DriveLetterWasAssigned) - { goto Release; - } - /* Now everything is fine, start processing */ + /* Now everything is fine, begin drive letter assignment */ if (RtlPrefixUnicodeString(&DeviceFloppy, &TargetDeviceName, TRUE)) { @@ -559,7 +555,6 @@ MountMgrNextDriveLetterWorker(IN PDEVICE_EXTENSION DeviceExtension, { DriveLetterInfo->DriveLetterWasAssigned = FALSE; DriveLetterInfo->CurrentDriveLetter = 0; - goto Release; } @@ -775,7 +770,7 @@ MountMgrAssignDriveLetters(IN PDEVICE_EXTENSION DeviceExtension) &DriveLetterInformation); } - /* If it was the system volume */ + /* If it's the system volume */ if (NT_SUCCESS(Status) && RtlEqualUnicodeString(&SystemVolumeName, &(DeviceInformation->DeviceName), TRUE)) { /* Keep track of it */ @@ -788,16 +783,19 @@ MountMgrAssignDriveLetters(IN PDEVICE_EXTENSION DeviceExtension) DeviceInformation->UniqueId->UniqueIdLength + sizeof(MOUNTDEV_UNIQUE_ID)); } - /* If it was not automount, ensure it gets mounted */ - if (!DeviceExtension->NoAutoMount) + /* Ensure it gets mounted if automount is disabled */ + if (DeviceExtension->NoAutoMount) { - DeviceExtension->NoAutoMount = TRUE; + /* Temporarily re-enable automount for the + * worker to mount and set a drive letter */ + DeviceExtension->NoAutoMount = FALSE; MountMgrNextDriveLetterWorker(DeviceExtension, &(DeviceInformation->DeviceName), &DriveLetterInformation); - DeviceExtension->NoAutoMount = FALSE; + /* And re-disable automount */ + DeviceExtension->NoAutoMount = TRUE; } } } @@ -2700,12 +2698,12 @@ MountMgrDeviceControl(IN PDEVICE_OBJECT DeviceObject, break; case IOCTL_MOUNTMGR_AUTO_DL_ASSIGNMENTS: + // NOTE: On Win7+, this is handled during driver re-initialization. DeviceExtension->AutomaticDriveLetter = TRUE; - Status = STATUS_SUCCESS; - MountMgrAssignDriveLetters(DeviceExtension); ReconcileAllDatabasesWithMaster(DeviceExtension); WaitForOnlinesToComplete(DeviceExtension); + Status = STATUS_SUCCESS; break; case IOCTL_MOUNTMGR_VOLUME_MOUNT_POINT_CREATED: diff --git a/drivers/storage/mountmgr/mntmgr.h b/drivers/storage/mountmgr/mntmgr.h index 77c7f8672cf..22e56a1e01b 100644 --- a/drivers/storage/mountmgr/mntmgr.h +++ b/drivers/storage/mountmgr/mntmgr.h @@ -19,7 +19,7 @@ typedef struct _DEVICE_EXTENSION PVOID NotificationEntry; KSEMAPHORE DeviceLock; KSEMAPHORE RemoteDatabaseLock; - ULONG AutomaticDriveLetter; + BOOLEAN AutomaticDriveLetter; LIST_ENTRY IrpListHead; ULONG EpicNumber; LIST_ENTRY SavedLinksListHead; @@ -240,12 +240,6 @@ HasDriveLetter( IN PDEVICE_INFORMATION DeviceInformation ); -CODE_SEG("INIT") -BOOLEAN -MountmgrReadNoAutoMount( - IN PUNICODE_STRING RegistryPath -); - /* database.c */ extern PWSTR DatabasePath; diff --git a/drivers/storage/mountmgr/mountmgr.c b/drivers/storage/mountmgr/mountmgr.c index a1d1ab23ff1..ac8eed375f9 100644 --- a/drivers/storage/mountmgr/mountmgr.c +++ b/drivers/storage/mountmgr/mountmgr.c @@ -835,26 +835,27 @@ MountMgrUnload(IN PDRIVER_OBJECT DriverObject) IoDeleteDevice(gdeviceObject); } -/* - * @implemented - */ +/** + * @brief Retrieves the "NoAutoMount" setting. + * @return TRUE if AutoMount is disabled; FALSE if AutoMount is enabled. + **/ CODE_SEG("INIT") BOOLEAN -MountmgrReadNoAutoMount(IN PUNICODE_STRING RegistryPath) +MountmgrReadNoAutoMount( + _In_ PUNICODE_STRING RegistryPath) { NTSTATUS Status; ULONG Result, Default = 0; RTL_QUERY_REGISTRY_TABLE QueryTable[2]; + /* Retrieve data from registry */ RtlZeroMemory(QueryTable, sizeof(QueryTable)); - - /* Simply read data from register */ QueryTable[0].Flags = RTL_QUERY_REGISTRY_DIRECT; QueryTable[0].Name = L"NoAutoMount"; QueryTable[0].EntryContext = &Result; - QueryTable[0].DefaultType = REG_NONE; + QueryTable[0].DefaultType = REG_DWORD; QueryTable[0].DefaultData = &Default; - QueryTable[0].DefaultLength = sizeof(ULONG); + QueryTable[0].DefaultLength = sizeof(Default); Status = RtlQueryRegistryValues(RTL_REGISTRY_ABSOLUTE, RegistryPath->Buffer, @@ -862,9 +863,7 @@ MountmgrReadNoAutoMount(IN PUNICODE_STRING RegistryPath) NULL, NULL); if (!NT_SUCCESS(Status)) - { - return (Default != 0); - } + Result = Default; return (Result != 0); } @@ -1258,7 +1257,7 @@ MountMgrMountedDeviceArrival(IN PDEVICE_EXTENSION DeviceExtension, DeviceInformation->SuggestedDriveLetter = 0; } /* Else, it's time to set up one */ - else if ((DeviceExtension->NoAutoMount || DeviceInformation->Removable) && + else if ((!DeviceExtension->NoAutoMount || DeviceInformation->Removable) && DeviceExtension->AutomaticDriveLetter && (HasGptDriveLetter || DeviceInformation->SuggestedDriveLetter) && !HasNoDriveLetterEntry(UniqueId)) @@ -1316,16 +1315,14 @@ MountMgrMountedDeviceArrival(IN PDEVICE_EXTENSION DeviceExtension, RtlCopyMemory(NewUniqueId->UniqueId, UniqueId->UniqueId, UniqueId->UniqueIdLength); } - /* If device's offline or valid, skip its notifications */ + /* If the device is offline or valid, skip its notifications */ if (IsOff || Valid) { DeviceInformation->SkipNotifications = TRUE; } - /* In case device is valid and is set to no automount, - * set it offline. - */ - if (DeviceExtension->NoAutoMount || IsDrvLetter) + /* If automount is enabled or the device already mounted, set it offline */ + if (!DeviceExtension->NoAutoMount || IsDrvLetter) { IsOff = !DeviceInformation->SkipNotifications; } @@ -1337,7 +1334,7 @@ MountMgrMountedDeviceArrival(IN PDEVICE_EXTENSION DeviceExtension, /* Finally, release the exclusive lock */ KeReleaseSemaphore(&(DeviceExtension->DeviceLock), IO_NO_INCREMENT, 1, FALSE); - /* If device is not offline, notify its arrival */ + /* If the device is not offline, notify its arrival */ if (!IsOff) { SendOnlineNotification(SymbolicName);