[Bug 54243] New: Warcraft 2 BNE: Fails CD check in wine versions >6.0.X

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54243

Bug ID: 54243
           Summary: Warcraft 2 BNE: Fails CD check in wine versions >6.0.X
           Product: Wine
           Version: 7.0
          Hardware: x86-64
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: kernel32
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

SYMPTOM: Starting with wine versions mid-6.X and including all 7.X through at
least 7.22, the game "Warcraft 2: Battle.net Edition" always fails a CD check
and pops up an error dialog:

"Warcraft II Battle.net Edition is unable to read a required file. Your
Warcraft II Battle.net Edition CD may not be in the CDROM drive. Please ensure
that the disk in in the CDROM drive and press OK. To leave the program, press
Exit."

----

BACKGROUND:

Wine had long been able to run "Warcraft 2: Battle.net Edition", up through
wine version 6.0.2 and some early 6.X versions.

As described in the howto of the application DB
https://appdb.winehq.org/objectManager.php?sClass=version&iId=592 (and other
places), in my setup I have long used an .iso of my original CD, with symlinks
for "d::" (linked to the iso) and "d:" (linked to the directory where the .iso
is loop-mounted).  (I actually use some wrapper scripts to temporarily mount it
only when needed, etc...)

I unsuccessfully asked about this in the wine forum last July
https://forum.winehq.org/viewtopic.php?f=8&t=36787&sid=e1b75b6a5ffa1...
and am finally digging deeper into it on my own.  There are also some
tangential mentions in my recent Gentoo Linux forum post:
https://forums.gentoo.org/viewtopic-p-8766703.html

----

ANALYSIS:

I have managed to git bisect this bug to commit
50903a15046354e405564aff6430ee505c01100a "kernelbase: Reimplement
GetVolumeInformation on top of GetVolumeInformationByHandle" (although I needed
to cherry-pick an unrelated "sincos()" build fix at each step...).

Digging deeper, the following sequence of events leads to the bug:

1. The game calls GetVolumeInformationW() with root=L"D:\".
     - The game only wants file system flags and FS type name.
     - Wine proceeds to:
 2. NtOpenFile() the root (I haven't traced into this much)...
 3. GetVolumeInformationByHandleW()
 4. NtQueryVolumeInformationFile() with
    info_class=FileFsAttributeInformation
 5. get_mountmgr_fs_info(), which determines
    unix_name="/archive/wineData/warcraftII-BNE-2001.iso"
    and letter=25 (Z:).
     - BUG: letter=25 definitely indicates a bug.  The game is asking
       about D:, not Z:.
     - MAYBE BUG: Not sure whether mapping "D:\"'s handle to the .iso
       rather than the loop mount point directory is a bug or not.
     - FYI: I'll note that NtQueryVolumeInformationFile()'s documentation
       says that if the handle "represents a direct device open, only
       FileFsDeviceInformation can be specified..." in the remarks (i.e.,
       not the info_class being asked for here).  There doesn't seem
       to be any attempt to enforce this in wine.  See

https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/nf-ntif...
 6. Since wine now thinks the game is asking about "Z:", it returns file
    system type "NTFS" and associated flags, rather than the correct
    "CDFS" (and flags).
 7. Presumably the game sees the "NTFS" and/or flags, decides this
    isn't a CD at all, and after some more searching, ultimately
    pops up the error dialog.

----

POTENTIAL FIXES:

I'm not sure the best way to fix this.  Some possibilities include:

A. Somehow ensure the handle that GetVolumeInformation() opens gets
    mapped to the mounted directory, instead of the .iso file?
     - This would seem to make sense, but I'm not sure.
 B. Add more logic to find_dos_device() to notice if path is a file
    (or device) that is pointed at by one of the double-colon symlinks?
     - It might need similar caching as get_drives_info(), etc?
       This would be just a little involved, but might be the best
       "smallish scale" fix.
 C. Just revert commit 50903a15046354e405564aff6430ee505c01100a?
    But that might leave GetVolumeInformationByHandle() broken,
    and it does seem cleaner to only do all the device/FS
    superblock parsing and determination in one place...
 D. Do a more thorough audit / cleanup of how different ways of
    representing and mapping between unix names, NT names, devices,
    mount points, symlinks, fallback logic, etc all interact with
    each other?  My initial vague impression is that a lot of this
    logic is a bit haphazard and only "mostly" works, rather than
    being robustly designed to always work.
     - Very involved and time consuming, and well beyond anything
       I currently have any interest in attempting myself.

----

WORKAROUND:

A quick end-user workaround that seems to work is to change the "d::" symlink
to point to the correct /dev/loop0 or loop1, or 2...., that just happens to
match whatever free loop device that "mount" dynamically picks to use.
  - I think this only works because these days /dev is usually
    mounted as a separate file system, and in wine there is
    find_dos_device() logic to stops looking if the st_dev changes
    and then later stuff falls back on doing something else...
  - Even disregarding that, it seems a bit ugly and fragile to
    assume a particular loop device, especially if you have other
    things you sometimes loop mount...  (Although maybe my wrapper
    script could dynamically update the symlink based on parsing
    /proc/mounts after things are mounted...)