Crash on VirtualPC

Please report only one bug per message!

Moderators: Stefan2, white, sheep, Hacker

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2014-01-07, 14:09 UTC

Well, this function was created specifically for checking version of mrxvpc.sys, so all the code is required. The algorithm is:

Initialization:

1) We check for existence of GetSystemWindowsDirectoryA API function.
2) We get the system Windows folder with GetSystemDirectoryA (in most cases: C:\WINDOWS\System32).
3) We get the root Windows folder with GetSystemWindowsDirectoryA / GetWindowsDirectoryA (in most cases: C:\WINDOWS).
4) We get the drive letter of Windows partition.

We will use all this information in the main loop. Main loop enumerates all drivers that are loaded into memory and searches for mrxvpc.sys. Required APIs for driver enumeration are located in PSAPI.dll:

5) We load PSAPI.dll to ensure, that it will be loaded into address space of our process - so next calls to PSAPI functions will be successful (these calls are in fact wrappers around LoadLibrary('PSAPI.dll') and GetProcAddress).
6) We get the size of required memory buffer with EnumDeviceDrivers(nil,0,...).
7) We allocate the memory buffer with GetMem and call EnumDeviceDrivers again to get the information about all drivers.
8) The main loop ("for"): we call GetDeviceDriverBaseNameA and check with AnsiCompareFileName if it is mrxvpc.sys.

If we found mrxvpc.sys, there is only one more thing to do: check its version. We can't do this directly, by reading its memory, because it exists in kernel memory range, so it's unavailable for us. So we must use GetFileVersion API with a correct path. But we can get a driver file name only with GetDeviceDriverFileNameA, and it may return the file name in various formats. Driver file name may start with '\??\', '\SystemRoot\' or '\'. So we use information from steps 1)..4) to convert these kernel mode paths into a file system path and finally call GetFileVersion.

User avatar
ghisler(Author)
Site Admin
Site Admin
Posts: 36554
Joined: 2003-02-04, 09:46 UTC
Location: Switzerland
Contact:

Post by *ghisler(Author) » 2014-01-07, 17:06 UTC

Ah, sorry, I wasn't clear - I just use this code:

Code: Select all

  S:=SystemDirectory+'Drivers\mrxvpc.sys';
  Version:=GetFileVersion(S);
  if Version=high(Version) then
    exit;      {driver not found}
  Result:=Version < $000D0334;
Why do all this enumeration stuff instead of just checking the file directly?
Author of Total Commander
http://www.ghisler.com

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2014-01-07, 23:02 UTC

The enumeration returns information about loaded drivers, so we check if mrxvpc.sys is currently loaded. It also maps a driver name (GetDeviceDriverBaseNameA) to its file name (GetDeviceDriverFileNameA). Without using enumeration we can check version of the driver file with GetFileVersion, but we can't determine if it is currently loaded - for example it may be not loaded when the operating system has been booted in safe mode. And enumeration also returns a path to the driver file (although in kernel-mode namespace) - so the "S:=SystemDirectory+'Drivers\mrxvpc.sys'" clause is in fact the "last resort" clause and it will never be called in practice - it would be called only when the driver information in system registry had been damaged.

And the enumeration shouldn't slow down the application in any visible manner. So the designed APIs for getting driver information should be preferred, and hardcoded paths as "S:=SystemDirectory+'Drivers\mrxvpc.sys'" should be used only for "last resort" cases.

User avatar
ghisler(Author)
Site Admin
Site Admin
Posts: 36554
Joined: 2003-02-04, 09:46 UTC
Location: Switzerland
Contact:

Post by *ghisler(Author) » 2014-01-08, 16:22 UTC

My reasoning is that the file itself is only there when inside of VirtualPC. If it is, and it is the older problematic version, then I hook the functions. In the rare case where the dll is there but not loaded (safe mode), it doesn't do any harm to hook the functions too.
Author of Total Commander
http://www.ghisler.com

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2014-01-12, 14:13 UTC

