Rob Shearman : rpcrt4: Set the destination pointer in PointerUnmarshall before calling the referenced type 's unmarshalling routine.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 2a9fae756564fff3ec13922d360031ff2ce8a4e7
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=2a9fae756564fff3ec13922d360031ff2ce8a4e7

Author: Rob Shearman <rob at codeweavers.com>
Date:   Tue Dec  4 13:22:56 2007 +0000

rpcrt4: Set the destination pointer in PointerUnmarshall before calling the referenced type's unmarshalling routine.

When a pointer that is dereferenced is encountered then this can result 
in a stale pointer (i.e. the one that is marshalled into the buffer for 
the embedded pointer unmarshalling case) being used instead of the one 
that was intended.

---

 dlls/rpcrt4/ndr_marshall.c |   17 +++++------------
 dlls/rpcrt4/tests/server.c |    1 -
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/dlls/rpcrt4/ndr_marshall.c b/dlls/rpcrt4/ndr_marshall.c
index 84a1990..781143a 100644
--- a/dlls/rpcrt4/ndr_marshall.c
+++ b/dlls/rpcrt4/ndr_marshall.c
@@ -1020,8 +1020,8 @@ static void PointerUnmarshall(PMIDL_STUB_MESSAGE pStubMsg,
        * unmarshalling routine for the benefit of the deref code below */
       if (!fMustAlloc) {
         if (pSrcPointer) {
-          TRACE("pSrcPointer = %p\n", pSrcPointer);
-          base_ptr_val = pSrcPointer;
+          TRACE("setting *pPointer to %p\n", pSrcPointer);
+          *pPointer = base_ptr_val = pSrcPointer;
         } else
           fMustAlloc = TRUE;
       }
@@ -1030,11 +1030,9 @@ static void PointerUnmarshall(PMIDL_STUB_MESSAGE pStubMsg,
       /* the memory in a stub is never initialised, so we have to work out here
        * whether we have to initialise it so we can use the optimisation of
        * setting the pointer to the buffer, if possible, or set fMustAlloc to
-       * TRUE. As there is no space used in the buffer for pointers when using
-       * reference pointers we must allocate memory in this case */
-      if (type == RPC_FC_RP || attr & RPC_FC_P_DEREF) {
+       * TRUE. */
+      if (attr & RPC_FC_P_DEREF) {
         fMustAlloc = TRUE;
-        base_ptr_val = NULL;
       } else {
         base_ptr_val = NULL;
         *current_ptr = NULL;
@@ -1044,6 +1042,7 @@ static void PointerUnmarshall(PMIDL_STUB_MESSAGE pStubMsg,
     if (attr & RPC_FC_P_DEREF) {
       if (fMustAlloc) {
         base_ptr_val = NdrAllocate(pStubMsg, sizeof(void *));
+        *pPointer = base_ptr_val;
         current_ptr = (unsigned char **)base_ptr_val;
       } else
         current_ptr = *(unsigned char***)current_ptr;
@@ -1057,12 +1056,6 @@ static void PointerUnmarshall(PMIDL_STUB_MESSAGE pStubMsg,
     if (type == RPC_FC_FP)
       NdrFullPointerInsertRefId(pStubMsg->FullPtrXlatTables, pointer_id,
                                 base_ptr_val);
-
-    /* this must be done after the call to the unmarshaller, since when we are
-     * unmarshalling reference pointers on the server side *pPointer will be
-     * pointing to valid data */
-    if ((!fMustAlloc || attr & RPC_FC_P_DEREF) && base_ptr_val)
-      *pPointer = base_ptr_val;
   }
 
   TRACE("pointer=%p\n", *pPointer);
diff --git a/dlls/rpcrt4/tests/server.c b/dlls/rpcrt4/tests/server.c
index 538f5a4..433657b 100644
--- a/dlls/rpcrt4/tests/server.c
+++ b/dlls/rpcrt4/tests/server.c
@@ -877,7 +877,6 @@ pointer_tests(void)
   name.name = buffer = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, name.size);
   get_name(&name);
   ok(name.name == buffer, "[in,out] pointer should have stayed as %p but instead changed to %p\n", name.name, buffer);
-  todo_wine
   ok(!strcmp(name.name, "Jeremy Wh"), "name didn't unmarshall properly, expected \"Jeremy Wh\", but got \"%s\"\n", name.name);
   HeapFree(GetProcessHeap(), 0, name.name);