[ros-diffs] [ion] 23328: - Mega whammy jammy fix commit: - Fix bootcd by fixing some bugs in CDFS (same as in VFAT). - Fix Broken Installers and other I/O programs that couldn't, for example, create temporary directories. - Fix Firefox installers and other apps crashing due to a bug in NtSetInformationFile. - Fix File Objects being referenced twice resulting in IRP_MJ_CLOSE/CLEANUP never being sent and several memory leaks. - Fix File Object Lock being incorrectly created and then misused by mm/section code

Author: ion
Date: Fri Jul 28 02:26:40 2006
New Revision: 23328

URL: http://svn.reactos.org/svn/reactos?rev=23328&view=rev
Log:
- Mega whammy jammy fix commit:
  - Fix bootcd by fixing some bugs in CDFS (same as in VFAT).
  - Fix Broken Installers and other I/O programs that couldn't, for example, create
temporary directories.
  - Fix Firefox installers and other apps crashing due to a bug in NtSetInformationFile.
  - Fix File Objects being referenced twice resulting in IRP_MJ_CLOSE/CLEANUP never being
sent and several memory leaks.
  - Fix File Object Lock being incorrectly created and then misused by mm/section code.
  - Fix creation of File Object before setting up the IRP, to properly cleanup during
failures.
- Add failure code if ObCreateObject fails.

Modified:
    trunk/reactos/drivers/filesystems/cdfs/finfo.c
    trunk/reactos/ntoskrnl/KrnlFun.c
    trunk/reactos/ntoskrnl/io/iomgr/file.c
    trunk/reactos/ntoskrnl/io/iomgr/iofunc.c
    trunk/reactos/ntoskrnl/mm/section.c

Modified: trunk/reactos/drivers/filesystems/cdfs/finfo.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/cdfs/f…
==============================================================================
--- trunk/reactos/drivers/filesystems/cdfs/finfo.c (original)
+++ trunk/reactos/drivers/filesystems/cdfs/finfo.c Fri Jul 28 02:26:40 2006
@@ -152,16 +152,16 @@
   ASSERT(Fcb != NULL);
 
   NameLength = wcslen(Fcb->PathName) * sizeof(WCHAR);
-  if (*BufferLength < sizeof(FILE_NAME_INFORMATION) + NameLength)
+  NameInfo->FileNameLength = NameLength;
+  if (*BufferLength < (FIELD_OFFSET(FILE_NAME_INFORMATION, FileName) + NameLength))
     return STATUS_BUFFER_OVERFLOW;
 
-  NameInfo->FileNameLength = NameLength;
   RtlCopyMemory(NameInfo->FileName,
 		Fcb->PathName,
 		NameLength + sizeof(WCHAR));
 
   *BufferLength -=
-    (sizeof(FILE_NAME_INFORMATION) + NameLength + sizeof(WCHAR));
+    (FIELD_OFFSET(FILE_NAME_INFORMATION, FileName) + NameLength);
 
   return STATUS_SUCCESS;
 }

Modified: trunk/reactos/ntoskrnl/KrnlFun.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/KrnlFun.c?rev=233…
==============================================================================
--- trunk/reactos/ntoskrnl/KrnlFun.c (original)
+++ trunk/reactos/ntoskrnl/KrnlFun.c Fri Jul 28 02:26:40 2006
@@ -9,10 +9,7 @@
 //              Failure to respect this will *ACHIEVE NOTHING*.
 //
 // Io:
-//  - Fix double-reference in IopCreateFile.
 //  - See why queueing IRPs and cancelling them causes crashes.
-//  - Find out why 7zip can't create temporary folders due to deferred I/O
-//    completion in IopParseDevice when creating a new File Object.
 //  - Add SEH to some places where it's missing (MDLs, etc) (iofunc).
 //  - Add a generic Cleanup/Exception Routine (iofunc).
 //  - Add another parameter to IopCleanupFailedIrp.

Modified: trunk/reactos/ntoskrnl/io/iomgr/file.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/file.c?r…
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/file.c (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/file.c Fri Jul 28 02:26:40 2006
@@ -69,7 +69,12 @@
 
     /* Reference the DO */
     Status = IopReferenceDeviceObject(OriginalDeviceObject);
-    if (!NT_SUCCESS(Status)) return Status;
+    if (!NT_SUCCESS(Status))
+    {
+        /* We failed, return status */
+        OpenPacket->FinalStatus = Status;
+        return Status;
+    }
 
     /* Map the generic mask and set the new mapping in the access state */
     RtlMapGenericMask(&AccessState->RemainingDesiredAccess,
@@ -134,6 +139,85 @@
             DeviceObject = IoGetAttachedDevice(DeviceObject);
         }
     }
