Skip to content

Commit

Permalink
[core] work around a Windows bug where GetVolumePathNamesForVolumeNam…
Browse files Browse the repository at this point in the history
…e() can return the wrong drive letter

* A user is reporting that, on one of their platforms, Rufus is writing to the wrong target during the file-copy
  phase and using their existing Y: local drive instead of the drive associated to the USB, despite the fact
  that Rufus is passing the right volume name to GetVolumePathNamesForVolumeName().

* Here's the PowerShell wmic output, confirming that the volume GUID obtained by Rufus is the right one:

  DriveLetter : Y:
  DeviceId    : \\?\Volume{000349b1-17d0-69f6-c13f-f31162930600}\
  Capacity    : 118540464128
  FileSystem  : NTFS
  Label       : Y-DISK

  DriveLetter : H:
  DeviceId    : \\?\Volume{b150ff4a-d62b-11ea-86e3-f49634660e54}\
  Capacity    : 15791824896
  FileSystem  : FAT32
  Label       : ADATA16GB

* And here's the Rufus log demonstrating that GetVolumePathNamesForVolumeName() is returning the *WRONG* letter:

  Found volume \\?\Volume{b150ff4a-d62b-11ea-86e3-f49634660e54}\
  \\?\Volume{b150ff4a-d62b-11ea-86e3-f49634660e54}\ is already mounted as Y: instead of H: - Will now use this target instead...

* The last line shows, without the shadow of a doubt, that we did feed "\\?\Volume{b150ff4a-d62b-11ea-86e3-f49634660e54}\" to
  GetVolumePathNamesForVolumeName() and that this API call was successful (returned a non zero size) but ultimately returned
  the wrong letter (Y: instead of H:)...

* Therefore, Windows is BUGGY and the use of GetVolumePathNamesForVolumeName() must be avoided.
  • Loading branch information
pbatard committed Aug 13, 2020
1 parent c2017ad commit ba40684
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 9 deletions.
8 changes: 8 additions & 0 deletions src/drive.c
Original file line number Diff line number Diff line change
Expand Up @@ -1522,12 +1522,19 @@ BOOL MountVolume(char* drive_name, char *volume_name)
{
char mounted_guid[52];
char mounted_letter[27] = { 0 };
#if defined(WINDOWS_IS_NOT_BUGGY)
DWORD size;
#endif

if ((drive_name == NULL) || (volume_name == NULL) || (drive_name[0] == '?') ||
(strncmp(volume_name, groot_name, groot_len) == 0))
return FALSE;

// Great: Windows has a *MAJOR BUG* whereas, in some circumstances, GetVolumePathNamesForVolumeName()
// can return the *WRONG* drive letter. And yes, we validated that this is *NOT* an issue like stack
// or buffer corruption and whatnot. It *IS* a Windows bug. So just drop the idea of updating the
// drive letter if already mounted and use the passed target always.
#if defined(WINDOWS_IS_NOT_BUGGY)
// Windows may already have the volume mounted but under a different letter.
// If that is the case, update drive_name to that letter.
if ( (GetVolumePathNamesForVolumeNameA(volume_name, mounted_letter, sizeof(mounted_letter), &size))
Expand All @@ -1537,6 +1544,7 @@ BOOL MountVolume(char* drive_name, char *volume_name)
drive_name[0] = mounted_letter[0];
return TRUE;
}
#endif

if (!SetVolumeMountPointA(drive_name, volume_name)) {
if (GetLastError() == ERROR_DIR_NOT_EMPTY) {
Expand Down
2 changes: 1 addition & 1 deletion src/format.c
Original file line number Diff line number Diff line change
Expand Up @@ -1732,7 +1732,7 @@ DWORD WINAPI FormatThread(void* param)
}
if (drive_letters[0] == 0) {
uprintf("No drive letter was assigned...");
drive_name[0] = GetUnusedDriveLetter();
drive_name[0] = GetUnusedDriveLetter();
if (drive_name[0] == 0) {
uprintf("Could not find a suitable drive letter");
FormatStatus = ERROR_SEVERITY_ERROR|FAC(FACILITY_STORAGE)|APPERR(ERROR_CANT_ASSIGN_LETTER);
Expand Down
6 changes: 3 additions & 3 deletions src/rufus.c
Original file line number Diff line number Diff line change
Expand Up @@ -3479,12 +3479,12 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine
SendMessage(hMainDialog, WM_COMMAND, IDC_LOG, 0);
continue;
}
#if defined(_DEBUG)
#if defined(_DEBUG) || defined(TEST)
// Ctrl-T => Alternate Test mode that doesn't require a full rebuild
if ((ctrl_without_focus || ((GetKeyState(VK_CONTROL) & 0x8000) && (msg.message == WM_KEYDOWN)))
&& (msg.wParam == 'T')) {
extern int TestChecksum(void);
TestChecksum();
//extern int TestChecksum(void);
//TestChecksum();
continue;
}
#endif
Expand Down
10 changes: 5 additions & 5 deletions src/rufus.rc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
IDD_DIALOG DIALOGEX 12, 12, 232, 326
STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU
EXSTYLE WS_EX_ACCEPTFILES
CAPTION "Rufus 3.12.1693"
CAPTION "Rufus 3.12.1694"
FONT 9, "Segoe UI Symbol", 400, 0, 0x0
BEGIN
LTEXT "Drive Properties",IDS_DRIVE_PROPERTIES_TXT,8,6,53,12,NOT WS_GROUP
Expand Down Expand Up @@ -395,8 +395,8 @@ END
//

VS_VERSION_INFO VERSIONINFO
FILEVERSION 3,12,1693,0
PRODUCTVERSION 3,12,1693,0
FILEVERSION 3,12,1694,0
PRODUCTVERSION 3,12,1694,0
FILEFLAGSMASK 0x3fL
#ifdef _DEBUG
FILEFLAGS 0x1L
Expand All @@ -414,13 +414,13 @@ BEGIN
VALUE "Comments", "https://rufus.ie"
VALUE "CompanyName", "Akeo Consulting"
VALUE "FileDescription", "Rufus"
VALUE "FileVersion", "3.12.1693"
VALUE "FileVersion", "3.12.1694"
VALUE "InternalName", "Rufus"
VALUE "LegalCopyright", "© 2011-2020 Pete Batard (GPL v3)"
VALUE "LegalTrademarks", "https://www.gnu.org/licenses/gpl-3.0.html"
VALUE "OriginalFilename", "rufus-3.12.exe"
VALUE "ProductName", "Rufus"
VALUE "ProductVersion", "3.12.1693"
VALUE "ProductVersion", "3.12.1694"
END
END
BLOCK "VarFileInfo"
Expand Down

0 comments on commit ba40684

Please sign in to comment.