From cd5665881ce1d39dd2923f4b81dd0b255d77cce1 Mon Sep 17 00:00:00 2001 From: Pete Batard Date: Tue, 25 Jun 2013 02:55:25 +0100 Subject: [PATCH] [core] drive handling improvements * Use IOCTL_DISK_UPDATE_PROPERTIES after partitioning * Use IOCTL_DISK_DELETE_DRIVE_LAYOUT to invalidate partitions before formatting * Fix handling of unpartitioned drives * Increase delay after partitioning * All of the above should help with the infamous #122 * Also fix display of error messages in ms-sys' file.c as well as stdio.c * Also add commandline option -f to list fixed drives --- src/drive.c | 45 +++++++++++++++++++++-- src/format.c | 92 +++++++++++++++++++++++++++++++++++------------ src/ms-sys/file.c | 16 ++++----- src/rufus.c | 12 ++++--- src/rufus.h | 1 + src/rufus.rc | 10 +++--- src/stdio.c | 59 +++++++++++++++--------------- 7 files changed, 163 insertions(+), 72 deletions(-) diff --git a/src/drive.c b/src/drive.c index 9b550cd1..af817788 100644 --- a/src/drive.c +++ b/src/drive.c @@ -202,12 +202,18 @@ out: } /* - * Return a handle to the first logical volume on the disk identified by DriveIndex + * Obtain a handle to the first logical volume on the disk identified by DriveIndex + * Returns INVALID_HANDLE_VALUE on error or NULL if no logical path exists (typical + * of unpartitioned drives) */ HANDLE GetLogicalHandle(DWORD DriveIndex, BOOL bWriteAccess, BOOL bLockDrive) { HANDLE hLogical = INVALID_HANDLE_VALUE; char* LogicalPath = GetLogicalName(DriveIndex, FALSE); + if (LogicalPath == NULL) { + return NULL; + } + hLogical = GetHandle(LogicalPath, bWriteAccess, bLockDrive); safe_free(LogicalPath); return hLogical; @@ -490,8 +496,9 @@ typedef struct _DRIVE_LAYOUT_INFORMATION_EX4 { * Create a partition table * See http://technet.microsoft.com/en-us/library/cc739412.aspx for some background info * NB: if you modify the MBR outside of using the Windows API, Windows still uses the cached - * copy it got from the last IOCTL, and ignore your changes until you replug the drive... - */ + * copy it got from the last IOCTL, and ignores your changes until you replug the drive + * or issue an IOCTL_DISK_UPDATE_PROPERTIES. + */ #if !defined(PARTITION_BASIC_DATA_GUID) const GUID PARTITION_BASIC_DATA_GUID = { 0xebd0a0a2, 0xb9e5, 0x4433, {0x87, 0xc0, 0x68, 0xb6, 0xb7, 0x26, 0x99, 0xc7} }; #endif @@ -627,6 +634,38 @@ BOOL CreatePartition(HANDLE hDrive, int partition_style, int file_system, BOOL m safe_closehandle(hDrive); return FALSE; } + // Diskpart does call the following IOCTL this after updating the partition table, so we do too + r = DeviceIoControl(hDrive, IOCTL_DISK_UPDATE_PROPERTIES, NULL, 0, NULL, 0, &size, NULL ); + if (!r) { + uprintf("Could not refresh drive layout: %s\n", WindowsErrorString()); + safe_closehandle(hDrive); + return FALSE; + } + + return TRUE; +} + +/* Delete the disk partition table */ +BOOL DeletePartitions(HANDLE hDrive) +{ + BOOL r; + DWORD size; + + PrintStatus(0, TRUE, "Erasing Partitions..."); + + r = DeviceIoControl(hDrive, IOCTL_DISK_DELETE_DRIVE_LAYOUT, NULL, 0, NULL, 0, &size, NULL ); + if (!r) { + uprintf("Could not delete drive layout: %s\n", WindowsErrorString()); + safe_closehandle(hDrive); + return FALSE; + } + + r = DeviceIoControl(hDrive, IOCTL_DISK_UPDATE_PROPERTIES, NULL, 0, NULL, 0, &size, NULL ); + if (!r) { + uprintf("Could not refresh drive layout: %s\n", WindowsErrorString()); + safe_closehandle(hDrive); + return FALSE; + } return TRUE; } diff --git a/src/format.c b/src/format.c index f1f6eb31..534c3043 100644 --- a/src/format.c +++ b/src/format.c @@ -372,12 +372,14 @@ static BOOL FormatFAT32(DWORD DriveIndex) // Open the drive (volume should already be locked) hLogicalVolume = GetLogicalHandle(DriveIndex, TRUE, FALSE); if (IS_ERROR(FormatStatus)) goto out; - if (hLogicalVolume == INVALID_HANDLE_VALUE) - die("Could not access logical volume\n", ERROR_OPEN_FAILED); + if ((hLogicalVolume == INVALID_HANDLE_VALUE) || (hLogicalVolume == NULL)) + die("Invalid logical volume handle\n", ERROR_INVALID_HANDLE); // Make sure we get exclusive access - if (!UnmountVolume(hLogicalVolume)) - return r; + if (!UnmountVolume(hLogicalVolume)) { + FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_ACCESS_DENIED; + goto out; + } // Work out drive params if (!DeviceIoControl (hLogicalVolume, IOCTL_DISK_GET_DRIVE_GEOMETRY, NULL, 0, &dgDrive, @@ -1088,19 +1090,45 @@ out: return r; } +/* + * Mount the volume identified by drive_guid to mountpoint drive_name + */ +static BOOL MountVolume(char* drive_name, char *drive_guid) +{ + char mounted_guid[50]; + + if (!SetVolumeMountPointA(drive_name, drive_guid)) { + // If the OS was faster than us at remounting the drive, this operation can fail + // with ERROR_DIR_NOT_EMPTY. If that's the case, just check that mountpoints match + if (GetLastError() == ERROR_DIR_NOT_EMPTY) { + if (!GetVolumeNameForVolumeMountPointA(drive_name, mounted_guid, sizeof(mounted_guid))) { + uprintf("%s already mounted, but volume GUID could not be checked\n", drive_name); + return FALSE; + } + if (safe_strcmp(drive_guid, mounted_guid) != 0) { + uprintf("%s already mounted, but volume GUID doesn't match:\r\n expected %s, got %s\n", + drive_name, drive_guid, mounted_guid); + return FALSE; + } + uprintf("%s was already mounted as %s\n", drive_guid, drive_name); + } else { + return FALSE; + } + } + return TRUE; +} + /* * Issue a complete remount of the volume */ -static BOOL RemountVolume(char drive_letter) +static BOOL RemountVolume(char* drive_name) { char drive_guid[50]; - char drive_name[] = "?:\\"; - drive_name[0] = drive_letter; if (GetVolumeNameForVolumeMountPointA(drive_name, drive_guid, sizeof(drive_guid))) { if (DeleteVolumeMountPointA(drive_name)) { Sleep(200); - if (SetVolumeMountPointA(drive_name, drive_guid)) { + if (MountVolume(drive_name, drive_guid)) { uprintf("Successfully remounted %s on %s\n", &drive_guid[4], drive_name); } else { uprintf("Failed to remount %s on %s\n", &drive_guid[4], drive_name); @@ -1212,22 +1240,39 @@ DWORD WINAPI FormatThread(LPVOID param) uprintf("Failed to delete mountpoint %s: %s\n", drive_name, WindowsErrorString()); // Try to continue. We will bail out if this causes an issue. } - uprintf("Will use '%c': as volume mountpoint\n", drive_name[0]); + uprintf("Will use '%c:' as volume mountpoint\n", drive_name[0]); + // ...but we need a lock to the logical drive to be able to write anything to it + // TODO: Use a retry loop for the lock hLogicalVolume = GetLogicalHandle(DriveIndex, FALSE, TRUE); if (hLogicalVolume == INVALID_HANDLE_VALUE) { uprintf("Could not lock volume\n"); FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_OPEN_FAILED; goto out; + } else if (hLogicalVolume == NULL) { + // NULL is returned for cases where the drive is not yet partitioned + uprintf("Drive does not appear to be partitioned\n"); + } else { + if (!UnmountVolume(hLogicalVolume)) + uprintf("Trying to continue regardless...\n"); } - UnmountVolume(hLogicalVolume); if (FormatStatus) goto out; // Check for user cancel PrintStatus(0, TRUE, "Analyzing existing boot records...\n"); AnalyzeMBR(hPhysicalDrive); - AnalyzePBR(hLogicalVolume); + if (hLogicalVolume != NULL) { + AnalyzePBR(hLogicalVolume); + } UpdateProgress(OP_ANALYZE_MBR, -1.0f); + // Zap any existing partitions. This should help to prevent access errors + // TODO: With this, we should be able to avoid having to deal with the logical volume above + if (!DeletePartitions(hPhysicalDrive)) { + uprintf("Could not reset partitions\n"); + FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_PARTITION_FAILURE; + goto out; + } + if (IsChecked(IDC_BADBLOCKS)) { do { // create a log file for bad blocks report. Since %USERPROFILE% may @@ -1290,11 +1335,13 @@ DWORD WINAPI FormatThread(LPVOID param) } // Close the (unmounted) volume before formatting, but keep the lock // According to MS this relinquishes the lock, so... - PrintStatus(0, TRUE, "Closing existing volume...\n"); - if (!CloseHandle(hLogicalVolume)) { - uprintf("Could not close volume: %s\n", WindowsErrorString()); - FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_ACCESS_DENIED; - goto out; + if (hLogicalVolume != NULL) { + PrintStatus(0, TRUE, "Closing existing volume...\n"); + if (!CloseHandle(hLogicalVolume)) { + uprintf("Could not close volume: %s\n", WindowsErrorString()); + FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_ACCESS_DENIED; + goto out; + } } hLogicalVolume = INVALID_HANDLE_VALUE; @@ -1318,8 +1365,9 @@ DWORD WINAPI FormatThread(LPVOID param) } UpdateProgress(OP_PARTITION, -1.0f); - // Add a small delay after partitioning to be safe - Sleep(200); + // Add a delay after partitioning to be safe + // TODO: Use a retry loop and test the lock + Sleep(2000); // If FAT32 is requested and we have a large drive (>32 GB) use // large FAT32 format, else use MS's FormatEx. @@ -1350,7 +1398,7 @@ DWORD WINAPI FormatThread(LPVOID param) } if (FormatStatus) goto out; // Check for user cancel - if (!SetVolumeMountPointA(drive_name, guid_volume)) { + if (!MountVolume(drive_name, guid_volume)) { uprintf("Could not remount %s on %s: %s\n", guid_volume, drive_name, WindowsErrorString()); FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|APPERR(ERROR_CANT_MOUNT_VOLUME); goto out; @@ -1368,7 +1416,7 @@ DWORD WINAPI FormatThread(LPVOID param) // We still have a lock, which we need to modify the volume boot record // => no need to reacquire the lock... hLogicalVolume = GetLogicalHandle(DriveIndex, TRUE, FALSE); - if (hLogicalVolume == INVALID_HANDLE_VALUE) { + if ((hLogicalVolume == INVALID_HANDLE_VALUE) || (hLogicalVolume == NULL)) { uprintf("Could not re-mount volume for partition boot record access\n"); FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|ERROR_OPEN_FAILED; goto out; @@ -1397,7 +1445,7 @@ DWORD WINAPI FormatThread(LPVOID param) // We issue a complete remount of the filesystem at on account of: // - Ensuring the file explorer properly detects that the volume was updated // - Ensuring that an NTFS system will be reparsed so that it becomes bootable - if (!RemountVolume(drive_name[0])) + if (!RemountVolume(drive_name)) goto out; if (FormatStatus) goto out; // Check for user cancel @@ -1449,7 +1497,7 @@ DWORD WINAPI FormatThread(LPVOID param) if (IsChecked(IDC_SET_ICON)) SetAutorun(drive_name); // Issue another complete remount before we exit, to ensure we're clean - RemountVolume(drive_name[0]); + RemountVolume(drive_name); // NTFS fixup (WinPE/AIK images don't seem to boot without an extra checkdisk) if ((dt == DT_ISO) && (fs == FS_NTFS)) { CheckDisk(drive_name[0]); diff --git a/src/ms-sys/file.c b/src/ms-sys/file.c index 0d89cca1..d0776857 100644 --- a/src/ms-sys/file.c +++ b/src/ms-sys/file.c @@ -41,15 +41,15 @@ int64_t write_sectors(HANDLE hDrive, uint64_t SectorSize, ptr.QuadPart = StartSector*SectorSize; if(!SetFilePointerEx(hDrive, ptr, NULL, FILE_BEGIN)) { - uprintf("write_sectors: Could not access sector %d - %s\n", StartSector, WindowsErrorString()); + uprintf("write_sectors: Could not access sector 0x%08llx - %s\n", StartSector, WindowsErrorString()); return -1; } if((!WriteFile(hDrive, pBuf, Size, &Size, NULL)) || (Size != nSectors*SectorSize)) { - uprintf("write_sectors: Write error - %s\n", WindowsErrorString()); - uprintf(" Wrote: %d, Expected: %d\n", Size, nSectors*SectorSize); - uprintf(" StartSector:%0X, nSectors:%0X, SectorSize:%0X\n", StartSector, nSectors, SectorSize); + uprintf("write_sectors: Write error %s\n", (GetLastError()!=ERROR_SUCCESS)?WindowsErrorString():""); + uprintf(" Wrote: %d, Expected: %lld\n", Size, nSectors*SectorSize); + uprintf(" StartSector: 0x%08llx, nSectors: 0x%llx, SectorSize: 0x%llx\n", StartSector, nSectors, SectorSize); return Size; } @@ -74,15 +74,15 @@ int64_t read_sectors(HANDLE hDrive, uint64_t SectorSize, ptr.QuadPart = StartSector*SectorSize; if(!SetFilePointerEx(hDrive, ptr, NULL, FILE_BEGIN)) { - uprintf("read_sectors: Could not access sector %d - %s\n", StartSector, WindowsErrorString()); + uprintf("read_sectors: Could not access sector 0x%08llx - %s\n", StartSector, WindowsErrorString()); return -1; } if((!ReadFile(hDrive, pBuf, Size, &Size, NULL)) || (Size != nSectors*SectorSize)) { - uprintf("read_sectors: Read error - %s\n", WindowsErrorString()); - uprintf(" Read: %d, Expected: %d\n", Size, nSectors*SectorSize); - uprintf(" StartSector:%0X, nSectors:%0X, SectorSize:%0X\n", StartSector, nSectors, SectorSize); + uprintf("read_sectors: Read error %s\n", (GetLastError()!=ERROR_SUCCESS)?WindowsErrorString():""); + uprintf(" Read: %d, Expected: %lld\n", Size, nSectors*SectorSize); + uprintf(" StartSector: 0x%08llx, nSectors: 0x%llx, SectorSize: 0x%llx\n", StartSector, nSectors, SectorSize); } return (int64_t)Size; diff --git a/src/rufus.c b/src/rufus.c index da464996..2e54b5d4 100644 --- a/src/rufus.c +++ b/src/rufus.c @@ -624,7 +624,7 @@ static BOOL GetUSBDevices(DWORD devnum) if(GetLastError() != ERROR_NO_MORE_ITEMS) { uprintf("SetupDiEnumDeviceInterfaces failed: %s\n", WindowsErrorString()); } else { - uprintf("A device was eliminated because it didn't report itself as a disk\n"); + uprintf("A device was eliminated because it didn't report itself as a removable disk\n"); } break; } @@ -1252,7 +1252,7 @@ static BOOL BootCheck(void) "- Select 'Yes' to connect to the internet and download the file\n" "- Select 'No' if you will manually copy this file on the drive later\n\n" "Note: The file will be downloaded in the current directory and once a " - "'%s' exists there, it will be reused automatically.\n", ldlinux_c32, ldlinux_c32, ldlinux_c32); + "'%s' exists there, it will be reused automatically.\n", ldlinux_c32, ldlinux_c32); safe_sprintf(msgbox_title, sizeof(msgbox_title), "Download %s?", ldlinux_c32); r = MessageBoxU(hMainDialog, msgbox, msgbox_title, MB_YESNOCANCEL|MB_ICONWARNING); if (r == IDCANCEL) @@ -1819,8 +1819,7 @@ static INT_PTR CALLBACK MainCallback(HWND hDlg, UINT message, WPARAM wParam, LPA SendMessage(hProgress, PBM_SETSTATE, (WPARAM)PBST_ERROR, 0); SetTaskbarProgressState(TASKBAR_ERROR); PrintStatus(0, FALSE, "FAILED"); - Notification(MSG_ERROR, NULL, "Error", "Error: %s.%s", StrError(FormatStatus), - (strchr(StrError(FormatStatus), '\n') != NULL)?"":"\nFor more information, please check the log."); + Notification(MSG_ERROR, NULL, "Error", "Error: %s", StrError(FormatStatus)); } FormatStatus = 0; format_op_in_progress = FALSE; @@ -1919,8 +1918,11 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine wait_for_mutex = 150; // Try to acquire the mutex for 15 seconds } - while ((opt = getopt_long(argc, argv, "?hi:w:", long_options, &option_index)) != EOF) + while ((opt = getopt_long(argc, argv, "?fhi:w:", long_options, &option_index)) != EOF) switch (opt) { + case 'f': + enable_fixed_disks = TRUE; + break; case 'i': if (_access(optarg, 0) != -1) { iso_path = safe_strdup(optarg); diff --git a/src/rufus.h b/src/rufus.h index 7619404f..004317ad 100644 --- a/src/rufus.h +++ b/src/rufus.h @@ -307,6 +307,7 @@ extern HANDLE GetLogicalHandle(DWORD DriveIndex, BOOL bWriteAccess, BOOL bLockDr extern char GetDriveLetter(DWORD DriveIndex); extern char GetUnusedDriveLetter(void); extern BOOL CreatePartition(HANDLE hDrive, int partition_style, int file_system, BOOL mbr_uefi_marker); +extern BOOL DeletePartitions(HANDLE hDrive); extern const char* GetPartitionType(BYTE Type); extern BOOL GetDrivePartitionData(DWORD DriveIndex, char* FileSystemName, DWORD FileSystemNameSize); extern BOOL GetDriveLabel(DWORD DriveIndex, char* letter, char** label); diff --git a/src/rufus.rc b/src/rufus.rc index 0a6ca5d0..c40966f1 100644 --- a/src/rufus.rc +++ b/src/rufus.rc @@ -30,7 +30,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL IDD_DIALOG DIALOGEX 12, 12, 206, 329 STYLE DS_SETFONT | DS_MODALFRAME | DS_FIXEDSYS | DS_CENTER | WS_POPUP | WS_CAPTION | WS_SYSMENU EXSTYLE WS_EX_APPWINDOW -CAPTION "Rufus v1.3.4.257" +CAPTION "Rufus v1.3.4.258" FONT 8, "MS Shell Dlg", 400, 0, 0x1 BEGIN DEFPUSHBUTTON "Start",IDC_START,94,291,50,14 @@ -278,8 +278,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 1,3,4,257 - PRODUCTVERSION 1,3,4,257 + FILEVERSION 1,3,4,258 + PRODUCTVERSION 1,3,4,258 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L @@ -296,13 +296,13 @@ BEGIN BEGIN VALUE "CompanyName", "Akeo Consulting (http://akeo.ie)" VALUE "FileDescription", "Rufus" - VALUE "FileVersion", "1.3.4.257" + VALUE "FileVersion", "1.3.4.258" VALUE "InternalName", "Rufus" VALUE "LegalCopyright", "© 2011-2013 Pete Batard (GPL v3)" VALUE "LegalTrademarks", "http://www.gnu.org/copyleft/gpl.html" VALUE "OriginalFilename", "rufus.exe" VALUE "ProductName", "Rufus" - VALUE "ProductVersion", "1.3.4.257" + VALUE "ProductVersion", "1.3.4.258" END END BLOCK "VarFileInfo" diff --git a/src/stdio.c b/src/stdio.c index a7bbfca0..e8e81a33 100644 --- a/src/stdio.c +++ b/src/stdio.c @@ -219,60 +219,61 @@ const char* StrError(DWORD error_code) } switch (SCODE_CODE(error_code)) { case ERROR_GEN_FAILURE: - return "Undetermined error while formatting"; + return "Undetermined error while formatting."; case ERROR_INCOMPATIBLE_FS: - return "Cannot use the selected file system for this media"; + return "Cannot use the selected file system for this media."; case ERROR_ACCESS_DENIED: - return "Access to the media is denied"; + return "Access to the device is denied."; case ERROR_WRITE_PROTECT: - return "Media is write protected"; + return "Media is write protected."; case ERROR_DEVICE_IN_USE: - return "The device is in use by another process\n" - "Please close any other process that may be accessing the device"; + return "The device is in use by another process. " + "Please close any other process that may be accessing the device."; case ERROR_CANT_QUICK_FORMAT: - return "Quick format is not available for this device"; + return "Quick format is not available for this device."; case ERROR_LABEL_TOO_LONG: - return "The volume label is invalid"; + return "The volume label is invalid."; + case ERROR_INVALID_HANDLE: + return "The device handle is invalid."; case ERROR_INVALID_CLUSTER_SIZE: - return "The selected cluster size is not valid for this device"; + return "The selected cluster size is not valid for this device."; case ERROR_INVALID_VOLUME_SIZE: - return "The volume size is invalid"; + return "The volume size is invalid."; case ERROR_NO_MEDIA_IN_DRIVE: - return "Please insert a media in drive"; + return "Please insert a media in drive."; case ERROR_NOT_SUPPORTED: - return "An unsupported command was received"; + return "An unsupported command was received."; case ERROR_NOT_ENOUGH_MEMORY: - return "Memory allocation error"; + return "Memory allocation error."; case ERROR_READ_FAULT: - return "Read error"; + return "Read error."; case ERROR_WRITE_FAULT: - return "Write error"; + return "Write error."; case ERROR_OPEN_FAILED: - return "Could not open media. It may be in use by another process.\n" - "Please re-plug the media and try again"; + return "Could not open media. It may be in use by another process. " + "Please re-plug the media and try again."; case ERROR_PARTITION_FAILURE: - return "Error while partitioning drive"; + return "Error while partitioning drive."; case ERROR_CANNOT_COPY: - return "Could not copy files to target drive"; + return "Could not copy files to target drive."; case ERROR_CANCELLED: - return "Cancelled by user"; + return "Cancelled by user."; case ERROR_CANT_START_THREAD: - return "Unable to create formatting thread"; + return "Unable to create formatting thread."; case ERROR_BADBLOCKS_FAILURE: - return "Bad blocks check didn't complete"; + return "Bad blocks check didn't complete."; case ERROR_ISO_SCAN: - return "ISO image scan failure"; + return "ISO image scan failure."; case ERROR_ISO_EXTRACT: - return "ISO image extraction failure"; + return "ISO image extraction failure."; case ERROR_CANT_REMOUNT_VOLUME: - return "Unable to remount volume. You may have to use the\n" - "mountvol.exe command to make your device accessible again"; + return "Unable to remount volume."; case ERROR_CANT_PATCH: - return "Unable to patch/setup files for boot"; + return "Unable to patch/setup files for boot."; case ERROR_CANT_ASSIGN_LETTER: - return "Unable to assign a drive letter"; + return "Unable to assign a drive letter."; case ERROR_CANT_MOUNT_VOLUME: - return "Can't mount GUID volume"; + return "Can't mount GUID volume."; default: uprintf("Unknown error: %08X\n", error_code); SetLastError(error_code);