+
+    /* Allocate the IRP */
+    Irp = IoAllocateIrp(DeviceObject->StackSize, TRUE);
+    if (!Irp)
+    {
+        /* Dereference the device and VPB, then fail */
+        IopDereferenceDeviceObject(DeviceObject, FALSE);
+        if (Vpb) IopDereferenceVpb(Vpb);
+        return STATUS_INSUFFICIENT_RESOURCES;
+    }
+
+    /* Now set the IRP data */
+    Irp->RequestorMode = AccessMode;
+    Irp->Flags = IRP_CREATE_OPERATION |
+                 IRP_SYNCHRONOUS_API |
+                 IRP_DEFER_IO_COMPLETION;
+    Irp->Tail.Overlay.Thread = PsGetCurrentThread();
+    Irp->UserIosb = &IoStatusBlock;
+    Irp->MdlAddress = NULL;
+    Irp->PendingReturned = FALSE;
+    Irp->UserEvent = NULL;
+    Irp->Cancel = FALSE;
+    Irp->CancelRoutine = NULL;
+    Irp->Tail.Overlay.AuxiliaryBuffer = NULL;
+
+    /* Setup the security context */
+    SecurityContext.SecurityQos = SecurityQos;
+    SecurityContext.AccessState = AccessState;
+    SecurityContext.DesiredAccess = AccessState->RemainingDesiredAccess;
+    SecurityContext.FullCreateOptions = OpenPacket->CreateOptions;
+
+    /* Get the I/O Stack location */
+    StackLoc = (PEXTENDED_IO_STACK_LOCATION)IoGetNextIrpStackLocation(Irp);
+    StackLoc->Control = 0;
+
+    /* Check what kind of file this is */
+    switch (OpenPacket->CreateFileType)
+    {
+        /* Normal file */
+        case CreateFileTypeNone:
+
+            /* Set the major function and EA Length */
+            StackLoc->MajorFunction = IRP_MJ_CREATE;
+            StackLoc->Parameters.Create.EaLength = OpenPacket->EaLength;
+
+            /* Set the flags */
+            StackLoc->Flags = OpenPacket->Options;
+            StackLoc->Flags |= !(Attributes & OBJ_CASE_INSENSITIVE) ?
+                                SL_CASE_SENSITIVE: 0;
+            break;
+
+        /* Named pipe */
+        case CreateFileTypeNamedPipe:
+
+            /* Set the named pipe MJ and set the parameters */
+            StackLoc->MajorFunction = IRP_MJ_CREATE_NAMED_PIPE;
+            StackLoc->Parameters.CreatePipe.Parameters = 
+                OpenPacket->MailslotOrPipeParameters;
+            break;
+
+        /* Mailslot */
+        case CreateFileTypeMailslot:
+
+            /* Set the mailslot MJ and set the parameters */
+            StackLoc->MajorFunction = IRP_MJ_CREATE_MAILSLOT;
+            StackLoc->Parameters.CreateMailslot.Parameters =
+                OpenPacket->MailslotOrPipeParameters;
+            break;
+    }
+
+    /* Set the common data */
+    Irp->Overlay.AllocationSize = OpenPacket->AllocationSize;
+    Irp->AssociatedIrp.SystemBuffer = OpenPacket->EaBuffer;
+    StackLoc->Parameters.Create.Options = (OpenPacket->Disposition << 24) |
+                                          (OpenPacket->CreateOptions &
+                                           0xFFFFFF);
+    StackLoc->Parameters.Create.FileAttributes = OpenPacket->FileAttributes;
+    StackLoc->Parameters.Create.ShareAccess = OpenPacket->ShareAccess;
+    StackLoc->Parameters.Create.SecurityContext = &SecurityContext;
 
     /* Check if we really need to create an object */
     if (!UseDummyFile)
@@ -153,6 +237,21 @@
                                 0,
                                 0,
                                 (PVOID*)&FileObject);
+        if (!NT_SUCCESS(Status))
+        {
+            /* Create failed, free the IRP */
+            IoFreeIrp(Irp);
+
+            /* Dereference the device and VPB */
+            IopDereferenceDeviceObject(DeviceObject, FALSE);
+            if (Vpb) IopDereferenceVpb(Vpb);
+
+            /* We failed, return status */
+            OpenPacket->FinalStatus = Status;
+            return Status;
+        }
+
+        /* Clear the file object */
         RtlZeroMemory(FileObject, sizeof(FILE_OBJECT));
 
         /* Check if this is Synch I/O */
