Index: /trunk/src/VBox/Devices/Graphics/DevVGA.h =================================================================== --- /trunk/src/VBox/Devices/Graphics/DevVGA.h (revision 55843) +++ /trunk/src/VBox/Devices/Graphics/DevVGA.h (revision 55844) @@ -661,6 +661,6 @@ void VBVARaiseIrqNoWait(PVGASTATE pVGAState, uint32_t fFlags); -int VBVAInfoView(PVGASTATE pVGAState, VBVAINFOVIEW *pView); -int VBVAInfoScreen(PVGASTATE pVGAState, VBVAINFOSCREEN *pScreen); +int VBVAInfoView(PVGASTATE pVGAState, const VBVAINFOVIEW *pView); +int VBVAInfoScreen(PVGASTATE pVGAState, const VBVAINFOSCREEN *pScreen); int VBVAGetInfoViewAndScreen(PVGASTATE pVGAState, uint32_t u32ViewIndex, VBVAINFOVIEW *pView, VBVAINFOSCREEN *pScreen); Index: /trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp =================================================================== --- /trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp (revision 55843) +++ /trunk/src/VBox/Devices/Graphics/DevVGA_VBVA.cpp (revision 55844) @@ -1991,59 +1991,68 @@ } -int VBVAInfoView(PVGASTATE pVGAState, VBVAINFOVIEW *pView) -{ - LogFlowFunc(("VBVA_INFO_VIEW: index %d, offset 0x%x, size 0x%x, max screen size 0x%x\n", - pView->u32ViewIndex, pView->u32ViewOffset, pView->u32ViewSize, pView->u32MaxScreenSize)); +int VBVAInfoView(PVGASTATE pVGAState, const VBVAINFOVIEW *pView) +{ + LogFlowFunc(("VBVA_INFO_VIEW: u32ViewIndex %d, u32ViewOffset 0x%x, u32ViewSize 0x%x, u32MaxScreenSize 0x%x\n", + pView->u32ViewIndex, pView->u32ViewOffset, pView->u32ViewSize, pView->u32MaxScreenSize)); PHGSMIINSTANCE pIns = pVGAState->pHGSMI; - VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext (pIns); - - /* @todo verify view data. */ - if (pView->u32ViewIndex >= pCtx->cViews) - { - Log(("View index too large %d!!!\n", - pView->u32ViewIndex)); - return VERR_INVALID_PARAMETER; - } - - pCtx->aViews[pView->u32ViewIndex].view = *pView; - - return VINF_SUCCESS; -} - -int VBVAInfoScreen(PVGASTATE pVGAState, VBVAINFOSCREEN *pScreen) -{ + VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pIns); + + if ( pView->u32ViewIndex < pCtx->cViews + && pView->u32ViewOffset <= pVGAState->vram_size + && pView->u32ViewSize <= pVGAState->vram_size + && pView->u32ViewOffset <= pVGAState->vram_size - pView->u32ViewSize + && pView->u32MaxScreenSize <= pView->u32ViewSize) + { + pCtx->aViews[pView->u32ViewIndex].view = *pView; + return VINF_SUCCESS; + } + + LogRelFlow(("VBVA_INFO_VIEW: invalid data: index %d(%d), offset 0x%x, size 0x%x, max 0x%x, vram size 0x%x\n", + pView->u32ViewIndex, pCtx->cViews, pView->u32ViewOffset, pView->u32ViewSize, + pView->u32MaxScreenSize, pVGAState->vram_size)); + return VERR_INVALID_PARAMETER; +} + +int VBVAInfoScreen(PVGASTATE pVGAState, const VBVAINFOSCREEN *pScreen) +{ + LogRel(("VBVA_INFO_SCREEN: [%d] @%d,%d %dx%d, line 0x%x, BPP %d, flags 0x%x\n", + pScreen->u32ViewIndex, pScreen->i32OriginX, pScreen->i32OriginY, + pScreen->u32Width, pScreen->u32Height, + pScreen->u32LineSize, pScreen->u16BitsPerPixel, pScreen->u16Flags)); + PHGSMIINSTANCE pIns = pVGAState->pHGSMI; - VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext (pIns); - VBVAINFOVIEW *pView = &pCtx->aViews[pScreen->u32ViewIndex].view; - /* Calculate the offset of the end of the screen so we can make - * sure it is inside the view. I assume that screen rollover is not - * implemented. */ - int64_t offEnd = (int64_t)pScreen->u32Height * pScreen->u32LineSize - + pScreen->u32Width + pScreen->u32StartOffset; - LogRel(("VBVA_INFO_SCREEN: [%d] @%d,%d %dx%d, line 0x%x, BPP %d, flags 0x%x\n", - pScreen->u32ViewIndex, pScreen->i32OriginX, pScreen->i32OriginY, - pScreen->u32Width, pScreen->u32Height, - pScreen->u32LineSize, pScreen->u16BitsPerPixel, pScreen->u16Flags)); - - if ( pScreen->u32ViewIndex < RT_ELEMENTS (pCtx->aViews) + VBVACONTEXT *pCtx = (VBVACONTEXT *)HGSMIContext(pIns); + + /* Allow pScreen->u16BitsPerPixel == 0 because legacy guest code used it for screen blanking. */ + if ( pScreen->u32ViewIndex < pCtx->cViews && pScreen->u16BitsPerPixel <= 32 && pScreen->u32Width <= UINT16_MAX && pScreen->u32Height <= UINT16_MAX - && pScreen->u32LineSize <= UINT16_MAX * 4 - && offEnd < pView->u32MaxScreenSize) - { - vbvaResize (pVGAState, &pCtx->aViews[pScreen->u32ViewIndex], pScreen); - return VINF_SUCCESS; - } - - LogRelFlow(("VBVA_INFO_SCREEN [%lu]: bad data: %lux%lu, line 0x%lx, BPP %u, start offset %lu, max screen size %lu\n", - (unsigned long)pScreen->u32ViewIndex, - (unsigned long)pScreen->u32Width, - (unsigned long)pScreen->u32Height, - (unsigned long)pScreen->u32LineSize, - (unsigned long)pScreen->u16BitsPerPixel, - (unsigned long)pScreen->u32StartOffset, - (unsigned long)pView->u32MaxScreenSize)); + && pScreen->u32LineSize <= UINT16_MAX * 4) + { + const VBVAINFOVIEW *pView = &pCtx->aViews[pScreen->u32ViewIndex].view; + const uint32_t u32BytesPerPixel = (pScreen->u16BitsPerPixel + 7) / 8; + if (pScreen->u32Width <= pScreen->u32LineSize / (u32BytesPerPixel? u32BytesPerPixel: 1)) + { + const uint64_t u64ScreenSize = (uint64_t)pScreen->u32LineSize * pScreen->u32Height; + if ( pScreen->u32StartOffset <= pView->u32ViewSize + && u64ScreenSize <= pView->u32MaxScreenSize + && pScreen->u32StartOffset <= pView->u32ViewSize - (uint32_t)u64ScreenSize) + { + vbvaResize(pVGAState, &pCtx->aViews[pScreen->u32ViewIndex], pScreen); + return VINF_SUCCESS; + } + + LogRelFlow(("VBVA_INFO_SCREEN: invalid data: size 0x%RX64, max 0x%RX32\n", + u64ScreenSize, pView->u32MaxScreenSize)); + } + } + else + { + LogRelFlow(("VBVA_INFO_SCREEN: invalid data: index %RU32(%RU32)\n", + pScreen->u32ViewIndex, pCtx->cViews)); + } + return VERR_INVALID_PARAMETER; } @@ -2241,5 +2250,6 @@ #endif - if (cbBuffer < sizeof (VBVAINFOVIEW)) + /* Expect at least one VBVAINFOVIEW structure. */ + if (cbBuffer < sizeof(VBVAINFOVIEW)) { rc = VERR_INVALID_PARAMETER; @@ -2248,12 +2258,11 @@ /* Guest submits an array of VBVAINFOVIEW structures. */ - VBVAINFOVIEW *pView = (VBVAINFOVIEW *)pvBuffer; - + const VBVAINFOVIEW *pView = (VBVAINFOVIEW *)pvBuffer; for (; - cbBuffer >= sizeof (VBVAINFOVIEW); - pView++, cbBuffer -= sizeof (VBVAINFOVIEW)) - { - VBVAINFOVIEW View = *pView; - rc = VBVAInfoView(pVGAState, &View); + cbBuffer >= sizeof(VBVAINFOVIEW); + ++pView, cbBuffer -= sizeof(VBVAINFOVIEW)) + { + VBVAINFOVIEW view = *pView; + rc = VBVAInfoView(pVGAState, &view); if (RT_FAILURE(rc)) break; @@ -2293,10 +2302,4 @@ case VBVA_INFO_SCREEN: { - if (cbBuffer < sizeof (VBVAINFOSCREEN)) - { - rc = VERR_INVALID_PARAMETER; - break; - } - #ifdef VBOX_WITH_CRHGSMI if (vboxCmdVBVAIsEnabled(pVGAState)) @@ -2308,6 +2311,11 @@ #endif - VBVAINFOSCREEN *pScreen = (VBVAINFOSCREEN *)pvBuffer; - VBVAINFOSCREEN Screen = *pScreen; + if (cbBuffer < sizeof(VBVAINFOSCREEN)) + { + rc = VERR_INVALID_PARAMETER; + break; + } + + VBVAINFOSCREEN Screen = *(VBVAINFOSCREEN *)pvBuffer; rc = VBVAInfoScreen(pVGAState, &Screen); } break; Index: /trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp =================================================================== --- /trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp (revision 55843) +++ /trunk/src/VBox/Devices/Graphics/DevVGA_VDMA.cpp (revision 55844) @@ -1437,52 +1437,76 @@ } +static int vboxVDMASetupScreenInfo(PVGASTATE pVGAState, VBVAINFOSCREEN *pScreen) +{ + const uint32_t u32ViewIndex = pScreen->u32ViewIndex; + const bool fDisabled = RT_BOOL(pScreen->u16Flags & VBVA_SCREEN_F_DISABLED); + + if (fDisabled) + { + if ( u32ViewIndex < pVGAState->cMonitors + || u32ViewIndex == UINT32_C(0xFFFFFFFF)) + { + RT_ZERO(*pScreen); + pScreen->u32ViewIndex = u32ViewIndex; + pScreen->u16Flags = VBVA_SCREEN_F_ACTIVE | VBVA_SCREEN_F_DISABLED; + return VINF_SUCCESS; + } + } + else + { + if ( u32ViewIndex < pVGAState->cMonitors + && pScreen->u16BitsPerPixel <= 32 + && pScreen->u32Width <= UINT16_MAX + && pScreen->u32Height <= UINT16_MAX + && pScreen->u32LineSize <= UINT16_MAX * 4) + { + const uint32_t u32BytesPerPixel = (pScreen->u16BitsPerPixel + 7) / 8; + if (pScreen->u32Width <= pScreen->u32LineSize / (u32BytesPerPixel? u32BytesPerPixel: 1)) + { + const uint64_t u64ScreenSize = (uint64_t)pScreen->u32LineSize * pScreen->u32Height; + if ( pScreen->u32StartOffset <= pVGAState->vram_size + && u64ScreenSize <= pVGAState->vram_size + && pScreen->u32StartOffset <= pVGAState->vram_size - (uint32_t)u64ScreenSize) + { + return VINF_SUCCESS; + } + } + } + } + + return VERR_INVALID_PARAMETER; +} + static int vboxVDMACrGuestCtlResizeEntryProcess(struct VBOXVDMAHOST *pVdma, VBOXCMDVBVA_RESIZE_ENTRY *pEntry) { PVGASTATE pVGAState = pVdma->pVGAState; VBVAINFOSCREEN Screen = pEntry->Screen; + + /* Verify and cleanup local copy of the input data. */ + int rc = vboxVDMASetupScreenInfo(pVGAState, &Screen); + if (RT_FAILURE(rc)) + { + WARN(("invalid screen data\n")); + return rc; + } + + VBOXCMDVBVA_SCREENMAP_DECL(uint32_t, aTargetMap); + memcpy(aTargetMap, pEntry->aTargetMap, sizeof(aTargetMap)); + ASMBitClearRange(aTargetMap, pVGAState->cMonitors, VBOX_VIDEO_MAX_SCREENS); + + rc = pVdma->CrSrvInfo.pfnResize(pVdma->CrSrvInfo.hSvr, &Screen, aTargetMap); + if (RT_FAILURE(rc)) + { + WARN(("pfnResize failed %d\n", rc)); + return rc; + } + + /* A fake view which contains the current screen for the 2D VBVAInfoView. */ VBVAINFOVIEW View; - VBOXCMDVBVA_SCREENMAP_DECL(uint32_t, aTargetMap); - uint32_t u32ViewIndex = Screen.u32ViewIndex; - uint16_t u16Flags = Screen.u16Flags; - bool fDisable = false; - - memcpy(aTargetMap, pEntry->aTargetMap, sizeof (aTargetMap)); - - ASMBitClearRange(aTargetMap, pVGAState->cMonitors, VBOX_VIDEO_MAX_SCREENS); - - if (u16Flags & VBVA_SCREEN_F_DISABLED) - { - fDisable = true; - memset(&Screen, 0, sizeof (Screen)); - Screen.u32ViewIndex = u32ViewIndex; - Screen.u16Flags = VBVA_SCREEN_F_ACTIVE | VBVA_SCREEN_F_DISABLED; - } - - if (u32ViewIndex > pVGAState->cMonitors) - { - if (u32ViewIndex != 0xffffffff) - { - WARN(("invalid view index\n")); - return VERR_INVALID_PARAMETER; - } - else if (!fDisable) - { - WARN(("0xffffffff view index only valid for disable requests\n")); - return VERR_INVALID_PARAMETER; - } - } - View.u32ViewOffset = 0; View.u32ViewSize = Screen.u32LineSize * Screen.u32Height + Screen.u32StartOffset; - View.u32MaxScreenSize = View.u32ViewSize + Screen.u32Width + 1; /* <- make VBVAInfoScreen logic (offEnd < pView->u32MaxScreenSize) happy */ - - int rc = VINF_SUCCESS; - - rc = pVdma->CrSrvInfo.pfnResize(pVdma->CrSrvInfo.hSvr, &Screen, aTargetMap); - if (RT_FAILURE(rc)) - { - WARN(("pfnResize failed %d\n", rc)); - return rc; - } + View.u32MaxScreenSize = Screen.u32LineSize * Screen.u32Height; + + const bool fDisable = RT_BOOL(Screen.u16Flags & VBVA_SCREEN_F_DISABLED); for (int i = ASMBitFirstSet(aTargetMap, pVGAState->cMonitors); @@ -1520,9 +1544,4 @@ } } - - if (RT_FAILURE(rc)) - return rc; - - Screen.u32ViewIndex = u32ViewIndex; return rc;