Index: /trunk/include/VBox/Graphics/VBoxVideoHost3D.h =================================================================== --- /trunk/include/VBox/Graphics/VBoxVideoHost3D.h (revision 71618) +++ /trunk/include/VBox/Graphics/VBoxVideoHost3D.h (revision 71619) @@ -42,5 +42,7 @@ typedef DECLCALLBACKPTR(void, PFNVBOXCRCMD_CLTSCR_UPDATE_BEGIN)(HVBOXCRCMDCLTSCR hClt, unsigned u32Screen); typedef DECLCALLBACKPTR(void, PFNVBOXCRCMD_CLTSCR_UPDATE_END)(HVBOXCRCMDCLTSCR hClt, unsigned uScreenId, int32_t x, int32_t y, uint32_t cx, uint32_t cy); -typedef DECLCALLBACKPTR(void, PFNVBOXCRCMD_CLTSCR_UPDATE_PROCESS)(HVBOXCRCMDCLTSCR hClt, unsigned u32Screen, const struct VBVACMDHDR *pCmd, size_t cbCmd); +typedef DECLCALLBACKPTR(void, PFNVBOXCRCMD_CLTSCR_UPDATE_PROCESS)(HVBOXCRCMDCLTSCR hClt, unsigned u32Screen, + struct VBVACMDHDR const RT_UNTRUSTED_VOLATILE_GUEST *pCmd, + size_t cbCmd); /*client callbacks to be used by the server Index: /trunk/include/VBox/vmm/pdmifs.h =================================================================== --- /trunk/include/VBox/vmm/pdmifs.h (revision 71618) +++ /trunk/include/VBox/vmm/pdmifs.h (revision 71619) @@ -903,5 +903,5 @@ */ DECLR3CALLBACKMEMBER(int, pfnVBVAEnable,(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId, - PVBVAHOSTFLAGS pHostFlags, bool fRenderThreadMode)); + struct VBVAHOSTFLAGS RT_UNTRUSTED_VOLATILE_GUEST *pHostFlags, bool fRenderThreadMode)); /** @@ -936,5 +936,5 @@ */ DECLR3CALLBACKMEMBER(void, pfnVBVAUpdateProcess,(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId, - PCVBVACMDHDR pCmd, size_t cbCmd)); + struct VBVACMDHDR const RT_UNTRUSTED_VOLATILE_GUEST *pCmd, size_t cbCmd)); /** Index: /trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp =================================================================== --- /trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp (revision 71618) +++ /trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp (revision 71619) @@ -26,4 +26,5 @@ #include #include +#include #include #include @@ -59,15 +60,14 @@ struct { - VBVABUFFER *pVBVA; /* Pointer to the guest memory with the VBVABUFFER. */ - uint8_t *pu8Data; /* For convenience, pointer to the guest ring buffer (VBVABUFFER::au8Data). */ + VBVABUFFER RT_UNTRUSTED_VOLATILE_GUEST *pVBVA; /**< Pointer to the guest memory with the VBVABUFFER. */ + uint8_t RT_UNTRUSTED_VOLATILE_GUEST *pu8Data; /**< For convenience, pointer to the guest ring buffer (VBVABUFFER::au8Data). */ } guest; - uint32_t u32VBVAOffset; /* VBVABUFFER offset in the guest VRAM. */ - VBVAPARTIALRECORD partialRecord; /* Partial record temporary storage. */ - uint32_t off32Data; /* The offset where the data starts in the VBVABUFFER. - * The host code uses it instead of VBVABUFFER::off32Data. - */ - uint32_t indexRecordFirst; /* Index of the first filled record in VBVABUFFER::aRecords. */ - uint32_t cbPartialWriteThreshold; /* Copy of VBVABUFFER::cbPartialWriteThreshold used by host code. */ - uint32_t cbData; /* Copy of VBVABUFFER::cbData used by host code. */ + uint32_t u32VBVAOffset; /**< VBVABUFFER offset in the guest VRAM. */ + VBVAPARTIALRECORD partialRecord; /**< Partial record temporary storage. */ + uint32_t off32Data; /**< The offset where the data starts in the VBVABUFFER. + * The host code uses it instead of VBVABUFFER::off32Data. */ + uint32_t indexRecordFirst; /**< Index of the first filled record in VBVABUFFER::aRecords. */ + uint32_t cbPartialWriteThreshold; /**< Copy of VBVABUFFER::cbPartialWriteThreshold used by host code. */ + uint32_t cbData; /**< Copy of VBVABUFFER::cbData used by host code. */ } VBVADATA; @@ -110,5 +110,6 @@ if (pVBVAData->guest.pVBVA) { - RT_ZERO(pVBVAData->guest.pVBVA->hostFlags); + pVBVAData->guest.pVBVA->hostFlags.u32HostEvents = 0; + pVBVAData->guest.pVBVA->hostFlags.u32SupportedOrders = 0; } @@ -119,8 +120,8 @@ } -/** Copies @a cb bytes from the VBVA ring buffer to the @a pu8Dst. +/** Copies @a cb bytes from the VBVA ring buffer to the @a pbDst. * Used for partial records or for records which cross the ring boundary. */ -static bool vbvaFetchBytes(VBVADATA *pVBVAData, uint8_t *pu8Dst, uint32_t cb) +static bool vbvaFetchBytes(VBVADATA *pVBVAData, uint8_t *pbDst, uint32_t cb) { if (cb >= pVBVAData->cbData) @@ -130,18 +131,18 @@ } + const uint8_t RT_UNTRUSTED_VOLATILE_GUEST *pbSrc = &pVBVAData->guest.pu8Data[pVBVAData->off32Data]; const uint32_t u32BytesTillBoundary = pVBVAData->cbData - pVBVAData->off32Data; - const uint8_t *pu8Src = &pVBVAData->guest.pu8Data[pVBVAData->off32Data]; - const int32_t i32Diff = cb - u32BytesTillBoundary; + const int32_t i32Diff = cb - u32BytesTillBoundary; if (i32Diff <= 0) { /* Chunk will not cross buffer boundary. */ - memcpy(pu8Dst, pu8Src, cb); + RT_BCOPY_VOLATILE(pbDst, pbSrc, cb); } else { /* Chunk crosses buffer boundary. */ - memcpy(pu8Dst, pu8Src, u32BytesTillBoundary); - memcpy(pu8Dst + u32BytesTillBoundary, &pVBVAData->guest.pu8Data[0], i32Diff); + RT_BCOPY_VOLATILE(pbDst, pbSrc, u32BytesTillBoundary); + RT_BCOPY_VOLATILE(pbDst + u32BytesTillBoundary, &pVBVAData->guest.pu8Data[0], i32Diff); } @@ -201,8 +202,9 @@ } -/* For contiguous chunks just return the address in the buffer. - * For crossing boundary - allocate a buffer from heap. +/** + * For contiguous chunks just return the address in the buffer. For crossing + * boundary - allocate a buffer from heap. */ -static bool vbvaFetchCmd(VBVADATA *pVBVAData, VBVACMDHDR **ppHdr, uint32_t *pcbCmd) +static bool vbvaFetchCmd(VBVADATA *pVBVAData, VBVACMDHDR RT_UNTRUSTED_VOLATILE_GUEST **ppHdr, uint32_t *pcbCmd) { VBVAPARTIALRECORD *pPartialRecord = &pVBVAData->partialRecord; @@ -307,5 +309,5 @@ /* The pointer to data in the ring buffer. */ - uint8_t *pu8Src = &pVBVAData->guest.pu8Data[pVBVAData->off32Data]; + uint8_t RT_UNTRUSTED_VOLATILE_GUEST *pbSrc = &pVBVAData->guest.pu8Data[pVBVAData->off32Data]; /* Fetch or point the data. */ @@ -313,5 +315,5 @@ { /* The command does not cross buffer boundary. Return address in the buffer. */ - *ppHdr = (VBVACMDHDR *)pu8Src; + *ppHdr = (VBVACMDHDR RT_UNTRUSTED_VOLATILE_GUEST *)pbSrc; /* The data offset will be updated in vbvaReleaseCmd. */ @@ -320,7 +322,6 @@ { /* The command crosses buffer boundary. Rare case, so not optimized. */ - uint8_t *pu8Dst = (uint8_t *)RTMemAlloc(cbRecord); - - if (!pu8Dst) + uint8_t *pbDst = (uint8_t *)RTMemAlloc(cbRecord); + if (!pbDst) { LogFlowFunc (("could not allocate %d bytes from heap!!!\n", cbRecord)); @@ -328,9 +329,9 @@ } - vbvaFetchBytes(pVBVAData, pu8Dst, cbRecord); - - *ppHdr = (VBVACMDHDR *)pu8Dst; - - LOGVBVABUFFER(("Allocated from heap %p\n", pu8Dst)); + vbvaFetchBytes(pVBVAData, pbDst, cbRecord); + + *ppHdr = (VBVACMDHDR *)pbDst; + + LOGVBVABUFFER(("Allocated from heap %p\n", pbDst)); } } @@ -348,14 +349,14 @@ } -static void vbvaReleaseCmd(VBVADATA *pVBVAData, VBVACMDHDR *pHdr, uint32_t cbCmd) -{ - VBVAPARTIALRECORD *pPartialRecord = &pVBVAData->partialRecord; - const uint8_t *au8RingBuffer = pVBVAData->guest.pu8Data; - - if ( (uintptr_t)pHdr >= (uintptr_t)au8RingBuffer - && (uintptr_t)pHdr < (uintptr_t)&au8RingBuffer[pVBVAData->cbData]) +static void vbvaReleaseCmd(VBVADATA *pVBVAData, VBVACMDHDR RT_UNTRUSTED_VOLATILE_GUEST *pHdr, uint32_t cbCmd) +{ + VBVAPARTIALRECORD *pPartialRecord = &pVBVAData->partialRecord; + const uint8_t RT_UNTRUSTED_VOLATILE_GUEST *pbRingBuffer = pVBVAData->guest.pu8Data; + + if ( (uintptr_t)pHdr >= (uintptr_t)pbRingBuffer + && (uintptr_t)pHdr < (uintptr_t)&pbRingBuffer[pVBVAData->cbData]) { /* The pointer is inside ring buffer. Must be continuous chunk. */ - Assert(pVBVAData->cbData - (uint32_t)((uint8_t *)pHdr - au8RingBuffer) >= cbCmd); + Assert(pVBVAData->cbData - (uint32_t)((uint8_t *)pHdr - pbRingBuffer) >= cbCmd); /* Advance data offset and sync with guest. */ @@ -380,5 +381,5 @@ } - RTMemFree(pHdr); + RTMemFree((void *)pHdr); } } @@ -403,13 +404,11 @@ for (;;) { - VBVACMDHDR *phdr = NULL; - uint32_t cbCmd = UINT32_MAX; - /* Fetch the command data. */ - if (!vbvaFetchCmd(pVBVAData, &phdr, &cbCmd)) + VBVACMDHDR RT_UNTRUSTED_VOLATILE_GUEST *pHdr = NULL; + uint32_t cbCmd = UINT32_MAX; + if (!vbvaFetchCmd(pVBVAData, &pHdr, &cbCmd)) { LogFunc(("unable to fetch command. off32Data = %d, off32Free = %d!!!\n", - pVBVAData->off32Data, pVBVAData->guest.pVBVA->off32Free)); - + pVBVAData->off32Data, pVBVAData->guest.pVBVA->off32Free)); return VERR_NOT_SUPPORTED; } @@ -438,15 +437,15 @@ /* Updates the rectangle and sends the command to the VRDP server. */ - pVGAState->pDrv->pfnVBVAUpdateProcess(pVGAState->pDrv, uScreenId, phdr, cbCmd); - - int32_t xRight = phdr->x + phdr->w; - int32_t yBottom = phdr->y + phdr->h; + pVGAState->pDrv->pfnVBVAUpdateProcess(pVGAState->pDrv, uScreenId, pHdr, cbCmd); + + int32_t xRight = pHdr->x + pHdr->w; + int32_t yBottom = pHdr->y + pHdr->h; /* These are global coords, relative to the primary screen. */ LOGVBVABUFFER(("cbCmd = %d, x=%d, y=%d, w=%d, h=%d\n", - cbCmd, phdr->x, phdr->y, phdr->w, phdr->h)); + cbCmd, pHdr->x, pHdr->y, pHdr->w, pHdr->h)); LogRel3(("%s: update command cbCmd = %d, x=%d, y=%d, w=%d, h=%d\n", - __FUNCTION__, cbCmd, phdr->x, phdr->y, phdr->w, phdr->h)); + __FUNCTION__, cbCmd, pHdr->x, pHdr->y, pHdr->w, pHdr->h)); /* Collect all rects into one. */ @@ -454,6 +453,6 @@ { /* This is the first rectangle to be added. */ - dirtyRect.xLeft = phdr->x; - dirtyRect.yTop = phdr->y; + dirtyRect.xLeft = pHdr->x; + dirtyRect.yTop = pHdr->y; dirtyRect.xRight = xRight; dirtyRect.yBottom = yBottom; @@ -463,12 +462,12 @@ { /* Adjust region coordinates. */ - if (dirtyRect.xLeft > phdr->x) + if (dirtyRect.xLeft > pHdr->x) { - dirtyRect.xLeft = phdr->x; + dirtyRect.xLeft = pHdr->x; } - if (dirtyRect.yTop > phdr->y) + if (dirtyRect.yTop > pHdr->y) { - dirtyRect.yTop = phdr->y; + dirtyRect.yTop = pHdr->y; } @@ -485,5 +484,5 @@ } - vbvaReleaseCmd(pVBVAData, phdr, cbCmd); + vbvaReleaseCmd(pVBVAData, pHdr, cbCmd); } @@ -516,12 +515,9 @@ { VBVADATA *pVBVAData = &pCtx->aViews[uScreenId].vbva; - if (pVBVAData->guest.pVBVA) { rc = vbvaFlushProcess(uScreenId, pVGAState, pVBVAData); if (RT_FAILURE(rc)) - { break; - } } } @@ -558,77 +554,73 @@ } -static int vbvaEnable(unsigned uScreenId, PVGASTATE pVGAState, VBVACONTEXT *pCtx, VBVABUFFER *pVBVA, uint32_t u32Offset, bool fRestored) -{ +static int vbvaEnable(unsigned uScreenId, PVGASTATE pVGAState, VBVACONTEXT *pCtx, + VBVABUFFER RT_UNTRUSTED_VOLATILE_GUEST *pVBVA, uint32_t u32Offset, bool fRestored) +{ + /* + * Copy into non-volatile memory and validate its content. + */ + VBVABUFFER VbgaSafe; + RT_COPY_VOLATILE(VbgaSafe, *pVBVA); + RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); + + uint32_t const cbVBVABuffer = RT_UOFFSETOF(VBVABUFFER, au8Data) + VbgaSafe.cbData; + ASSERT_GUEST_RETURN( VbgaSafe.cbData <= UINT32_MAX - RT_UOFFSETOF(VBVABUFFER, au8Data) + && cbVBVABuffer <= pVGAState->vram_size + && u32Offset > pVGAState->vram_size - cbVBVABuffer, + VERR_INVALID_PARAMETER); + if (!fRestored) + { + ASSERT_GUEST_RETURN(VbgaSafe.off32Data == 0, VERR_INVALID_PARAMETER); + ASSERT_GUEST_RETURN(VbgaSafe.off32Free == 0, VERR_INVALID_PARAMETER); + ASSERT_GUEST_RETURN(VbgaSafe.indexRecordFirst == 0, VERR_INVALID_PARAMETER); + ASSERT_GUEST_RETURN(VbgaSafe.indexRecordFree == 0, VERR_INVALID_PARAMETER); + } + ASSERT_GUEST_RETURN( VbgaSafe.cbPartialWriteThreshold < VbgaSafe.cbData + && VbgaSafe.cbPartialWriteThreshold != 0, + VERR_INVALID_PARAMETER); + RT_UNTRUSTED_VALIDATED_FENCE(); + + /* + * Okay, try do the job. + */ int rc; - - /* Check if VBVABUFFER content makes sense. */ - const VBVABUFFER parms = *pVBVA; - - uint32_t cbVBVABuffer = RT_UOFFSETOF(VBVABUFFER, au8Data) + parms.cbData; - if ( parms.cbData > UINT32_MAX - RT_UOFFSETOF(VBVABUFFER, au8Data) - || cbVBVABuffer > pVGAState->vram_size - || u32Offset > pVGAState->vram_size - cbVBVABuffer) - { - return VERR_INVALID_PARAMETER; - } - - if (!fRestored) - { - if ( parms.off32Data != 0 - || parms.off32Free != 0 - || parms.indexRecordFirst != 0 - || parms.indexRecordFree != 0) - { - return VERR_INVALID_PARAMETER; - } - } - - if ( parms.cbPartialWriteThreshold >= parms.cbData - || parms.cbPartialWriteThreshold == 0) - { - return VERR_INVALID_PARAMETER; - } - if (pVGAState->pDrv->pfnVBVAEnable) { - RT_ZERO(pVBVA->hostFlags); + pVBVA->hostFlags.u32HostEvents = 0; + pVBVA->hostFlags.u32SupportedOrders = 0; rc = pVGAState->pDrv->pfnVBVAEnable(pVGAState->pDrv, uScreenId, &pVBVA->hostFlags, false); + if (RT_SUCCESS(rc)) + { + /* pVBVA->hostFlags has been set up by pfnVBVAEnable. */ + LogFlowFunc(("u32HostEvents=0x%08x u32SupportedOrders=0x%08x\n", + pVBVA->hostFlags.u32HostEvents, pVBVA->hostFlags.u32SupportedOrders)); + + VBVADATA *pVBVAData = &pCtx->aViews[uScreenId].vbva; + pVBVAData->guest.pVBVA = pVBVA; + pVBVAData->guest.pu8Data = &pVBVA->au8Data[0]; + pVBVAData->u32VBVAOffset = u32Offset; + pVBVAData->off32Data = VbgaSafe.off32Data; + pVBVAData->indexRecordFirst = VbgaSafe.indexRecordFirst; + pVBVAData->cbPartialWriteThreshold = VbgaSafe.cbPartialWriteThreshold; + pVBVAData->cbData = VbgaSafe.cbData; + + if (!fRestored) + { + /** @todo Actually this function must not touch the partialRecord structure at all, + * because initially it is a zero and when VBVA is disabled this should be set to zero. + * But I'm not sure that no code depends on zeroing partialRecord here. + * So for now (a quick fix for 4.1) just do not do this if the VM was restored, + * when partialRecord might be loaded already from the saved state. + */ + pVBVAData->partialRecord.pu8 = NULL; + pVBVAData->partialRecord.cb = 0; + } + + /* VBVA is working so disable the pause. */ + pCtx->fPaused = false; + } } else - { rc = VERR_NOT_SUPPORTED; - } - - if (RT_SUCCESS(rc)) - { - /* pVBVA->hostFlags has been set up by pfnVBVAEnable. */ - LogFlowFunc(("u32HostEvents 0x%08X, u32SupportedOrders 0x%08X\n", - pVBVA->hostFlags.u32HostEvents, pVBVA->hostFlags.u32SupportedOrders)); - - VBVADATA *pVBVAData = &pCtx->aViews[uScreenId].vbva; - pVBVAData->guest.pVBVA = pVBVA; - pVBVAData->guest.pu8Data = &pVBVA->au8Data[0]; - pVBVAData->u32VBVAOffset = u32Offset; - pVBVAData->off32Data = parms.off32Data; - pVBVAData->indexRecordFirst = parms.indexRecordFirst; - pVBVAData->cbPartialWriteThreshold = parms.cbPartialWriteThreshold; - pVBVAData->cbData = parms.cbData; - - if (!fRestored) - { - /** @todo Actually this function must not touch the partialRecord structure at all, - * because initially it is a zero and when VBVA is disabled this should be set to zero. - * But I'm not sure that no code depends on zeroing partialRecord here. - * So for now (a quick fix for 4.1) just do not do this if the VM was restored, - * when partialRecord might be loaded already from the saved state. - */ - pVBVAData->partialRecord.pu8 = NULL; - pVBVAData->partialRecord.cb = 0; - } - - /* VBVA is working so disable the pause. */ - pCtx->fPaused = false; - } - return rc; } @@ -811,5 +803,4 @@ /* Check which view contains the buffer. */ HGSMIOFFSET offBuffer = HGSMIPointerToOffsetHost(pIns, pvBuffer); - if (offBuffer != HGSMIOFFSET_VOID) { @@ -818,14 +809,8 @@ { const VBVAINFOVIEW *pView = &pCtx->aViews[uScreenId].view; - - if ( pView->u32ViewSize > 0 - && pView->u32ViewOffset <= offBuffer - && offBuffer <= pView->u32ViewOffset + pView->u32ViewSize - 1) - { + if ((uint32_t)(offBuffer - pView->u32ViewOffset) < pView->u32ViewSize) return pView->u32ViewIndex; - } - } - } - + } + } return UINT32_MAX; } @@ -2116,94 +2101,58 @@ static int vbvaHandleQueryConf32(PVGASTATE pVGAState, VBVACONF32 RT_UNTRUSTED_VOLATILE_GUEST *pConf32) { - int rc = VINF_SUCCESS; - PHGSMIINSTANCE pIns = pVGAState->pHGSMI; - VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pIns); - - const uint32_t u32Index = pConf32->u32Index; - ASMCompilerBarrier(); - - LogFlowFunc(("VBVA_QUERY_CONF32: u32Index %d, u32Value 0x%x\n", - u32Index, pConf32->u32Value)); - - if (u32Index == VBOX_VBVA_CONF32_MONITOR_COUNT) - { - pConf32->u32Value = pCtx->cViews; - } - else if (u32Index == VBOX_VBVA_CONF32_HOST_HEAP_SIZE) - { - /** @todo a value calculated from the vram size */ - pConf32->u32Value = _64K; - } - else if ( u32Index == VBOX_VBVA_CONF32_MODE_HINT_REPORTING - || u32Index == VBOX_VBVA_CONF32_GUEST_CURSOR_REPORTING) - { - pConf32->u32Value = VINF_SUCCESS; - } - else if (u32Index == VBOX_VBVA_CONF32_CURSOR_CAPABILITIES) - { - pConf32->u32Value = pVGAState->fHostCursorCapabilities; - } - else if (u32Index == VBOX_VBVA_CONF32_SCREEN_FLAGS) - { - pConf32->u32Value = VBVA_SCREEN_F_ACTIVE - | VBVA_SCREEN_F_DISABLED - | VBVA_SCREEN_F_BLANK - | VBVA_SCREEN_F_BLANK2; - } - else if (u32Index == VBOX_VBVA_CONF32_MAX_RECORD_SIZE) - { - pConf32->u32Value = VBVA_MAX_RECORD_SIZE; - } + uint32_t const idxQuery = pConf32->u32Index; + RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); + LogFlowFunc(("VBVA_QUERY_CONF32: u32Index %d, u32Value 0x%x\n", idxQuery, pConf32->u32Value)); + + VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pVGAState->pHGSMI); + uint32_t uValue; + if (idxQuery == VBOX_VBVA_CONF32_MONITOR_COUNT) + uValue = pCtx->cViews; + else if (idxQuery == VBOX_VBVA_CONF32_HOST_HEAP_SIZE) + uValue = _64K; /** @todo a value calculated from the vram size */ + else if ( idxQuery == VBOX_VBVA_CONF32_MODE_HINT_REPORTING + || idxQuery == VBOX_VBVA_CONF32_GUEST_CURSOR_REPORTING) + uValue = VINF_SUCCESS; + else if (idxQuery == VBOX_VBVA_CONF32_CURSOR_CAPABILITIES) + uValue = pVGAState->fHostCursorCapabilities; + else if (idxQuery == VBOX_VBVA_CONF32_SCREEN_FLAGS) + uValue = VBVA_SCREEN_F_ACTIVE + | VBVA_SCREEN_F_DISABLED + | VBVA_SCREEN_F_BLANK + | VBVA_SCREEN_F_BLANK2; + else if (idxQuery == VBOX_VBVA_CONF32_MAX_RECORD_SIZE) + uValue = VBVA_MAX_RECORD_SIZE; else - { - Log(("Unsupported VBVA_QUERY_CONF32 index %d!!!\n", - u32Index)); - rc = VERR_INVALID_PARAMETER; - } - - return rc; -} - -static int vbvaHandleSetConf32(PVGASTATE pVGAState, VBVACONF32 RT_UNTRUSTED_VOLATILE_GUEST *pConf32) -{ - NOREF(pVGAState); - - VBVACONF32 parms; - parms.u32Index = pConf32->u32Index; - parms.u32Value = pConf32->u32Value; - ASMCompilerBarrier(); - - LogFlowFunc(("VBVA_SET_CONF32: u32Index %d, u32Value 0x%x\n", - parms.u32Index, parms.u32Value)); - - int rc = VINF_SUCCESS; - if (parms.u32Index == VBOX_VBVA_CONF32_MONITOR_COUNT) - { - /* do nothing. this is a const. */ - } - else if (parms.u32Index == VBOX_VBVA_CONF32_HOST_HEAP_SIZE) - { - /* do nothing. this is a const. */ - } + ASSERT_GUEST_MSG_FAILED_RETURN(("Invalid index %#x\n", idxQuery), VERR_INVALID_PARAMETER); + + pConf32->u32Value = uValue; + return VINF_SUCCESS; +} + +static int vbvaHandleSetConf32(VBVACONF32 RT_UNTRUSTED_VOLATILE_GUEST *pConf32) +{ + uint32_t const idxQuery = pConf32->u32Index; + uint32_t const uValue = pConf32->u32Value; + RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); + LogFlowFunc(("VBVA_SET_CONF32: u32Index %d, u32Value 0x%x\n", idxQuery, uValue)); + + if (idxQuery == VBOX_VBVA_CONF32_MONITOR_COUNT) + { /* do nothing. this is a const. */ } + else if (idxQuery == VBOX_VBVA_CONF32_HOST_HEAP_SIZE) + { /* do nothing. this is a const. */ } else - { - Log(("Unsupported VBVA_SET_CONF32 index %d!!!\n", - parms.u32Index)); - rc = VERR_INVALID_PARAMETER; - } - - return rc; + ASSERT_GUEST_MSG_FAILED_RETURN(("Invalid index %#x (value=%u)\n", idxQuery, uValue), VERR_INVALID_PARAMETER); + + return VINF_SUCCESS; } static int vbvaHandleInfoHeap(PVGASTATE pVGAState, const VBVAINFOHEAP RT_UNTRUSTED_VOLATILE_GUEST *pInfoHeap) { - VBVAINFOHEAP parms; - parms.u32HeapOffset = pInfoHeap->u32HeapOffset; - parms.u32HeapSize = pInfoHeap->u32HeapSize; - ASMCompilerBarrier(); - LogFlowFunc(("VBVA_INFO_HEAP: offset 0x%x, size 0x%x\n", - parms.u32HeapOffset, parms.u32HeapSize)); - - return HGSMIHostHeapSetup(pVGAState->pHGSMI, parms.u32HeapOffset, parms.u32HeapSize); + uint32_t const offHeap = pInfoHeap->u32HeapOffset; + uint32_t const cbHeap = pInfoHeap->u32HeapSize; + RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); + LogFlowFunc(("VBVA_INFO_HEAP: offset 0x%x, size 0x%x\n", offHeap, cbHeap)); + + return HGSMIHostHeapSetup(pVGAState->pHGSMI, offHeap, cbHeap); } @@ -2211,37 +2160,34 @@ { VBVAINFOVIEW view; - view.u32ViewIndex = pView->u32ViewIndex; - view.u32ViewOffset = pView->u32ViewOffset; - view.u32ViewSize = pView->u32ViewSize; - view.u32MaxScreenSize = pView->u32MaxScreenSize; - ASMCompilerBarrier(); + RT_COPY_VOLATILE(view, *pView); + RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); LogFlowFunc(("VBVA_INFO_VIEW: u32ViewIndex %d, u32ViewOffset 0x%x, u32ViewSize 0x%x, u32MaxScreenSize 0x%x\n", view.u32ViewIndex, view.u32ViewOffset, view.u32ViewSize, view.u32MaxScreenSize)); - PHGSMIINSTANCE pIns = pVGAState->pHGSMI; - VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pIns); - - if ( view.u32ViewIndex < pCtx->cViews - && view.u32ViewOffset <= pVGAState->vram_size - && view.u32ViewSize <= pVGAState->vram_size - && view.u32ViewOffset <= pVGAState->vram_size - view.u32ViewSize - && view.u32MaxScreenSize <= view.u32ViewSize) - { - pCtx->aViews[view.u32ViewIndex].view = view; - return VINF_SUCCESS; - } - - LogRelFlow(("VBVA: InfoView: invalid data! index %d(%d), offset 0x%x, size 0x%x, max 0x%x, vram size 0x%x\n", - view.u32ViewIndex, pCtx->cViews, view.u32ViewOffset, view.u32ViewSize, - view.u32MaxScreenSize, pVGAState->vram_size)); - return VERR_INVALID_PARAMETER; + VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pVGAState->pHGSMI); + ASSERT_GUEST_LOGREL_MSG_RETURN( view.u32ViewIndex < pCtx->cViews + && view.u32ViewOffset <= pVGAState->vram_size + && view.u32ViewSize <= pVGAState->vram_size + && view.u32ViewOffset <= pVGAState->vram_size - view.u32ViewSize + && view.u32MaxScreenSize <= view.u32ViewSize, + ("index %d(%d), offset 0x%x, size 0x%x, max 0x%x, vram size 0x%x\n", + view.u32ViewIndex, pCtx->cViews, view.u32ViewOffset, view.u32ViewSize, + view.u32MaxScreenSize, pVGAState->vram_size), + VERR_INVALID_PARAMETER); + RT_UNTRUSTED_VALIDATED_FENCE(); + + pCtx->aViews[view.u32ViewIndex].view = view; + return VINF_SUCCESS; } int VBVAInfoScreen(PVGASTATE pVGAState, const VBVAINFOSCREEN RT_UNTRUSTED_VOLATILE_GUEST *pScreen) { + /* + * Copy input into non-volatile buffer. + */ VBVAINFOSCREEN screen; - memcpy(&screen, (void *)pScreen, sizeof(screen)); - ASMCompilerBarrier(); + RT_COPY_VOLATILE(screen, *pScreen); + RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); LogRel(("VBVA: InfoScreen: [%d] @%d,%d %dx%d, line 0x%x, BPP %d, flags 0x%x\n", screen.u32ViewIndex, screen.i32OriginX, screen.i32OriginY, @@ -2249,35 +2195,43 @@ screen.u32LineSize, screen.u16BitsPerPixel, screen.u16Flags)); - PHGSMIINSTANCE pIns = pVGAState->pHGSMI; - VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pIns); - + /* + * Validate input. + */ /* Allow screen.u16BitsPerPixel == 0 because legacy guest code used it for screen blanking. */ - if ( screen.u32ViewIndex < pCtx->cViews - && screen.u16BitsPerPixel <= 32 - && screen.u32Width <= UINT16_MAX - && screen.u32Height <= UINT16_MAX - && screen.u32LineSize <= UINT16_MAX * 4) - { - const VBVAINFOVIEW *pView = &pCtx->aViews[screen.u32ViewIndex].view; - const uint32_t u32BytesPerPixel = (screen.u16BitsPerPixel + 7) / 8; - if (screen.u32Width <= screen.u32LineSize / (u32BytesPerPixel? u32BytesPerPixel: 1)) - { - const uint64_t u64ScreenSize = (uint64_t)screen.u32LineSize * screen.u32Height; - if ( screen.u32StartOffset <= pView->u32ViewSize - && u64ScreenSize <= pView->u32MaxScreenSize - && screen.u32StartOffset <= pView->u32ViewSize - (uint32_t)u64ScreenSize) - { - vbvaResize(pVGAState, &pCtx->aViews[screen.u32ViewIndex], &screen, true); - return VINF_SUCCESS; - } - - LogRelFlow(("VBVA: InfoScreen: invalid data! size %#RX64, max %#RX32\n", - u64ScreenSize, pView->u32MaxScreenSize)); - } - } - else - LogRelFlow(("VBVA: InfoScreen: invalid data! index %RU32(%RU32)\n", screen.u32ViewIndex, pCtx->cViews)); - - return VERR_INVALID_PARAMETER; + VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pVGAState->pHGSMI); + ASSERT_GUEST_LOGREL_MSG_RETURN(screen.u32ViewIndex < pCtx->cViews, + ("Screen index %#x is out of bound (cViews=%#x)\n", screen.u32ViewIndex, pCtx->cViews), + VERR_INVALID_PARAMETER); + ASSERT_GUEST_LOGREL_MSG_RETURN( screen.u16BitsPerPixel <= 32 + && screen.u32Width <= UINT16_MAX + && screen.u32Height <= UINT16_MAX + && screen.u32LineSize <= UINT16_MAX * UINT32_C(4), + ("One or more values out of range: u16BitsPerPixel=%#x u32Width=%#x u32Height=%#x u32LineSize=%#x\n", + screen.u16BitsPerPixel, screen.u32Width, screen.u32Height, screen.u32LineSize), + VERR_INVALID_PARAMETER); + RT_UNTRUSTED_VALIDATED_FENCE(); + + const VBVAINFOVIEW *pView = &pCtx->aViews[screen.u32ViewIndex].view; + const uint32_t cbPerPixel = (screen.u16BitsPerPixel + 7) / 8; + ASSERT_GUEST_LOGREL_MSG_RETURN(screen.u32Width <= screen.u32LineSize / (cbPerPixel ? cbPerPixel : 1), + ("u32Width=%#x u32LineSize=%3x cbPerPixel=%#x\n", + screen.u32Width, screen.u32LineSize, cbPerPixel), + VERR_INVALID_PARAMETER); + + const uint64_t u64ScreenSize = (uint64_t)screen.u32LineSize * screen.u32Height; + + ASSERT_GUEST_LOGREL_MSG_RETURN( screen.u32StartOffset <= pView->u32ViewSize + && u64ScreenSize <= pView->u32MaxScreenSize + && screen.u32StartOffset <= pView->u32ViewSize - (uint32_t)u64ScreenSize, + ("u32StartOffset=%#x u32ViewSize=%#x u64ScreenSize=%#RX64 u32MaxScreenSize=%#x\n", + screen.u32StartOffset, pView->u32ViewSize, u64ScreenSize), + VERR_INVALID_PARAMETER); + RT_UNTRUSTED_VALIDATED_FENCE(); + + /* + * Do the job. + */ + vbvaResize(pVGAState, &pCtx->aViews[screen.u32ViewIndex], &screen, true); + return VINF_SUCCESS; } @@ -2299,84 +2253,68 @@ } -static int vbvaHandleEnable(PVGASTATE pVGAState, VBVAENABLE const volatile *pVbvaEnable, uint32_t u32ScreenId) -{ +static int vbvaHandleEnable(PVGASTATE pVGAState, uint32_t fEnableFlags, uint32_t offEnable, uint32_t idScreen) +{ + LogFlowFunc(("VBVA_ENABLE[%u]: fEnableFlags=0x%x offEnable=%#x\n", idScreen, fEnableFlags, offEnable)); + PHGSMIINSTANCE pIns = pVGAState->pHGSMI; + VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pIns); + + /* + * Validate input. + */ + ASSERT_GUEST_LOGREL_MSG_RETURN(idScreen < pCtx->cViews, ("idScreen=%#x cViews=%#x\n", idScreen, pCtx->cViews), VERR_INVALID_PARAMETER); + ASSERT_GUEST_LOGREL_MSG_RETURN( (fEnableFlags & (VBVA_F_ENABLE | VBVA_F_DISABLE)) == VBVA_F_ENABLE + || (fEnableFlags & (VBVA_F_ENABLE | VBVA_F_DISABLE)) == VBVA_F_DISABLE, + ("fEnableFlags=%#x\n", fEnableFlags), + VERR_INVALID_PARAMETER); + if (fEnableFlags & VBVA_F_ENABLE) + { + ASSERT_GUEST_LOGREL_MSG_RETURN(offEnable < pVGAState->vram_size, + ("offEnable=%#x vram_size=%#x\n", offEnable, pVGAState->vram_size), + VERR_INVALID_PARAMETER); + if (fEnableFlags & VBVA_F_ABSOFFSET) + /* Offset from VRAM start. */ + ASSERT_GUEST_LOGREL_MSG_RETURN( pVGAState->vram_size >= RT_UOFFSETOF(VBVABUFFER, au8Data) + && offEnable <= pVGAState->vram_size - RT_UOFFSETOF(VBVABUFFER, au8Data), + ("offEnable=%#x vram_size=%#x\n", offEnable, pVGAState->vram_size), + VERR_INVALID_PARAMETER); + else + { + /* Offset from the view start. We'd be using idScreen here to fence required. */ + RT_UNTRUSTED_VALIDATED_FENCE(); + const VBVAINFOVIEW *pView = &pCtx->aViews[idScreen].view; + ASSERT_GUEST_LOGREL_MSG_RETURN( pVGAState->vram_size - offEnable >= pView->u32ViewOffset + && pView->u32ViewSize >= RT_UOFFSETOF(VBVABUFFER, au8Data) + && offEnable <= pView->u32ViewSize - RT_UOFFSETOF(VBVABUFFER, au8Data), + ("offEnable=%#x vram_size=%#x view: %#x LB %#x\n", + offEnable, pVGAState->vram_size, pView->u32ViewOffset, pView->u32ViewSize), + VERR_INVALID_PARAMETER); + offEnable += pView->u32ViewOffset; + } + ASSERT_GUEST_LOGREL_MSG_RETURN(HGSMIIsOffsetValid(pIns, offEnable), + ("offEnable=%#x area %#x LB %#x\n", + offEnable, HGSMIGetAreaOffset(pIns), HGSMIGetAreaSize(pIns)), + VERR_INVALID_PARAMETER); + } + RT_UNTRUSTED_VALIDATED_FENCE(); + + /* + * Execute. + */ int rc = VINF_SUCCESS; - PHGSMIINSTANCE pIns = pVGAState->pHGSMI; - VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pIns); - - if (u32ScreenId > pCtx->cViews) - return VERR_INVALID_PARAMETER; - - uint32_t fEnableFlags = pVbvaEnable->u32Flags; - uint32_t offEnable = pVbvaEnable->u32Offset; - ASMCompilerBarrier(); - - LogFlowFunc(("VBVA_ENABLE[%d]: u32Flags 0x%x u32Offset %#x\n", u32ScreenId, fEnableFlags, offEnable)); - - if ((fEnableFlags & (VBVA_F_ENABLE | VBVA_F_DISABLE)) == VBVA_F_ENABLE) - { - if (offEnable < pVGAState->vram_size) - { - /* Guest reported offset either absolute or relative to view. */ - if (fEnableFlags & VBVA_F_ABSOFFSET) - { - /* Offset from VRAM start. */ - if ( pVGAState->vram_size < RT_UOFFSETOF(VBVABUFFER, au8Data) - || offEnable > pVGAState->vram_size - RT_UOFFSETOF(VBVABUFFER, au8Data)) - { - rc = VERR_INVALID_PARAMETER; - } - } - else - { - /* Offset from the view start. */ - const VBVAINFOVIEW *pView = &pCtx->aViews[u32ScreenId].view; - if ( pVGAState->vram_size - offEnable < pView->u32ViewOffset - || pView->u32ViewSize < RT_UOFFSETOF(VBVABUFFER, au8Data) - || offEnable > pView->u32ViewSize - RT_UOFFSETOF(VBVABUFFER, au8Data)) - { - rc = VERR_INVALID_PARAMETER; - } - else - { - offEnable += pView->u32ViewOffset; - } - } - } - else - { - rc = VERR_INVALID_PARAMETER; - } - - if (RT_SUCCESS(rc)) - { - VBVABUFFER *pVBVA = (VBVABUFFER *)HGSMIOffsetToPointerHost(pIns, offEnable); - if (pVBVA) - { - /* Process any pending orders and empty the VBVA ring buffer. */ - vbvaFlush(pVGAState, pCtx); - - rc = vbvaEnable(u32ScreenId, pVGAState, pCtx, pVBVA, offEnable, false /* fRestored */); - } - else - { - Log(("Invalid VBVABUFFER offset 0x%x!!!\n", offEnable)); - rc = VERR_INVALID_PARAMETER; - } - } - + if (fEnableFlags & VBVA_F_ENABLE) + { + VBVABUFFER RT_UNTRUSTED_VOLATILE_GUEST *pVBVA + = (VBVABUFFER RT_UNTRUSTED_VOLATILE_GUEST *)HGSMIOffsetToPointerHost(pIns, offEnable); + ASSERT_GUEST_LOGREL_RETURN(pVBVA, VERR_INVALID_PARAMETER); /* already check above, but let's be careful. */ + + /* Process any pending orders and empty the VBVA ring buffer. */ + vbvaFlush(pVGAState, pCtx); + + rc = vbvaEnable(idScreen, pVGAState, pCtx, pVBVA, offEnable, false /* fRestored */); if (RT_FAILURE(rc)) LogRelMax(8, ("VBVA: can not enable: %Rrc\n", rc)); } - else if ((fEnableFlags & (VBVA_F_ENABLE | VBVA_F_DISABLE)) == VBVA_F_DISABLE) - { - rc = vbvaDisable(u32ScreenId, pVGAState, pCtx); - } else - { - Log(("Invalid VBVA_ENABLE flags 0x%x!!!\n", fEnableFlags)); - rc = VERR_INVALID_PARAMETER; - } - + rc = vbvaDisable(idScreen, pVGAState, pCtx); return rc; } @@ -2512,5 +2450,5 @@ case VBVA_SET_CONF32: if (cbBuffer >= sizeof(VBVACONF32)) - rc = vbvaHandleSetConf32(pVGAState, (VBVACONF32 RT_UNTRUSTED_VOLATILE_GUEST *)pvBuffer); + rc = vbvaHandleSetConf32((VBVACONF32 RT_UNTRUSTED_VOLATILE_GUEST *)pvBuffer); else rc = VERR_INVALID_PARAMETER; @@ -2569,22 +2507,19 @@ { VBVAENABLE RT_UNTRUSTED_VOLATILE_GUEST *pVbvaEnable = (VBVAENABLE RT_UNTRUSTED_VOLATILE_GUEST *)pvBuffer; - const uint32_t u32Flags = pVbvaEnable->u32Flags; + uint32_t const fEnableFlags = pVbvaEnable->u32Flags; + uint32_t const offEnable = pVbvaEnable->u32Offset; RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); - uint32_t u32ScreenId; - if (u32Flags & VBVA_F_EXTENDED) + uint32_t idScreen; + if (fEnableFlags & VBVA_F_EXTENDED) { - if (cbBuffer >= sizeof(VBVAENABLE_EX)) - u32ScreenId = ((VBVAENABLE_EX RT_UNTRUSTED_VOLATILE_GUEST *)pvBuffer)->u32ScreenId; - else - { - rc = VERR_INVALID_PARAMETER; - break; - } + ASSERT_GUEST_STMT_BREAK(cbBuffer >= sizeof(VBVAENABLE_EX), rc = VERR_INVALID_PARAMETER); + idScreen = ((VBVAENABLE_EX RT_UNTRUSTED_VOLATILE_GUEST *)pvBuffer)->u32ScreenId; + RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); } else - u32ScreenId = vbvaViewFromBufferPtr(pIns, pCtx, pvBuffer); - - rc = vbvaHandleEnable(pVGAState, pVbvaEnable, u32ScreenId); + idScreen = vbvaViewFromBufferPtr(pIns, pCtx, pvBuffer); + + rc = vbvaHandleEnable(pVGAState, fEnableFlags, offEnable, idScreen); pVbvaEnable->i32Result = rc; } @@ -2594,6 +2529,6 @@ if (cbBuffer >= sizeof(VBVAMOUSEPOINTERSHAPE)) { - VBVAMOUSEPOINTERSHAPE RT_UNTRUSTED_VOLATILE_GUEST *pShape; - pShape = (VBVAMOUSEPOINTERSHAPE RT_UNTRUSTED_VOLATILE_GUEST *)pvBuffer; + VBVAMOUSEPOINTERSHAPE RT_UNTRUSTED_VOLATILE_GUEST *pShape + = (VBVAMOUSEPOINTERSHAPE RT_UNTRUSTED_VOLATILE_GUEST *)pvBuffer; rc = vbvaMousePointerShape(pVGAState, pCtx, pShape, cbBuffer); pShape->i32Result = rc; Index: /trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp =================================================================== --- /trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp (revision 71618) +++ /trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp (revision 71619) @@ -3087,4 +3087,6 @@ VBOXVDMA_CTL_TYPE enmCtl = pCmd->enmCtl; RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); + + int rc; if (enmCtl < VBOXVDMA_CTL_TYPE_END) { @@ -3094,21 +3096,21 @@ { case VBOXVDMA_CTL_TYPE_ENABLE: - pCmd->i32Result = VINF_SUCCESS; + rc = VINF_SUCCESS; break; case VBOXVDMA_CTL_TYPE_DISABLE: - pCmd->i32Result = VINF_SUCCESS; + rc = VINF_SUCCESS; break; case VBOXVDMA_CTL_TYPE_FLUSH: - pCmd->i32Result = VINF_SUCCESS; + rc = VINF_SUCCESS; break; + case VBOXVDMA_CTL_TYPE_WATCHDOG: #ifdef VBOX_VDMA_WITH_WATCHDOG - case VBOXVDMA_CTL_TYPE_WATCHDOG: - pCmd->i32Result = vboxVDMAWatchDogCtl(pVdma, pCmd->u32Offset); + rc = vboxVDMAWatchDogCtl(pVdma, pCmd->u32Offset); +#else + rc = VERR_NOT_SUPPORTED; +#endif break; -#endif default: - WARN(("cmd not supported")); - pCmd->i32Result = VERR_NOT_SUPPORTED; - break; + AssertFailedBreakStmt(rc = VERR_IPE_NOT_REACHED_DEFAULT_CASE); } } @@ -3116,9 +3118,10 @@ { RT_UNTRUSTED_VALIDATED_FENCE(); - WARN(("cmd not supported")); - pCmd->i32Result = VERR_NOT_SUPPORTED; - } - - int rc = VBoxSHGSMICommandComplete(pIns, pCmd); + ASSERT_GUEST_FAILED(); + rc = VERR_NOT_SUPPORTED; + } + + pCmd->i32Result = rc; + rc = VBoxSHGSMICommandComplete(pIns, pCmd); AssertRC(rc); } Index: /trunk/src/VBox/Devices/Graphics/HGSMI/HGSMIHost.cpp =================================================================== --- /trunk/src/VBox/Devices/Graphics/HGSMI/HGSMIHost.cpp (revision 71618) +++ /trunk/src/VBox/Devices/Graphics/HGSMI/HGSMIHost.cpp (revision 71619) @@ -66,4 +66,5 @@ #include +#include #include #define LOG_GROUP LOG_GROUP_HGSMI @@ -491,14 +492,14 @@ * */ -static int hgsmiHostHeapLock (HGSMIINSTANCE *pIns) -{ - int rc = RTCritSectEnter (&pIns->hostHeapCritSect); +static int hgsmiHostHeapLock(HGSMIINSTANCE *pIns) +{ + int rc = RTCritSectEnter(&pIns->hostHeapCritSect); AssertRC (rc); return rc; } -static void hgsmiHostHeapUnlock (HGSMIINSTANCE *pIns) -{ - int rc = RTCritSectLeave (&pIns->hostHeapCritSect); +static void hgsmiHostHeapUnlock(HGSMIINSTANCE *pIns) +{ + int rc = RTCritSectLeave(&pIns->hostHeapCritSect); AssertRC (rc); } @@ -961,59 +962,45 @@ }; -int HGSMIHostHeapSetup(PHGSMIINSTANCE pIns, - HGSMIOFFSET offHeap, - HGSMISIZE cbHeap) +int HGSMIHostHeapSetup(PHGSMIINSTANCE pIns, HGSMIOFFSET RT_UNTRUSTED_GUEST offHeap, HGSMISIZE RT_UNTRUSTED_GUEST cbHeap) { LogFlowFunc(("pIns %p, offHeap 0x%08X, cbHeap = 0x%08X\n", pIns, offHeap, cbHeap)); - int rc = VINF_SUCCESS; - + /* + * Validate input. + */ AssertPtrReturn(pIns, VERR_INVALID_PARAMETER); - if ( offHeap >= pIns->area.cbArea - || cbHeap > pIns->area.cbArea - || offHeap > pIns->area.cbArea - cbHeap) - { - AssertLogRelMsgFailed(("offHeap 0x%08X, cbHeap = 0x%08X, pIns->area.cbArea 0x%08X\n", - offHeap, cbHeap, pIns->area.cbArea)); - rc = VERR_INVALID_PARAMETER; - } - else - { - rc = hgsmiHostHeapLock (pIns); - - if (RT_SUCCESS (rc)) - { - if (pIns->hostHeap.cRefs) - { - AssertLogRelMsgFailed(("HGSMI[%s]: host heap setup ignored. %d allocated.\n", - pIns->pszName, pIns->hostHeap.cRefs)); - /* It is possible to change the heap only if there is no pending allocations. */ - rc = VERR_ACCESS_DENIED; - } - else - { - rc = HGSMIAreaInitialize(&pIns->hostHeap.area, pIns->area.pu8Base + offHeap, cbHeap, offHeap); - if (RT_SUCCESS(rc)) - { - rc = HGSMIMAInit(&pIns->hostHeap.u.ma, &pIns->hostHeap.area, NULL, 0, 0, &g_hgsmiEnv); - } - - if (RT_SUCCESS(rc)) - { - pIns->hostHeap.u32HeapType = HGSMI_HEAP_TYPE_MA; - } - else - { - HGSMIAreaClear(&pIns->hostHeap.area); - } - } - - hgsmiHostHeapUnlock (pIns); - } - } + ASSERT_GUEST_LOGREL_MSG_RETURN( offHeap < pIns->area.cbArea + && cbHeap <= pIns->area.cbArea + && offHeap <= pIns->area.cbArea - cbHeap, + ("Heap: %#x LB %#x; Area: %#x LB %#x\n", offHeap, cbHeap, pIns->area.offBase, pIns->area.cbArea), + VERR_INVALID_PARAMETER); + RT_UNTRUSTED_VALIDATED_FENCE(); + + + /* + * Lock the heap and do the job. + */ + int rc = hgsmiHostHeapLock(pIns); + AssertReturn(rc, rc); + + /* It is possible to change the heap only if there is no pending allocations. */ + ASSERT_GUEST_LOGREL_MSG_STMT_RETURN(pIns->hostHeap.cRefs == 0, + ("HGSMI[%s]: host heap setup ignored. %d allocated.\n", pIns->pszName, pIns->hostHeap.cRefs), + hgsmiHostHeapUnlock(pIns), + VERR_ACCESS_DENIED); + rc = HGSMIAreaInitialize(&pIns->hostHeap.area, pIns->area.pu8Base + offHeap, cbHeap, offHeap); + if (RT_SUCCESS(rc)) + { + rc = HGSMIMAInit(&pIns->hostHeap.u.ma, &pIns->hostHeap.area, NULL, 0, 0, &g_hgsmiEnv); + if (RT_SUCCESS(rc)) + pIns->hostHeap.u32HeapType = HGSMI_HEAP_TYPE_MA; + else + HGSMIAreaClear(&pIns->hostHeap.area); + } + + hgsmiHostHeapUnlock(pIns); LogFlowFunc(("rc = %Rrc\n", rc)); - return rc; } @@ -1521,4 +1508,38 @@ +/** + * Checks if @a offBuffer is within the area of this instance. + * + * This is for use in input validations. + * + * @returns true / false. + * @param pIns The instance. + * @param offBuffer The buffer offset to check. + */ +bool HGSMIIsOffsetValid(PHGSMIINSTANCE pIns, HGSMIOFFSET offBuffer) +{ + return pIns + && offBuffer - pIns->area.offBase < pIns->area.cbArea; +} + + +/** + * Returns the area offset for use in logging and assertion messages. + */ +HGSMIOFFSET HGSMIGetAreaOffset(PHGSMIINSTANCE pIns) +{ + return pIns ? pIns->area.offBase : ~(HGSMIOFFSET)0; +} + + +/** + * Returns the area size for use in logging and assertion messages. + */ +HGSMIOFFSET HGSMIGetAreaSize(PHGSMIINSTANCE pIns) +{ + return pIns ? pIns->area.cbArea : 0; +} + + void *HGSMIContext (PHGSMIINSTANCE pIns) { Index: /trunk/src/VBox/Devices/Graphics/HGSMI/HGSMIHost.h =================================================================== --- /trunk/src/VBox/Devices/Graphics/HGSMI/HGSMIHost.h (revision 71618) +++ /trunk/src/VBox/Devices/Graphics/HGSMI/HGSMIHost.h (revision 71619) @@ -50,4 +50,8 @@ void RT_UNTRUSTED_VOLATILE_GUEST *HGSMIOffsetToPointerHost(PHGSMIINSTANCE pIns, HGSMIOFFSET offBuffer); HGSMIOFFSET HGSMIPointerToOffsetHost(PHGSMIINSTANCE pIns, const void RT_UNTRUSTED_VOLATILE_GUEST *pv); +bool HGSMIIsOffsetValid(PHGSMIINSTANCE pIns, HGSMIOFFSET offBuffer); +HGSMIOFFSET HGSMIGetAreaOffset(PHGSMIINSTANCE pIns); +HGSMIOFFSET HGSMIGetAreaSize(PHGSMIINSTANCE pIns); + int HGSMIHostChannelRegister(PHGSMIINSTANCE pIns, uint8_t u8Channel, @@ -61,5 +65,5 @@ #endif -int HGSMIHostHeapSetup(PHGSMIINSTANCE pIns, HGSMIOFFSET offHeap, HGSMISIZE cbHeap); +int HGSMIHostHeapSetup(PHGSMIINSTANCE pIns, HGSMIOFFSET RT_UNTRUSTED_GUEST offHeap, HGSMISIZE RT_UNTRUSTED_GUEST cbHeap); /* Index: /trunk/src/VBox/Main/include/DisplayImpl.h =================================================================== --- /trunk/src/VBox/Main/include/DisplayImpl.h (revision 71618) +++ /trunk/src/VBox/Main/include/DisplayImpl.h (revision 71619) @@ -87,5 +87,5 @@ bool fVBVAForceResize; bool fRenderThreadMode; - PVBVAHOSTFLAGS pVBVAHostFlags; + VBVAHOSTFLAGS RT_UNTRUSTED_VOLATILE_GUEST *pVBVAHostFlags; #endif /* VBOX_WITH_HGSMI */ @@ -363,9 +363,9 @@ #ifdef VBOX_WITH_HGSMI static DECLCALLBACK(int) i_displayVBVAEnable(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId, - PVBVAHOSTFLAGS pHostFlags, bool fRenderThreadMode); + VBVAHOSTFLAGS RT_UNTRUSTED_VOLATILE_GUEST *pHostFlags, bool fRenderThreadMode); static DECLCALLBACK(void) i_displayVBVADisable(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId); static DECLCALLBACK(void) i_displayVBVAUpdateBegin(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId); static DECLCALLBACK(void) i_displayVBVAUpdateProcess(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId, - PCVBVACMDHDR pCmd, size_t cbCmd); + struct VBVACMDHDR const RT_UNTRUSTED_VOLATILE_GUEST *pCmd, size_t cbCmd); static DECLCALLBACK(void) i_displayVBVAUpdateEnd(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId, int32_t x, int32_t y, uint32_t cx, uint32_t cy); Index: /trunk/src/VBox/Main/src-client/DisplayImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/DisplayImpl.cpp (revision 71618) +++ /trunk/src/VBox/Main/src-client/DisplayImpl.cpp (revision 71619) @@ -4038,5 +4038,9 @@ #ifdef VBOX_WITH_HGSMI -DECLCALLBACK(int) Display::i_displayVBVAEnable(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId, PVBVAHOSTFLAGS pHostFlags, +/** + * @interface_method_impl{PDMIDISPLAYCONNECTOR,pfnVBVAEnable} + */ +DECLCALLBACK(int) Display::i_displayVBVAEnable(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId, + VBVAHOSTFLAGS RT_UNTRUSTED_VOLATILE_GUEST *pHostFlags, bool fRenderThreadMode) { @@ -4065,4 +4069,7 @@ } +/** + * @interface_method_impl{PDMIDISPLAYCONNECTOR,pfnVBVADisable} + */ DECLCALLBACK(void) Display::i_displayVBVADisable(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId) { @@ -4124,8 +4131,14 @@ } +/** + * @interface_method_impl{PDMIDISPLAYCONNECTOR,pfnVBVAUpdateProcess} + */ DECLCALLBACK(void) Display::i_displayVBVAUpdateProcess(PPDMIDISPLAYCONNECTOR pInterface, unsigned uScreenId, - PCVBVACMDHDR pCmd, size_t cbCmd) + struct VBVACMDHDR const RT_UNTRUSTED_VOLATILE_GUEST *pCmd, size_t cbCmd) { LogFlowFunc(("uScreenId %d pCmd %p cbCmd %d, @%d,%d %dx%d\n", uScreenId, pCmd, cbCmd, pCmd->x, pCmd->y, pCmd->w, pCmd->h)); + VBVACMDHDR hdrSaved; + RT_COPY_VOLATILE(hdrSaved, *pCmd); + RT_UNTRUSTED_NONVOLATILE_COPY_FENCE(); PDRVMAINDISPLAY pDrv = PDMIDISPLAYCONNECTOR_2_MAINDISPLAY(pInterface); @@ -4139,5 +4152,5 @@ && !pFBInfo->fDisabled) { - pDrv->pUpPort->pfnUpdateDisplayRect(pDrv->pUpPort, pCmd->x, pCmd->y, pCmd->w, pCmd->h); + pDrv->pUpPort->pfnUpdateDisplayRect(pDrv->pUpPort, hdrSaved.x, hdrSaved.y, hdrSaved.w, hdrSaved.h); } else if ( !pFBInfo->pSourceBitmap.isNull() @@ -4160,10 +4173,10 @@ if (SUCCEEDED(hrc)) { - uint32_t width = pCmd->w; - uint32_t height = pCmd->h; + uint32_t width = hdrSaved.w; + uint32_t height = hdrSaved.h; const uint8_t *pu8Src = pFBInfo->pu8FramebufferVRAM; - int32_t xSrc = pCmd->x - pFBInfo->xOrigin; - int32_t ySrc = pCmd->y - pFBInfo->yOrigin; + int32_t xSrc = hdrSaved.x - pFBInfo->xOrigin; + int32_t ySrc = hdrSaved.y - pFBInfo->yOrigin; uint32_t u32SrcWidth = pFBInfo->w; uint32_t u32SrcHeight = pFBInfo->h; @@ -4193,6 +4206,8 @@ } - VBVACMDHDR hdrSaved = *pCmd; - + /* + * Here is your classic 'temporary' solution. + */ + /** @todo New SendUpdate entry which can get a separate cmd header or coords. */ VBVACMDHDR *pHdrUnconst = (VBVACMDHDR *)pCmd; @@ -4200,5 +4215,4 @@ pHdrUnconst->y -= (int16_t)pFBInfo->yOrigin; - /** @todo new SendUpdate entry which can get a separate cmd header or coords. */ pThis->mParent->i_consoleVRDPServer()->SendUpdate(uScreenId, pHdrUnconst, (uint32_t)cbCmd);