@@ -170,6 +269,13 @@
             }
         }
 
+        /* Check if this is synch I/O */
+        if (FileObject->Flags & FO_SYNCHRONOUS_IO)
+        {
+            /* Initialize the event. FIXME: Should be FALSE */
+            KeInitializeEvent(&FileObject->Lock, SynchronizationEvent, FALSE);
+        }
+
         /* Check if the caller requested no intermediate buffering */
         if (OpenPacket->CreateOptions & FILE_NO_INTERMEDIATE_BUFFERING)
         {
@@ -226,94 +332,9 @@
         FileObject->Flags |= FO_OPENED_CASE_SENSITIVE;
     }
 
-    /* Setup the security context */
-    SecurityContext.SecurityQos = SecurityQos;
-    SecurityContext.AccessState = AccessState;
-    SecurityContext.DesiredAccess = AccessState->RemainingDesiredAccess;
-    SecurityContext.FullCreateOptions = OpenPacket->CreateOptions;
-
-    /* Check if this is synch I/O */
-    if (FileObject->Flags & FO_SYNCHRONOUS_IO)
-    {
-        /* Initialize the event */
-        KeInitializeEvent(&FileObject->Lock, SynchronizationEvent, TRUE);
-    }
-
-    /* Allocate the IRP */
-    Irp = IoAllocateIrp(DeviceObject->StackSize, TRUE);
-    if (!Irp)
-    {
-        /* Dereference the device and VPB, then fail */
-        IopDereferenceDeviceObject(DeviceObject, FALSE);
-        if (Vpb) IopDereferenceVpb(Vpb);
-        return STATUS_INSUFFICIENT_RESOURCES;
-    }
-
-    /* Now set the IRP data */
+    /* Now set the file object */
     Irp->Tail.Overlay.OriginalFileObject = FileObject;
-    Irp->RequestorMode = AccessMode;
-    Irp->Flags = IRP_CREATE_OPERATION |
-                 IRP_SYNCHRONOUS_API |
-                 IRP_DEFER_IO_COMPLETION;
-    Irp->Tail.Overlay.Thread = PsGetCurrentThread();
-    Irp->UserEvent = &FileObject->Event;
-    Irp->UserIosb = &IoStatusBlock;
-    Irp->MdlAddress = NULL;
-    Irp->PendingReturned = FALSE;
-    Irp->UserEvent = NULL;
-    Irp->Cancel = FALSE;
-    Irp->CancelRoutine = NULL;
-    Irp->Tail.Overlay.AuxiliaryBuffer = NULL;
-
-    /* Get the I/O Stack location */
-    StackLoc = (PEXTENDED_IO_STACK_LOCATION)IoGetNextIrpStackLocation(Irp);
-    StackLoc->Control = 0;
     StackLoc->FileObject = FileObject;
-
-    /* Check what kind of file this is */
-    switch (OpenPacket->CreateFileType)
-    {
-        /* Normal file */
-        case CreateFileTypeNone:
-
-            /* Set the major function and EA Length */
-            StackLoc->MajorFunction = IRP_MJ_CREATE;
-            StackLoc->Parameters.Create.EaLength = OpenPacket->EaLength;
-
-            /* Set the flags */
-            StackLoc->Flags = OpenPacket->Options;
-            StackLoc->Flags |= !(Attributes & OBJ_CASE_INSENSITIVE) ?
-                                SL_CASE_SENSITIVE: 0;
-            break;
-
-        /* Named pipe */
-        case CreateFileTypeNamedPipe:
-
-            /* Set the named pipe MJ and set the parameters */
-            StackLoc->MajorFunction = IRP_MJ_CREATE_NAMED_PIPE;
-            StackLoc->Parameters.CreatePipe.Parameters = 
-                OpenPacket->MailslotOrPipeParameters;
-            break;
-
-        /* Mailslot */
-        case CreateFileTypeMailslot:
-
-            /* Set the mailslot MJ and set the parameters */
-            StackLoc->MajorFunction = IRP_MJ_CREATE_MAILSLOT;
-            StackLoc->Parameters.CreateMailslot.Parameters =
-                OpenPacket->MailslotOrPipeParameters;
-            break;
-    }
-
-    /* Set the common data */
-    Irp->Overlay.AllocationSize = OpenPacket->AllocationSize;
-    Irp->AssociatedIrp.SystemBuffer =OpenPacket->EaBuffer;
-    StackLoc->Parameters.Create.Options = (OpenPacket->Disposition << 24) |
-                                          (OpenPacket->CreateOptions &
-                                           0xFFFFFF);
-    StackLoc->Parameters.Create.FileAttributes = OpenPacket->FileAttributes;
-    StackLoc->Parameters.Create.ShareAccess = OpenPacket->ShareAccess;
-    StackLoc->Parameters.Create.SecurityContext = &SecurityContext;
 
     /* Check if the file object has a name */
     if (RemainingName->Length)
