binary1230 / virtualbox-bug-7861-repro

Uses vagrant to repro a vboxfs bug in https://www.virtualbox.org/ticket/8761
0 stars 0 forks source link

bugfix notes #1

Open binary1230 opened 5 years ago

binary1230 commented 5 years ago

linux kernel module for this https://www.virtualbox.org/browser/vbox/trunk/src/VBox/Additions/linux/sharedfolders

binary1230 commented 4 years ago

https://github.com/mdaniel/virtualbox-org-svn-vbox-trunk

leads: https://elixir.bootlin.com/linux/latest/source/fs/vboxsf/vboxsf_wrappers.c#L179

SHFL_FN_RENAME return vboxsf_call(SHFL_FN_RENAME, &parms, SHFL_CPARMS_RENAME, NULL);

https://elixir.bootlin.com/linux/latest/source/fs/vboxsf/vboxsf_wrappers.c#L53

idea on fix, patch the rename() call to do something different like a copy and delete? less efficient but maybe don't have to actually fix the real bug. https://ruinedsec.wordpress.com/2013/04/04/modifying-system-calls-dispatching-linux/

binary1230 commented 4 years ago

src/VBox/HostServices/SharedFolders/VBoxSharedFoldersSvc.cpp has:

static DECLCALLBACK(void) svcCall (void *, VBOXHGCMCALLHANDLE callHandle, uint32_t u32ClientID, void *pvClient, uint32_t u32Function, uint32_t cParms, VBOXHGCMSVCPARM paParms[], uint64_t tsArrival)
{
...
...
switch (u32Function)
    {
...

        case SHFL_FN_RENAME:
        {
            pStat     = &g_StatRename;
            pStatFail = &g_StatRenameFail;
            Log(("SharedFolders host service: svcCall: SHFL_FN_RENAME\n"));

            /* Verify parameter count and types. */
            if (cParms != SHFL_CPARMS_RENAME)
            {
                rc = VERR_INVALID_PARAMETER;
            }
            else if (   paParms[0].type != VBOX_HGCM_SVC_PARM_32BIT   /* root */
                     || paParms[1].type != VBOX_HGCM_SVC_PARM_PTR   /* src */
                     || paParms[2].type != VBOX_HGCM_SVC_PARM_PTR   /* dest */
                     || paParms[3].type != VBOX_HGCM_SVC_PARM_32BIT /* flags */
                    )
            {
                rc = VERR_INVALID_PARAMETER;
            }
            else
            {
                /* Fetch parameters. */
                SHFLROOT    root        = (SHFLROOT)paParms[0].u.uint32;
                SHFLSTRING *pSrc        = (SHFLSTRING *)paParms[1].u.pointer.addr;
                SHFLSTRING *pDest       = (SHFLSTRING *)paParms[2].u.pointer.addr;
                uint32_t    flags       = paParms[3].u.uint32;

                /* Verify parameters values. */
                if (    !ShflStringIsValidIn(pSrc, paParms[1].u.pointer.size, RT_BOOL(pClient->fu32Flags & SHFL_CF_UTF8))
                    ||  !ShflStringIsValidIn(pDest, paParms[2].u.pointer.size, RT_BOOL(pClient->fu32Flags & SHFL_CF_UTF8))
                   )
                {
                    rc = VERR_INVALID_PARAMETER;
                }
                else
                {
                    /* Execute the function. */
                    rc = vbsfRename (pClient, root, pSrc, pDest, flags);
                    if (RT_SUCCESS(rc))
                    {
                        /* Update parameters.*/
                        ; /* none */
                    }
                }
            }
            break;
        }
binary1230 commented 4 years ago

src/VBox/HostServices/SharedFolders/vbsf.cpp

int vbsfRename(SHFLCLIENTDATA *pClient, SHFLROOT root, SHFLSTRING *pSrc, SHFLSTRING *pDest, uint32_t flags)
{
    int rc = VINF_SUCCESS;

    /* Validate input */
    if (   flags & ~(SHFL_RENAME_FILE|SHFL_RENAME_DIR|SHFL_RENAME_REPLACE_IF_EXISTS)
        || pSrc == 0
        || pDest == 0)
    {
        AssertFailed();
        return VERR_INVALID_PARAMETER;
    }

    /* Build a host full path for the given path
     * and convert ucs2 to utf8 if necessary.
     */
    char *pszFullPathSrc = NULL;
    char *pszFullPathDest = NULL;

    rc = vbsfBuildFullPath(pClient, root, pSrc, pSrc->u16Size + SHFLSTRING_HEADER_SIZE, &pszFullPathSrc, NULL);
    if (rc != VINF_SUCCESS)
        return rc;

    rc = vbsfBuildFullPath(pClient, root, pDest, pDest->u16Size + SHFLSTRING_HEADER_SIZE, &pszFullPathDest, NULL, false, true);
    if (RT_SUCCESS (rc))
    {
        Log(("Rename %s to %s\n", pszFullPathSrc, pszFullPathDest));

        /* is the guest allowed to write to this share? */
        bool fWritable;
        rc = vbsfMappingsQueryWritable(pClient, root, &fWritable);
        if (RT_FAILURE(rc) || !fWritable)
            rc = VERR_WRITE_PROTECT;

        if (RT_SUCCESS(rc))
        {
            if ((flags & (SHFL_RENAME_FILE | SHFL_RENAME_DIR)) == (SHFL_RENAME_FILE | SHFL_RENAME_DIR))
            {
                rc = RTPathRename(pszFullPathSrc, pszFullPathDest,
                                  flags & SHFL_RENAME_REPLACE_IF_EXISTS ? RTPATHRENAME_FLAGS_REPLACE : 0);
            }
            else if (flags & SHFL_RENAME_FILE)
            {
                rc = RTFileMove(pszFullPathSrc, pszFullPathDest,
                                ((flags & SHFL_RENAME_REPLACE_IF_EXISTS) ? RTFILEMOVE_FLAGS_REPLACE : 0));
            }
            else
            {
                /* NT ignores the REPLACE flag and simply return and already exists error. */
                rc = RTDirRename(pszFullPathSrc, pszFullPathDest,
                                 ((flags & SHFL_RENAME_REPLACE_IF_EXISTS) ? RTPATHRENAME_FLAGS_REPLACE : 0));
            }
#ifndef RT_OS_WINDOWS
            if (   rc == VERR_FILE_NOT_FOUND
                && SHFL_CLIENT_NEED_WINDOWS_ERROR_STYLE_ADJUST_ON_POSIX(pClient)
                && vbsfErrorStyleIsWindowsPathNotFound2(pszFullPathSrc, pszFullPathDest))
                rc = VERR_PATH_NOT_FOUND;
#endif
        }

        /* free the path string */
        vbsfFreeFullPath(pszFullPathDest);
    }
    /* free the path string */
    vbsfFreeFullPath(pszFullPathSrc);
    return rc;
}
binary1230 commented 4 years ago


RTR3DECL(int) RTFileRename(const char *pszSrc, const char *pszDst, unsigned fRename)
{
    /*
     * Validate input.
     */
    AssertMsgReturn(VALID_PTR(pszSrc), ("%p\n", pszSrc), VERR_INVALID_POINTER);
    AssertMsgReturn(VALID_PTR(pszDst), ("%p\n", pszDst), VERR_INVALID_POINTER);
    AssertMsgReturn(*pszSrc, ("%p\n", pszSrc), VERR_INVALID_PARAMETER);
    AssertMsgReturn(*pszDst, ("%p\n", pszDst), VERR_INVALID_PARAMETER);
    AssertMsgReturn(!(fRename & ~RTPATHRENAME_FLAGS_REPLACE), ("%#x\n", fRename), VERR_INVALID_PARAMETER);

    /*
     * Take common cause with RTPathRename.
     */
    int rc = rtPathPosixRename(pszSrc, pszDst, fRename, RTFS_TYPE_FILE);

    LogFlow(("RTDirRename(%p:{%s}, %p:{%s}, %#x): returns %Rrc\n",
             pszSrc, pszSrc, pszDst, pszDst, fRename, rc));
    return rc;
}```

rtPathPosixRename gets down to the metal