Shared Folders implementation have a bug with access rights for reading file attributes

[sporaw]

Shared Folders implementation have a bug with access rights for reading file attributes

Host: Windows Guest: Windows

When you are trying from the GUEST machine to open files on HOST-shared folder for reading attributes with the rights [FILE_READ_ATTRIBUTES | SYNCHRONIZE] in practice (on the HOST machine) it tries to open files with [GENERIC_READ] right, so the code that shown below is not working correctly (the second try to open a file will fail with SHARING_VIOLATION error):

Request for reading file attributes is differ from a simple file reading, because in case of attribute reading "file share reading" must be ignored.

Here is a sample program:

• when all is correct it must print Le1 = 0, Le2 = 0
• if you will try to sumbit a file that is located on shared folder, the second open will fail with sharing violation error.

#include <stdio.h>
#include <windows.h>

int main(int argc, char *argv[])
{
    if (argc < 2)
    {
        printf("ERROR: No filename specified!\n");
        return -1;
    }

    HANDLE h1 = CreateFile(argv[1], GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL);
    printf("Le1 = %u\n", GetLastError());

    HANDLE h2 = CreateFile(argv[1], FILE_READ_ATTRIBUTES | SYNCHRONIZE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, 0, NULL);
    printf("Le2 = %u\n", GetLastError());

    if (h1 != INVALID_HANDLE_VALUE)
    {
        CloseHandle(h1);
    }

    if (h2 != INVALID_HANDLE_VALUE)
    {
        CloseHandle(h2);
    }

    return 0;
}

To checkout access rights and flags of opening file on HOST machine and GUEST machine you can use ProcessMonitor utility: ​https://technet.microsoft.com/en-us/library/bb896645.aspx

Here is a sample of wrong work: On HOST machine:

17:54:16.1144379	VirtualBox.exe	3196	IRP_MJ_CREATE	D:\tmp\file.pdb	SHARING VIOLATION	Desired Access: Generic Read, Disposition: Open, Options: Synchronous IO Non-Alert, Non-Directory File, Attributes: N, ShareMode: Read, Write, AllocationSize: n/a

But inside the VM (GUEST machine) it was opened with following rights/flags:

18:09:16,6181478	mspdbsrv.exe	2472	IRP_MJ_CREATE	\\VBOXSVR\shared\file.pdb	SHARING VIOLATION	Desired Access: Read Attributes, Synchronize, Disposition: Open, Options: Synchronous IO Non-Alert, Non-Directory File, Attributes: n/a, ShareMode: Read, Write, Delete, AllocationSize: n/a

What must be fixed (here is a code that must be fixed/modified) -- all modifications are marked with "<=" sign:

1) file: .\src\VBox\HostServices\SharedFolders\vbsf.cpp

function: static int vbsfConvertFileOpenFlags(unsigned fShflFlags, RTFMODE fMode, SHFLHANDLE handleInitial, uint32_t *pfOpen)

        case SHFL_CF_ACCESS_NONE:
        {
            /** @todo treat this as read access, but theoretically this could be a no access request. */
            fOpen |= RTFILE_O_READ;  <========= (!) This must be deleted; opening file for reading attributes is NOT a reading a file!
            Log(("FLAG: SHFL_CF_ACCESS_NONE\n"));
            break;
        }

2) file: .\src\VBox\Runtime\r3\fileio.cpp

function: int rtFileRecalcAndValidateFlags(uint64_t *pfOpen)

    switch (fOpen & RTFILE_O_ACCESS_MASK)
    {
        <= (!) add:
        <= case 0: break; 0 - allowed value (file is opened to access attributes, but not for reading/writing a data)
        case RTFILE_O_READ:
            fOpen |= g_fOpenReadSet;
            fOpen &= ~g_fOpenReadMask;
            break;
        case RTFILE_O_WRITE:
            fOpen |= g_fOpenWriteSet;
            fOpen &= ~g_fOpenWriteMask;
            break;
        case RTFILE_O_READWRITE:
            fOpen |= g_fOpenReadWriteSet;
            fOpen &= ~g_fOpenReadWriteMask;
            break;
        default:
            AssertMsgFailed(("Invalid RW value, fOpen=%#llx\n", fOpen));
            return VERR_INVALID_PARAMETER;
    }

3) file: .\src\VBox\Runtime\r3\win\fileio-win.cpp

function: RTR3DECL(int)·RTFileOpen(PRTFILE·pFile,·const·char·*pszFilename,·uint64_t·fOpen)

    DWORD dwDesiredAccess; <=========== (!) add = 0 (init value)
    switch (fOpen & RTFILE_O_ACCESS_MASK)
    {
        <= (!) add:
        <= case 0: break; 0 - allowed value (file is opened to access attributes, but not for reading/writing a data)
	case RTFILE_O_READ:
            dwDesiredAccess = FILE_GENERIC_READ; /* RTFILE_O_APPEND is ignored. */
            break;
        case RTFILE_O_WRITE:
            dwDesiredAccess = fOpen & RTFILE_O_APPEND
                            ? FILE_GENERIC_WRITE & ~FILE_WRITE_DATA
                            : FILE_GENERIC_WRITE;
            break;
        case RTFILE_O_READWRITE:
            dwDesiredAccess = fOpen & RTFILE_O_APPEND
                            ? FILE_GENERIC_READ | (FILE_GENERIC_WRITE & ~FILE_WRITE_DATA)
                            : FILE_GENERIC_READ | FILE_GENERIC_WRITE;
            break;
        default:
            AssertMsgFailed(("Impossible fOpen=%#llx\n", fOpen));
            return VERR_INVALID_PARAMETER;
    }

[Frank Mehnert]

Your changes are almost correct. We only need to restrict the changes to Windows hosts. Therefore your example will still not work on non-Windows hosts (e.g. Linux). See r58768, r58769 and r58770. This build contains the fixes. Is there any chance that you install this build and test if your example works as intended? Thank you!

[Frank Mehnert]

And here is the corresponding Extension Pack if you need it.

[sporaw]

VirtualBox v5.0.51-104214 + Guest add-on

Guest OS: Windows 2003

Test sample, that I provided in this ticket, does not working with fix in test build:

C:\>test.exe X:\share\halmacpi.dll
Le1 = 0
Le2 = 87

You can use a test utility by yourself (source included in ticket).

In your patch you made some strange flags, so it does not equal to mine patch in results. (Yes, I understand about Linux version, but logic of fix was modified)

[Frank Mehnert]

There was an additional fix required. The latest Windows test build from here (>= revision 104286) contains the fix. It should now work with your example, please confirm.

[sporaw]

Now it's fixed (tested, all is fine), thank you.

[Frank Mehnert]

Thank you for the confirmation!

[Frank Mehnert]

Fix is part of VBox 5.0.12.