Index: /trunk/include/VBox/HostServices/VBoxClipboardSvc.h =================================================================== --- /trunk/include/VBox/HostServices/VBoxClipboardSvc.h (revision 82505) +++ /trunk/include/VBox/HostServices/VBoxClipboardSvc.h (revision 82506) @@ -286,6 +286,6 @@ /** Reads data in specified format from the host. * - * This function takes three parameters, a 32-bit format bit, a buffer - * and 32-bit number of bytes read (output). + * This function takes three parameters, a 32-bit format bit + * (VBOX_SHCL_FMT_XXX), a buffer and 32-bit number of bytes read (output). * * There was a period during 6.1 development where it would take five parameters @@ -305,5 +305,23 @@ */ #define VBOX_SHCL_GUEST_FN_DATA_READ 3 -/** Writes data in requested format to the host. */ +/** Writes requested data to the host. + * + * This function takes either 2 or 3 parameters. The last two parameters are a + * 32-bit format bit (VBOX_SHCL_FMT_XXX) and a data buffer holding the related + * data. The three parameter variant have a context ID first, which shall be a + * copy of the ID in the data request message. + * + * There was a period during 6.1 development where there would be a 5 parameter + * version of this, inserting an unused flags parameter between the context ID + * and the format bit, as well as a 32-bit data buffer size repate between the + * format bit and the data buffer. This format is still accepted, though + * deprecated. + * + * @retval VINF_SUCCESS on success. + * @retval VERR_INVALID_CLIENT_ID + * @retval VERR_WRONG_PARAMETER_COUNT + * @retval VERR_WRONG_PARAMETER_TYPE + * @retval VERR_INVALID_CONTEXT if the context ID didn't match up. + */ #define VBOX_SHCL_GUEST_FN_DATA_WRITE 4 @@ -704,49 +722,44 @@ typedef struct VBoxShClParmDataRead { - /** uint32_t, in: Requested format. */ + /** uint32_t, in: Requested format (VBOX_SHCL_FMT_XXX). */ HGCMFunctionParameter f32Format; - /** ptr, out: The data buffer to put the data in on success. */ + /** ptr, out: The data buffer to put the data in on success. */ HGCMFunctionParameter pData; - /** uint32_t, out: Size of returned data, if larger than the buffer, then no + /** uint32_t, out: Size of returned data, if larger than the buffer, then no * data was actually transferred and the guest must repeat the call. */ HGCMFunctionParameter cb32Needed; } VBoxShClParmDataRead; + #define VBOX_SHCL_CPARMS_DATA_READ 3 /**< The parameter count for VBOX_SHCL_GUEST_FN_DATA_READ. */ #define VBOX_SHCL_CPARMS_DATA_READ_61B 5 /**< The 6.1 dev cycle variant, see VBOX_SHCL_GUEST_FN_DATA_READ. */ /** @} */ -/** - * Writes clipboard data. - */ -typedef struct _VBoxShClWriteDataMsg -{ - VBGLIOCHGCMCALL hdr; - - union - { - struct - { - /** Returned format as requested in the VBOX_SHCL_HOST_MSG_READ_DATA message. */ - HGCMFunctionParameter format; /* IN uint32_t */ - /** Data. */ - HGCMFunctionParameter ptr; /* IN linear pointer. */ - } v0; - struct - { - /** uint64_t, out: Context ID. */ - HGCMFunctionParameter uContext; - /** uint32_t, out: Write flags; currently unused and must be set to 0. */ - HGCMFunctionParameter fFlags; - /** uint32_t, out: Requested format to read data in. */ - HGCMFunctionParameter uFormat; - /** uint32_t, out: Size of data (in bytes). */ - HGCMFunctionParameter cbData; - /** ptr, out: Actual data. */ - HGCMFunctionParameter pvData; - } v1; - } u; -} VBoxShClWriteDataMsg; - -#define VBOX_SHCL_CPARMS_WRITE_DATA 5 +/** @name + * @{ */ + +/** VBOX_SHCL_GUEST_FN_DATA_WRITE parameters. */ +typedef struct VBoxShClParmDataWrite +{ + /** uint64_t, in: Context ID from VBOX_SHCL_HOST_MSG_READ_DATA. */ + HGCMFunctionParameter id64Context; + /** uint32_t, in: The data format (VBOX_SHCL_FMT_XXX). */ + HGCMFunctionParameter f32Format; + /** ptr, in: The data. */ + HGCMFunctionParameter pData; +} VBoxShClParmDataWrite; + +/** Old VBOX_SHCL_GUEST_FN_DATA_WRITE parameters. */ +typedef struct VBoxShClParmDataWriteOld +{ + /** uint32_t, in: The data format (VBOX_SHCL_FMT_XXX). */ + HGCMFunctionParameter f32Format; + /** ptr, in: The data. */ + HGCMFunctionParameter pData; +} VBoxShClParmDataWriteOld; + +#define VBOX_SHCL_CPARMS_DATA_WRITE 3 /**< The variant used when VBOX_SHCL_GF_0_CONTEXT_ID is reported. */ +#define VBOX_SHCL_CPARMS_DATA_WRITE_OLD 2 /**< The variant used when VBOX_SHCL_GF_0_CONTEXT_ID isn't reported. */ +#define VBOX_SHCL_CPARMS_DATA_WRITE_61B 5 /**< The 6.1 dev cycle variant, see VBOX_SHCL_GUEST_FN_DATA_WRITE. */ +/** @} */ /** Index: /trunk/src/VBox/Additions/common/VBoxGuest/lib/VBoxGuestR3LibClipboard.cpp =================================================================== --- /trunk/src/VBox/Additions/common/VBoxGuest/lib/VBoxGuestR3LibClipboard.cpp (revision 82505) +++ /trunk/src/VBox/Additions/common/VBoxGuest/lib/VBoxGuestR3LibClipboard.cpp (revision 82506) @@ -2395,14 +2395,15 @@ LogFlowFuncEnter(); - VBoxShClWriteDataMsg Msg; - RT_ZERO(Msg); - - VBGL_HGCM_HDR_INIT(&Msg.hdr, idClient, - VBOX_SHCL_GUEST_FN_DATA_WRITE, 2); - - VbglHGCMParmUInt32Set(&Msg.u.v0.format, fFormat); - VbglHGCMParmPtrSet(&Msg.u.v0.ptr, pv, cb); - - int rc = VbglR3HGCMCall(&Msg.hdr, sizeof(Msg.hdr) + sizeof(Msg.u.v0)); + struct + { + VBGLIOCHGCMCALL Hdr; + VBoxShClParmDataWriteOld Parms; + } Msg; + + VBGL_HGCM_HDR_INIT(&Msg.Hdr, idClient, VBOX_SHCL_GUEST_FN_DATA_WRITE, VBOX_SHCL_CPARMS_DATA_WRITE_OLD); + VbglHGCMParmUInt32Set(&Msg.Parms.f32Format, fFormat); + VbglHGCMParmPtrSet(&Msg.Parms.pData, pv, cb); + + int rc = VbglR3HGCMCall(&Msg.Hdr, sizeof(Msg)); LogFlowFuncLeaveRC(rc); @@ -2435,19 +2436,18 @@ else { - VBoxShClWriteDataMsg Msg; - RT_ZERO(Msg); - - VBGL_HGCM_HDR_INIT(&Msg.hdr, pCtx->uClientID, - VBOX_SHCL_GUEST_FN_DATA_WRITE, VBOX_SHCL_CPARMS_WRITE_DATA); + struct + { + VBGLIOCHGCMCALL Hdr; + VBoxShClParmDataWrite Parms; + } Msg; + + VBGL_HGCM_HDR_INIT(&Msg.Hdr, pCtx->uClientID, VBOX_SHCL_GUEST_FN_DATA_WRITE, VBOX_SHCL_CPARMS_DATA_WRITE); + Msg.Parms.id64Context.SetUInt64(pCtx->uContextID); + Msg.Parms.f32Format.SetUInt32(pData->uFormat); + Msg.Parms.pData.SetPtr(pData->pvData, pData->cbData); LogFlowFunc(("CID=%RU32\n", pCtx->uContextID)); - Msg.u.v1.uContext.SetUInt64(pCtx->uContextID); - Msg.u.v1.uFormat.SetUInt32(pData->uFormat); - Msg.u.v1.fFlags.SetUInt32(0); - Msg.u.v1.cbData.SetUInt32(pData->cbData); - Msg.u.v1.pvData.SetPtr(pData->pvData, pData->cbData); - - rc = VbglR3HGCMCall(&Msg.hdr, sizeof(Msg)); + rc = VbglR3HGCMCall(&Msg.Hdr, sizeof(Msg)); } Index: /trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc-internal.h =================================================================== --- /trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc-internal.h (revision 82505) +++ /trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc-internal.h (revision 82506) @@ -146,5 +146,6 @@ RTCList queueMsg; /** The client's own event source. - * Needed for events which are not bound to a specific transfer. */ + * Needed for events which are not bound to a specific transfer. + * @todo r=bird: s/Events/EventSrc/ !! */ SHCLEVENTSOURCE Events; #ifdef VBOX_WITH_SHARED_CLIPBOARD_TRANSFERS Index: /trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp =================================================================== --- /trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp (revision 82505) +++ /trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp (revision 82506) @@ -1459,86 +1459,110 @@ LogFlowFuncEnter(); - if ( ShClSvcGetMode() != VBOX_SHCL_MODE_GUEST_TO_HOST - && ShClSvcGetMode() != VBOX_SHCL_MODE_BIDIRECTIONAL) - { + /* + * Check if the service mode allows this operation and whether the guest is + * supposed to be reading from the host. + */ + uint32_t uMode = ShClSvcGetMode(); + if ( uMode == VBOX_SHCL_MODE_BIDIRECTIONAL + || uMode == VBOX_SHCL_MODE_GUEST_TO_HOST) + { /* likely */ } + else return VERR_ACCESS_DENIED; - } - - /* Is the guest supposed to write any clipboard data from the host? */ - if (!(pClient->State.fFlags & SHCLCLIENTSTATE_FLAGS_WRITE_ACTIVE)) - return VERR_WRONG_ORDER; - - int rc; - - SHCLDATABLOCK dataBlock; - RT_ZERO(dataBlock); - + + /** @todo r=bird: This whole active flag stuff is broken, so disabling for now. */ + //if (pClient->State.fFlags & SHCLCLIENTSTATE_FLAGS_WRITE_ACTIVE) + //{ /* likely */ } + //else + // return VERR_WRONG_ORDER; + + /* + * Digest parameters. + * + * There are 3 different format here, formatunately no parameters have been + * switch around so it's plain sailing compared to the DATA_READ message. + */ + ASSERT_GUEST_RETURN(pClient->State.fGuestFeatures0 & VBOX_SHCL_GF_0_CONTEXT_ID + ? cParms == VBOX_SHCL_CPARMS_DATA_WRITE || cParms == VBOX_SHCL_CPARMS_DATA_WRITE_61B + : cParms == VBOX_SHCL_CPARMS_DATA_WRITE_OLD, + VERR_WRONG_PARAMETER_COUNT); + + uintptr_t iParm = 0; SHCLCLIENTCMDCTX cmdCtx; RT_ZERO(cmdCtx); - - if (!(pClient->State.fGuestFeatures0 & VBOX_SHCL_GF_0_CONTEXT_ID)) /* Legacy, Guest Additions < 6.1. */ - { - if (cParms != 2) - { - rc = VERR_INVALID_PARAMETER; - } - else - { - rc = HGCMSvcGetU32(&paParms[0], &dataBlock.uFormat); - if (RT_SUCCESS(rc)) - { - if (pClient->State.POD.uFormat == VBOX_SHCL_FMT_NONE) - pClient->State.POD.uFormat = dataBlock.uFormat; - - if ( dataBlock.uFormat == VBOX_SHCL_FMT_NONE - || dataBlock.uFormat != pClient->State.POD.uFormat) - { - LogFunc(("Invalid format (client=%RU32 vs host=%RU32)\n", dataBlock.uFormat, pClient->State.POD.uFormat)); - rc = VERR_INVALID_PARAMETER; - } - else - rc = HGCMSvcGetBuf(&paParms[1], &dataBlock.pvData, &dataBlock.cbData); - } - } + if (cParms > VBOX_SHCL_CPARMS_DATA_WRITE_OLD) + { + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_64BIT, VERR_WRONG_PARAMETER_TYPE); + cmdCtx.uContextID = paParms[iParm].u.uint64; + uint64_t const idCtxExpected = VBOX_SHCL_CONTEXTID_MAKE(pClient->State.uSessionID, pClient->Events.uID, + VBOX_SHCL_CONTEXTID_GET_EVENT(cmdCtx.uContextID)); + ASSERT_GUEST_MSG_RETURN(cmdCtx.uContextID == idCtxExpected, + ("Wrong context ID: %#RX64, expected %#RX64\n", cmdCtx.uContextID, idCtxExpected), + VERR_INVALID_CONTEXT); + iParm++; } else { - if (cParms != VBOX_SHCL_CPARMS_WRITE_DATA) - { - rc = VERR_INVALID_PARAMETER; - } - else - { - rc = HGCMSvcGetU64(&paParms[0], &cmdCtx.uContextID); - - /** @todo Handle paParms[1] flags. */ - - if (RT_SUCCESS(rc)) - rc = HGCMSvcGetU32(&paParms[2], &dataBlock.uFormat); - if (RT_SUCCESS(rc)) - rc = HGCMSvcGetBuf(&paParms[4], &dataBlock.pvData, &dataBlock.cbData); - } - } - + /** @todo supply CID from client state? Setting it in ShClSvcDataReadRequest? */ + } + if (cParms == VBOX_SHCL_CPARMS_DATA_WRITE_61B) + { + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_32BIT, VERR_WRONG_PARAMETER_TYPE); + ASSERT_GUEST_RETURN(paParms[iParm].u.uint32 == 0, VERR_INVALID_FLAGS); + iParm++; + } + SHCLDATABLOCK dataBlock; + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_32BIT, VERR_WRONG_PARAMETER_TYPE); /* Format bit. */ + dataBlock.uFormat = paParms[iParm].u.uint32; + iParm++; + if (cParms == VBOX_SHCL_CPARMS_DATA_WRITE_61B) + { + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_32BIT, VERR_WRONG_PARAMETER_TYPE); /* "cbData" - duplicates buffer size. */ + iParm++; + } + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_PTR, VERR_WRONG_PARAMETER_TYPE); /* Data buffer */ + dataBlock.pvData = paParms[iParm].u.pointer.addr; + dataBlock.cbData = paParms[iParm].u.pointer.size; + iParm++; + Assert(iParm == cParms); + + /* + * For some reason we need to do this (makes absolutely no sense to bird). + */ + /** @todo r=bird: I really don't get why you need the State.POD.uFormat + * member. I'm sure there is a reason. Incomplete code? */ + if (!(pClient->State.fGuestFeatures0 & VBOX_SHCL_GF_0_CONTEXT_ID)) + { + if (pClient->State.POD.uFormat == VBOX_SHCL_FMT_NONE) + pClient->State.POD.uFormat = dataBlock.uFormat; + /** @todo r=bird: this must be buggy to, I've disabled it without testing + * though. */ + //ASSERT_GUEST_MSG_RETURN(pClient->State.POD.uFormat == dataBlock.uFormat, + // ("Requested %#x, POD.uFormat=%#x\n", dataBlock.uFormat, pClient->State.POD.uFormat), + // VERR_BAD_EXE_FORMAT /*VERR_INTERNAL_ERROR*/); + } + + /* + * Write the data to the active host side clipboard. + */ + int rc; + if (g_ExtState.pfnExtension) + { + SHCLEXTPARMS parms; + RT_ZERO(parms); + parms.uFormat = dataBlock.uFormat; + parms.u.pvData = dataBlock.pvData; + parms.cbData = dataBlock.cbData; + + g_ExtState.pfnExtension(g_ExtState.pvExtension, VBOX_CLIPBOARD_EXT_FN_DATA_WRITE, &parms, sizeof(parms)); + rc = VINF_SUCCESS; + } + else + rc = ShClSvcImplWriteData(pClient, &cmdCtx, &dataBlock); if (RT_SUCCESS(rc)) { - if (g_ExtState.pfnExtension) - { - SHCLEXTPARMS parms; - RT_ZERO(parms); - - parms.uFormat = dataBlock.uFormat; - parms.u.pvData = dataBlock.pvData; - parms.cbData = dataBlock.cbData; - - g_ExtState.pfnExtension(g_ExtState.pvExtension, VBOX_CLIPBOARD_EXT_FN_DATA_WRITE, &parms, sizeof(parms)); - } - - rc = ShClSvcImplWriteData(pClient, &cmdCtx, &dataBlock); - if (RT_SUCCESS(rc)) - { - /* Remove "write active" flag after successful read again. */ - pClient->State.fFlags &= ~SHCLCLIENTSTATE_FLAGS_WRITE_ACTIVE; - } + /* Remove "write active" flag after successful read again. */ + /** @todo r=bird: This doesn't make any effing sense. What if the host + * wants to have the guest write it another format??? */ + pClient->State.fFlags &= ~SHCLCLIENTSTATE_FLAGS_WRITE_ACTIVE; } @@ -1848,19 +1872,30 @@ &parms, sizeof(parms)); } - - SHCLCLIENTCMDCTX cmdCtx; - RT_ZERO(cmdCtx); - - SHCLFORMATDATA formatData; - RT_ZERO(formatData); - - formatData.Formats = uFormats; - Assert(formatData.Formats != VBOX_SHCL_FMT_NONE); /* Sanity. */ - - rc = ShClSvcImplFormatAnnounce(pClient, &cmdCtx, &formatData); + else + { + + SHCLCLIENTCMDCTX cmdCtx; + RT_ZERO(cmdCtx); + + SHCLFORMATDATA formatData; + RT_ZERO(formatData); + + formatData.Formats = uFormats; + Assert(formatData.Formats != VBOX_SHCL_FMT_NONE); /* Sanity. */ + + rc = ShClSvcImplFormatAnnounce(pClient, &cmdCtx, &formatData); + } + + /** @todo r=bird: I'm not sure if the guest should be automatically allowed + * to write the host clipboard now. It would make more sense to disallow + * host clipboard reads until the host reports formats. + * + * The writes should only really be allowed upon request from the host, + * shouldn't they? (Though, I'm not sure, maybe there are situations + * where the guest side will just want to push the content over + * immediately while it's still available, I don't quite recall now... + */ if (RT_SUCCESS(rc)) - { pClient->State.fFlags |= SHCLCLIENTSTATE_FLAGS_WRITE_ACTIVE; - } } } @@ -1875,8 +1910,6 @@ case VBOX_SHCL_GUEST_FN_DATA_WRITE: - { rc = shClSvcGetDataWrite(pClient, cParms, paParms); break; - } case VBOX_SHCL_GUEST_FN_CANCEL: