[Bug 33232] New: Amazon MP3 Downloader crashes during download of mp3 files (ntdll/server fd cache doesn't take FILE_APPEND_DATA flag implicit write access into account)

http://bugs.winehq.org/show_bug.cgi?id=33232

Bug #: 33232
           Summary: Amazon MP3 Downloader crashes during download of mp3
                    files (ntdll/server fd cache doesn't take
                    FILE_APPEND_DATA flag implicit write access into
                    account)
           Product: Wine
           Version: 1.5.26
          Platform: x86
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ntdll
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
    Classification: Unclassified

Hello folks,

some time ago newer versions of "Amazon MP3 Downloader" application broke.
The app crashes while trying to download .mp3 files after purchase (during
processing of .amz).

Application log hints at a write error after receiving first data chunk:

--- snip ---
[AMD] [Download::OnCurlWriteData()] Output Stream failed to write, returning
error."
--- snip ---

It seems Wine ntdll/server fd cache only cares for access bit 0-1 when checking
for read-write permissions.

Debug session:

--- snip ---
Wine-dbg>bt
Backtrace:
=>0 0x7bc79c52 add_fd_to_cache(handle=0xe0, fd=0x22, type=FD_TYPE_FILE,
access=0x120114, options=0x60)
[/home/focht/projects/wine/wine-git/dlls/ntdll/server.c:470] in ntdll
(0x00b9e1e8)
  1 0x7bc43707 NtQueryInformationFile+0x192(hFile=<couldn't compute location>,
io=<couldn't compute location>, ptr=<couldn't compute location>, len=<couldn't
compute location>, class=<couldn't compute location>)
[/home/focht/projects/wine/wine-git/dlls/ntdll/file.c:1927] in ntdll
(0x00b9e408)
  2 0x7b83ee76 SetFilePointerEx+0xc2(hFile=<couldn't compute location>,
distance={u={LowPart=0, HighPart=0}, QuadPart=0}, newpos=<couldn't compute
location>, method=<couldn't compute location>)
[/home/focht/projects/wine/wine-git/dlls/kernel32/file.c:1066] in kernel32
(0x00b9e498)
  3 0x7b83ed6f SetFilePointer+0x69(hFile=<couldn't compute location>,
distance=<couldn't compute location>, highword=<couldn't compute location>,
method=<couldn't compute location>)
[/home/focht/projects/wine/wine-git/dlls/kernel32/file.c:1035] in kernel32
(0x00b9e508)
  4 0x006fe405 in amazonmp3downloader (+0x2fe404) (0x00b9e5b4)
  5 0x004b3cfa in amazonmp3downloader (+0xb3cf9) (0x00b9e5d0)
  6 0x0048e9b6 in amazonmp3downloader (+0x8e9b5) (0x00b9e62c)
  7 0x004abd42 in amazonmp3downloader (+0xabd41) (0x00b9e8a0)
  8 0x004acdd3 in amazonmp3downloader (+0xacdd2) (0x00b9e8ac)
  9 0x004b8863 in amazonmp3downloader (+0xb8862) (0x00b9e8b8)
  10 0x004cabe2 in amazonmp3downloader (+0xcabe1) (0x00b9e934)
  11 0x004ca51e in amazonmp3downloader (+0xca51d) (0x00b9e960)
  12 0x004ca416 in amazonmp3downloader (+0xca415) (0x00b9e9ac)
  13 0x007132f3 in amazonmp3downloader (+0x3132f2) (0x00b9e9d4)
  14 0x0064afd5 in amazonmp3downloader (+0x24afd4) (0x00b9ea0c)
  15 0x0064b0fd in amazonmp3downloader (+0x24b0fc) (0x00b9ea18)

Wine-dbg>info locals
0x7bc79dfd add_fd_to_cache+0x1ab: (00b9e098)
    HANDLE handle=0xe0 (parameter [EBP+8])
    int fd=0x22 (parameter [EBP+12])
    enum server_fd_type type=FD_TYPE_FILE (parameter [EBP+16])
    unsigned int access=0x120114 (parameter [EBP+20])
    unsigned int options=0x60 (parameter [EBP+24])
    unsigned int entry=0 (local [EBP-32])
    unsigned int idx=0x37 (local [EBP-12])
    int prev_fd=0xffffffff (local [EBP-28])
--- snip ---

An fd entry with access flags "0x120114" is added.

0x120114 (hard-coded in application code) decodes to:

FILE_APPEND_DATA        0x00000004
FILE_WRITE_EA           0x00000010
FILE_WRITE_ATTRIBUTES   0x00000100
READ_CONTROL            0x00020000 (STANDARD_RIGHTS_READ,
STANDARD_RIGHTS_WRITE, STANDARD_RIGHTS_EXECUTE)
SYNCHRONIZE             0x00100000

It didn't pass "FILE_WRITE_DATA" contained in FILE_GENERIC_WRITE (see end for
explanation):

--- snip ---
#define FILE_GENERIC_WRITE        (STANDARD_RIGHTS_WRITE | FILE_WRITE_DATA | \
                                   FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | \
                                   FILE_APPEND_DATA | SYNCHRONIZE)
--- snip ---

The fd was created here:

--- snip ---
...
0023:Call KERNEL32.CreateFileW(00cbcaf8 L"C:\users\focht\My Music\Amazon
MP3\Harry Gregson - Williams\D\00e9j\00e0
Vu\amazon.temp",00120114,00000003,00000000,00000004,00000080,00000000)
ret=006fe144 
...
0023: create_file( access=00120114, attributes=00000040, sharing=00000003,
create=3, options=00000060, attrs=00000080,
objattr={rootdir=0000,sd={},name=L""},
filename="/home/focht/.wine/dosdevices/c:/users/focht/My Music/Amazon MP3/Harry
Gregson - Williams/Déjà Vu/amazon.temp" ) 
...
0023: create_file() = 0 { handle=00f0 }
...
0023:Ret  KERNEL32.CreateFileW() retval=000000f0 ret=006fe144 
--- snip ---

The code that retrieves the cached fd only checks for FILE_READ_DATA |
FILE_WRITE_DATA set:

--- snip ---
Wine-dbg>bt
Backtrace:
=>0 0x7bc79e6e get_cached_fd+0x66(handle=0xe0, type=0xb9e008, access=0xb9de88,
options=0xb9e018) [/home/focht/projects/wine/wine-git/dlls/ntdll/server.c:515]
in ntdll (0x00b9dde8)
  1 0x7bc79fd8 server_get_unix_fd+0x78(handle=0xe0, wanted_access=0x2,
unix_fd=0xb9e020, needs_close=0xb9e01c, type=0xb9e008, options=0xb9e018)
[/home/focht/projects/wine/wine-git/dlls/ntdll/server.c:557] in ntdll
(0x00b9df38)
  2 0x7bc413d7 NtWriteFile+0x110(hFile=<couldn't compute location>,
hEvent=<couldn't compute location>, apc=<couldn't compute location>,
apc_user=<couldn't compute location>, io_status=<couldn't compute location>,
buffer=<couldn't compute location>, length=<couldn't compute location>,
offset=<couldn't compute location>, key=<couldn't compute location>)
[/home/focht/projects/wine/wine-git/dlls/ntdll/file.c:939] in ntdll
(0x00b9e088)
  3 0x7b83dfbc WriteFile+0x176(hFile=<couldn't compute location>,
buffer=<couldn't compute location>, bytesToWrite=<couldn't compute location>,
bytesWritten=<couldn't compute location>, overlapped=<couldn't compute
location>) [/home/focht/projects/wine/wine-git/dlls/kernel32/file.c:555] in
kernel32 (0x00b9e118)
  4 0x006fe6ca in amazonmp3downloader (+0x2fe6c9) (0x00b9e160)
  5 0x004b3bf3 in amazonmp3downloader (+0xb3bf2) (0x00b9e17c)
  6 0x0045c2a4 in amazonmp3downloader (+0x5c2a3) (0x00b9e1a8)
  7 0x0048e754 in amazonmp3downloader (+0x8e753) (0x00b9e1f0)
  8 0x0048d14a in amazonmp3downloader (+0x8d149) (0x00b9e274)
  9 0x00672a99 in amazonmp3downloader (+0x272a98) (0x00cdbad4)

Wine-dbg>info locals
0x7bc79ee1 get_cached_fd+0xd9: (00b9dde8)
    HANDLE handle=0xe0 (parameter [EBP+8])
    enum server_fd_type* type=0xb9e008 (parameter [EBP+12])
    unsigned int* access=0xb9de88 (parameter [EBP+16])
    unsigned int* options=0xb9e018 (parameter [EBP+20])
    unsigned int entry=0 (local [EBP-16])
    unsigned int idx=0x37 (local [EBP-12])
    int fd=0x22 (local [EBP-8])

Wine-dbg>p *access
0

Wine-dbg>n
582        if (!ret && ((access & wanted_access) != wanted_access))
--- snip ---

wanted access: FILE_WRITE_DATA
retrieved access: 0 (only bit 0-1 are preserved, all others got cut off when
the fd cache entry was created -> 2 bits reserved in fd cache entry).

So the WriteFile() call fails in the end.

Wine code:

http://source.winehq.org/git/wine.git/blob/0effd926b6475842baefc5379b9dd932c...

--- snip ---
 543 int server_get_unix_fd( HANDLE handle, unsigned int wanted_access, int
*unix_fd,
 544                         int *needs_close, enum server_fd_type *type,
unsigned int *options )
 545 {
 546     sigset_t sigset;
 547     obj_handle_t fd_handle;
 548     int ret = 0, fd;
 549     unsigned int access = 0;
 550 
 551     *unix_fd = -1;
 552     *needs_close = 0;
 553     wanted_access &= FILE_READ_DATA | FILE_WRITE_DATA;
 554 
 555     server_enter_uninterrupted_section( &fd_cache_section, &sigset );
 556 
 557     fd = get_cached_fd( handle, type, &access, options );
 558     if (fd != -1) goto done;
...
 580 done:
 581     server_leave_uninterrupted_section( &fd_cache_section, &sigset );
 582     if (!ret && ((access & wanted_access) != wanted_access))
 583     {
 584         ret = STATUS_ACCESS_DENIED;
 585         if (*needs_close) close( fd );
 586     }
 587     if (!ret) *unix_fd = fd;
 588     return ret;
 589 }
--- snip ---

So what did the application intend by _not_ passing FILE_WRITE_DATA?

Fortunately there is a hint hidden in the MSDN entry for CreateFile() that
explains it:

--- quote ---
You can get atomic append on local files by opening a file with
FILE_APPEND_DATA access and _without_ FILE_WRITE_DATA access. If you do this
then all writes will ignore the the current file pointer and be done at the
end-of file. (Actually, I'm not sure if the current file pointer is updated to
EOF at each write or not, I haven't tested that behavior.) Eg

HANDLE hFile = CreateFile(TEXT("c:\file.txt"), FILE_APPEND_DATA ,
FILE_SHARE_WRITE, 0, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0);

The append behavior is properly synchronized between multiple writes (with or
without multiple handles), where the typical way I've seen this implemented (by
seeking to EOF and then writing) has a race condition if multiple threads /
processes are appending to the same file.

This behavior is documented in Windows Driver Kit / Device and Driver
Technologies / Installable File System / Reference / IO Manager Routines /
IoCreateFileSpecifyDeviceObject (currently at
http://msdn2.microsoft.com/en-us/library/ms795642.aspx)
--- quote ---

If you fix that, the application no longer crashes and purchased mp3 files are
successfully downloaded (already tested).

$ du -sh AmazonMP3DownloaderInstall.exe 
2.2M    AmazonMP3DownloaderInstall.exe

$ sha1sum AmazonMP3DownloaderInstall.exe 
11b08d68fe11006ed402701a85183565ee8e139e  AmazonMP3DownloaderInstall.exe

$ wine --version
wine-1.5.26-19-g6ed2d9b

Regards