The function we are talking about has name IsBuggyVirtualPCDriverLoaded, so it checks if the buggy driver is currently loaded. If you prefer just to check if the buggy driver is installed, the function will be in fact much simpler:

Code: Select all

{$IFNDEF WIN32}
function IsBuggyVirtualPCDriverInstalled : Boolean;
begin
  Result:=False;
end;
{$ELSE}
function IsBuggyVirtualPCDriverInstalled : Boolean;

  function GetFileVersion(const FileName : AnsiString) : Cardinal;
  var
    FileNameCopy : AnsiString;
    VersionInfoSize : Cardinal;
    Temp : UINT;
    Buffer : Pointer;
    VSFixedFileInfo : PVSFixedFileInfo;
  begin
    Result:=High(Result);

    {GetFileVersionInfoA/W may modify the filename parameter,
     so we create a local, writable copy of FileName}
    FileNameCopy:=FileName;
    UniqueString(FileNameCopy);

    VersionInfoSize:=GetFileVersionInfoSizeA(PAnsiChar(FileNameCopy),Temp);
    if VersionInfoSize > 0 then
    begin
      GetMem(Buffer,VersionInfoSize);
      try
        if GetFileVersionInfoA(PAnsiChar(FileNameCopy),0,VersionInfoSize,Buffer) then
        if VerQueryValueA(Buffer,'\',Pointer(VSFixedFileInfo),Temp) then
        if VSFixedFileInfo <> nil then
          Result:=VSFixedFileInfo^.dwFileVersionMS;
      finally
        FreeMem(Buffer);
      end;
    end;
  end;

{$IFDEF VER90} {Delphi 2}
  {In all places of the IncludeTrailingBackslash usage, input string comes from
   GetSystemDirectoryA call. This function always returns path without a backslash
   at the end, unless the result points to the root directory. So we can omit
   MBCS problems by checking the string length instead of checking if the last
   char is a backslash.}
  function IncludeTrailingBackslash(const S : AnsiString) : AnsiString;
  begin
    if Length(S) > 3 then
      Result:=S+'\'
    else
      Result:=S;
  end;
{$ENDIF}

var
  Len : UINT;
  SystemDirectory : AnsiString;
begin
  Result:=False;

  if Win32Platform <> VER_PLATFORM_WIN32_NT then
    Exit;

  SetLength(SystemDirectory,GetSystemDirectoryA(nil,0));
  Len:=GetSystemDirectoryA(PAnsiChar(SystemDirectory),Length(SystemDirectory));
  if (Len = 0) or (Len > UINT(Length(SystemDirectory))) then
    Exit;
  SetLength(SystemDirectory,Len);
  SystemDirectory:=AnsiString(IncludeTrailingBackslash(string(SystemDirectory)));

  {"Drivers" subdirectory name is hardcoded in setupapi.dll (in some Windows versions)
   and in ntoskrnl.exe (in all Windows versions, including 64-bit - ntoskrnl.exe loads
   drivers during booting)}
  Result:=GetFileVersion(SystemDirectory+'Drivers\mrxvpc.sys') < $000D0334;
end;
{$ENDIF}

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2014-02-08, 17:57 UTC

Recently, I reviewed my code - I was searching for certain kinds of bugs and also introduced some improvements (in particular, the goal was to make the code 64-bit compatible). So I also reviewed the code for VirtualPC patch and slightly updated it in my first post - below is a detailed list of changes.


1) Checking for existence of buggy driver (function IsBuggyVirtualPCDriverLoaded):

Code: Select all

  TPointerArray = array[0..High(Integer) div SizeOf(Pointer)-1] of Pointer;
=>
  TPointerArray = packed array[0..High(Integer) div SizeOf(Pointer)-1] of Pointer;
This is to strictly follow the EnumDeviceDrivers API specification and avoid potential problems with compilers other than Delphi.

Code: Select all

  PSAPIHandle : HMODULE;
  Buffer : array[0..MAX_COMPUTERNAME_LENGTH+MAX_PATH] of AnsiChar;
=>
  PSAPIHandle : HINST;
  Buffer : packed array[0..$8000-1] of AnsiChar;