@@ -347,9 +368,6 @@
     /* Copy the name */
     RtlCopyUnicodeString(&FileObject->FileName, RemainingName);
 
-    /* Reference the file object */
-    ObReferenceObject(FileObject);
-
     /* Initialize the File Object event and set the FO */
     KeInitializeEvent(&FileObject->Event, NotificationEvent, FALSE);
     OpenPacket->FileObject = FileObject;
@@ -373,6 +391,7 @@
     {
         /* We'll have to complete it ourselves */
         ASSERT(!Irp->PendingReturned);
+        ASSERT(!Irp->MdlAddress );
 
         /* Completion happens at APC_LEVEL */
         KeRaiseIrql(APC_LEVEL, &OldIrql);
@@ -385,8 +404,8 @@
         FileObject->Event.Header.SignalState = 1;
 
         /* Now that we've signaled the events, de-associate the IRP */
-        RemoveEntryList(&Irp->ThreadListEntry);
-        InitializeListHead(&Irp->ThreadListEntry);
+        //RemoveEntryList(&Irp->ThreadListEntry);
+        //InitializeListHead(&Irp->ThreadListEntry);
 
         /* Check if the IRP had an input buffer */
         if ((Irp->Flags & IRP_BUFFERED_IO) &&

Modified: trunk/reactos/ntoskrnl/io/iomgr/iofunc.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/iofunc.c…
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/iofunc.c (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/iofunc.c Fri Jul 28 02:26:40 2006
@@ -2159,7 +2159,7 @@
         if (LocalEvent)
         {
             /* Then to a non-alertable wait */
-            Status = KeWaitForSingleObject(&Event,
+            Status = KeWaitForSingleObject(Event,
                                            Executive,
                                            PreviousMode,
                                            FALSE,

Modified: trunk/reactos/ntoskrnl/mm/section.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/section.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/section.c (original)
+++ trunk/reactos/ntoskrnl/mm/section.c Fri Jul 28 02:26:40 2006
@@ -155,7 +155,8 @@
 static NTSTATUS
 MmspWaitForFileLock(PFILE_OBJECT File)
 {
-   return KeWaitForSingleObject(&File->Lock, 0, KernelMode, FALSE, NULL);
+    return STATUS_SUCCESS;
+   //return KeWaitForSingleObject(&File->Lock, 0, KernelMode, FALSE, NULL);
 }
 
 
@@ -2476,7 +2477,7 @@
                                       TAG_MM_SECTION_SEGMENT);
       if (Segment == NULL)
       {
-         KeSetEvent((PVOID)&FileObject->Lock, IO_NO_INCREMENT, FALSE);
+         //KeSetEvent((PVOID)&FileObject->Lock, IO_NO_INCREMENT, FALSE);
          ObDereferenceObject(Section);
          ObDereferenceObject(FileObject);
          return(STATUS_NO_MEMORY);
@@ -2531,7 +2532,7 @@
    Section->FileObject = FileObject;
    Section->MaximumSize = MaximumSize;
    CcRosReferenceCache(FileObject);
-   KeSetEvent((PVOID)&FileObject->Lock, IO_NO_INCREMENT, FALSE);
+   //KeSetEvent((PVOID)&FileObject->Lock, IO_NO_INCREMENT, FALSE);
    *SectionObject = Section;
    return(STATUS_SUCCESS);
 }
@@ -3362,7 +3363,7 @@
    }
    Section->FileObject = FileObject;
    CcRosReferenceCache(FileObject);
-   KeSetEvent((PVOID)&FileObject->Lock, IO_NO_INCREMENT, FALSE);
+   //KeSetEvent((PVOID)&FileObject->Lock, IO_NO_INCREMENT, FALSE);
    *SectionObject = Section;
    return(Status);
 }