[ros-diffs] [arty] 34783: Fix several problems with lookasides and temporary captures: ob_x.h: Add a proper define for the size of a lookaside name buffer oblink.c: Use move memory rather than copy in the case that we re-use the name buffer. We probably never reused it before, because MaximumLength was never set properly. See below. oblife.c: Several things ObpCaptureObjectName - Properly set MaximumLength rather than copping out and setting it to just string + nul. This was dangerous because later, we'll u

Author: arty
Date: Fri Jul 25 08:42:05 2008
New Revision: 34783

URL: http://svn.reactos.org/svn/reactos?rev=34783&view=rev
Log:
Fix several problems with lookasides and temporary captures:
ob_x.h: Add a proper define for the size of a lookaside name buffer
oblink.c: Use move memory rather than copy in the case that we re-use the name
  buffer.  We probably never reused it before, because MaximumLength was never
  set properly.  See below.
oblife.c: Several things
  ObpCaptureObjectName
  - Properly set MaximumLength rather than copping out and setting it to just
    string + nul.  This was dangerous because later, we'll use MaximumLength
    to determine whether we allocated the name from the lookaside list or the
    heap.
  - Since we use MaximumLength to determine where the allocation came from
    make sure that MaximumLength never equals the magic value if the string
    comes from the heap for whatever reason.
  - Free the string using the right symmetry if we would fault copying.
  ObpCaptureObjectCreateInformation
  - We didn't allocate the ObjectCreateInfo, but we might've allocated the
    security descriptor, so free it if needed, rather than borking some non
    heap.

Modified:
    trunk/reactos/ntoskrnl/include/internal/ob_x.h
    trunk/reactos/ntoskrnl/ob/oblife.c
    trunk/reactos/ntoskrnl/ob/oblink.c

Modified: trunk/reactos/ntoskrnl/include/internal/ob_x.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/ob_x.h [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/ob_x.h [iso-8859-1] Fri Jul 25 08:42:05 2008
@@ -14,6 +14,8 @@
 #define OBP_LOCK_STATE_POST_ACQUISITION_SHARED      0xDDDD1234
 #define OBP_LOCK_STATE_RELEASED                     0xEEEE1234
 #define OBP_LOCK_STATE_INITIALIZED                  0xFFFF1234
+
+#define OBP_NAME_LOOKASIDE_MAX_SIZE 248
 
 ULONG
 FORCEINLINE

Modified: trunk/reactos/ntoskrnl/ob/oblife.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/oblife.c?rev=3…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/oblife.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ob/oblife.c [iso-8859-1] Fri Jul 25 08:42:05 2008
@@ -300,9 +300,18 @@
     MaximumLength = Length + sizeof(UNICODE_NULL);
 
     /* Check if we should use the lookaside buffer */
-    if (!(UseLookaside) || (MaximumLength > 248))
+    if (!(UseLookaside) || (MaximumLength > OBP_NAME_LOOKASIDE_MAX_SIZE))
     {
         /* Nope, allocate directly from pool */
+	/* Since we later use MaximumLength to detect that we're not allocating
+	 * from a list, we need at least MaximumLength + sizeof(UNICODE_NULL)
+	 * here.
+	 *
+	 * People do call this with UseLookasideList FALSE so the distinction
+	 * is critical.
+	 */
+	if (MaximumLength <= OBP_NAME_LOOKASIDE_MAX_SIZE)
+	    MaximumLength = OBP_NAME_LOOKASIDE_MAX_SIZE + sizeof(UNICODE_NULL);
         Buffer = ExAllocatePoolWithTag(PagedPool,
                                        MaximumLength,
                                        OB_NAME_TAG);
@@ -310,13 +319,13 @@
     else
     {
         /* Allocate from the lookaside */
-        //MaximumLength = 248; <= hack, we should actually set this...!
+	MaximumLength = OBP_NAME_LOOKASIDE_MAX_SIZE;
         Buffer = ObpAllocateObjectCreateInfoBuffer(LookasideNameBufferList);
     }
 
     /* Setup the string */
+    ObjectName->MaximumLength = (USHORT)MaximumLength;
     ObjectName->Length = (USHORT)Length;
-    ObjectName->MaximumLength = (USHORT)MaximumLength;
     ObjectName->Buffer = Buffer;
     return Buffer;
 }
@@ -328,7 +337,7 @@
     PVOID Buffer = Name->Buffer;
 
     /* We know this is a pool-allocation if the size doesn't match */
-    if (Name->MaximumLength != 248)
+    if (Name->MaximumLength != OBP_NAME_LOOKASIDE_MAX_SIZE)
     {
         /* Free it from the pool */
         ExFreePool(Buffer);
@@ -408,7 +417,7 @@
     {
         /* Handle exception and free the string buffer */
         Status = _SEH_GetExceptionCode();
-        if (StringBuffer) ExFreePool(StringBuffer);
+        if (StringBuffer) ObpFreeObjectNameBuffer(CapturedName);
     }
     _SEH_END;
 
@@ -477,7 +486,7 @@
                 if(!NT_SUCCESS(Status))
                 {
                     /* Capture failed, quit */
-                    ObjectCreateInfo->SecurityDescriptor = NULL;
+		    ObjectCreateInfo->SecurityDescriptor = NULL;
                     _SEH_LEAVE;
                 }
 
@@ -541,7 +550,10 @@
     }
 
     /* Cleanup if we failed */
-    if (!NT_SUCCESS(Status)) ObpFreeObjectCreateInformation(ObjectCreateInfo);
+    if (!NT_SUCCESS(Status))
+    {
+	ObpReleaseObjectCreateInformation(ObjectCreateInfo);
+    }
 
     /* Return status to caller */
     return Status;

Modified: trunk/reactos/ntoskrnl/ob/oblink.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ob/oblink.c?rev=3…
==============================================================================
--- trunk/reactos/ntoskrnl/ob/oblink.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ob/oblink.c [iso-8859-1] Fri Jul 25 08:42:05 2008
@@ -181,7 +181,7 @@
     if (RemainingName->Length)
     {
         /* Copy the new path */
-        RtlCopyMemory((PVOID)((ULONG_PTR)NewTargetPath + TargetPath->Length),
+        RtlMoveMemory((PVOID)((ULONG_PTR)NewTargetPath + TargetPath->Length),
                       RemainingName->Buffer,
                       RemainingName->Length);
     }