PSAPIHandle definition was changed just for consistence - to use same types for same things everywhere (HINST and HMODULE are fully compatible each other). Buffer definition was changed to be on the safe side - we use it to get paths from NT kernel, which can be up to $7FFF chars + terminating null char = $8000 chars long.

Code: Select all

  GetSystemWindowsDirectoryA:=GetProcAddress(GetModuleHandle(kernel32),'GetSystemWindowsDirectoryA');
=>
  GetSystemWindowsDirectoryA:=GetProcAddress(GetModuleHandle(kernel32),PAnsiChar('GetSystemWindowsDirectoryA'));
and

Code: Select all

    GetSystemWindowsDirectoryA:=GetProcAddress(GetModuleHandle(kernel32),'GetWindowsDirectoryA');
=>
    GetSystemWindowsDirectoryA:=GetProcAddress(GetModuleHandle(kernel32),PAnsiChar('GetWindowsDirectoryA'));
These are optimizations for Delphi 2009 and up, which use unicode strings by default. GetProcAddress is a very unusual function - it has only an ANSI version named just 'GetProcAddress', there are no 'GetProcAddressA' and 'GetProcAddressW' functions. But unicode Delphi uses a unicode string as an input parameter, and then it implements a wrapper around GetProcAddress to convert this unicode string to ANSI string. By using PAnsiChar('...') typecast, we force the compiler to use an ANSI string directly, so the compiler can call Windows API directly, without any wrappers and string conversions.



2) Checking if drive is being shared by VirtualPC (function IsDriveSharedOnNTByVirtualPC):

Code: Select all

  Module : HMODULE;
=>
  Module : HINST;
Reason as above.

Code: Select all

  NtOpenSymbolicLinkObject:=GetProcAddress(Module,'NtOpenSymbolicLinkObject');
=>
  NtOpenSymbolicLinkObject:=GetProcAddress(Module,PAnsiChar('NtOpenSymbolicLinkObject'));
and

Code: Select all

  NtQuerySymbolicLinkObject:=GetProcAddress(Module,'NtQuerySymbolicLinkObject');
=>
  NtQuerySymbolicLinkObject:=GetProcAddress(Module,PAnsiChar('NtQuerySymbolicLinkObject'));
Reason as above.



3) PSAPI.pas:

Code: Select all

  hPSAPI: THandle;
=>
  hPSAPI: HINST;
THandle is not a valid type for module instances.

Code: Select all

    @_EnumDeviceDrivers := GetProcAddress(hPSAPI, 'EnumDeviceDrivers');
    @_GetDeviceDriverBaseNameA := GetProcAddress(hPSAPI, 'GetDeviceDriverBaseNameA');
    @_GetDeviceDriverFileNameA := GetProcAddress(hPSAPI, 'GetDeviceDriverFileNameA');
=>
    @_EnumDeviceDrivers := GetProcAddress(hPSAPI, PAnsiChar('EnumDeviceDrivers'));
    @_GetDeviceDriverBaseNameA := GetProcAddress(hPSAPI, PAnsiChar('GetDeviceDriverBaseNameA'));
    @_GetDeviceDriverFileNameA := GetProcAddress(hPSAPI, PAnsiChar('GetDeviceDriverFileNameA'));
Reason as above.



The optimization with GetProcAddress + PAnsiChar could also be used in other code in this thread: unit VirtualPCPatch.pas, program Loader.dpr, unit HookCreateFile.pas.

Regards

User avatar
ghisler(Author)
Site Admin
Site Admin
Posts: 36554
Joined: 2003-02-04, 09:46 UTC
Location: Switzerland
Contact:

Post by *ghisler(Author) » 2014-02-09, 14:41 UTC

Thanks, but I don't use 1, and 2+3 are not relevant for Delphi 2.
Author of Total Commander
http://www.ghisler.com

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2014-02-10, 14:02 UTC

This trick with GetProcAddress + PAnsiChar is in fact not a big optimization, but I suppose it may also be useful for 64-bit Lazarus.

Post Reply