Re: [ros-dev] win2k poroblem

[Hartmut Birr]

Hi,
I get this again. I'm using cmd as login shell and starting the
explorer. I get hundreds of lines like this:
(ex/handle.c:721) Looking up invalid handle 0xffffffff
Frames: <ntoskrnl.exe:26efd (ex/handle.c:722 (ExpLookupHandleTableEntry))>
<ntoskrnl.exe:27576 (ex/handle.c:915 (ExMapHandleToPointer))>
<ntoskrnl.exe:748d8 (ps/cid.c:106 (PsLookupCidHandle))>
<ntoskrnl.exe:7c497 (ps/process.c:2709 (PsLookupProcessByProcessId))>
<win32k.sys:464d6 (objects/gdiobj.c:1219 (GDIOBJ_SetOwnership))>
<win32k.sys:6840 (eng/surface.c:466 (EngDeleteSurface))>
 <win32k.sys:52d16 (objects/text.c:1922 (NtGdiExtTextOut))>
<ntoskrnl.exe:3fb2 (D:\DOKUME~1\hb\LOKALE~1\Temp/ccUVaaaa.s:178
(KiSystemService))>
<gdi32.dll:9c22 (objects/text.c:272 (ExtTextOutW))>
The starting point does change but GDIOBJ_SetOwnership and later are
always the same. I attach my changes in ntoskrnl. I think that the
changes in ob/handle.c are not relevant. It is the smp build on my smp
machine.
- Hartmut
Filip Navara schrieb:
>   Hartmut Birr wrote:
> >   Hi,
> >  while looking for the console closing problem, I've seen that win2k
> >  calls PsLookupProcessByProcessId very often with a id of 0xffffffff.
> >  - Hartmut
> >  
>  I've put ASSERT(ProcessId != (HANDLE)-1) in PsLookupProcessByProcessId
>  on my local copy and it was never triggered. My test was booting to
>  Explorer and running OpenOffice.org 1.1.1 Word Processor...
> >   (ex/handle.c:721) Looking up invalid handle
> > 0xffffffff
> >  Frames:
> >  <ntoskrnl.exe:26f2d (ex/handle.c:722 (ExpLookupHandleTableEntry))>
> >  <ntoskrnl.exe:275ce (ex/handle.c:919 (ExMapHandleToPointer))>
> >  <ntoskrnl.exe:74af8 (ps/cid.c:106 (PsLookupCidHandle))>
> >  <ntoskrnl.exe:7c6d5 (ps/process.c:2709 (PsLookupProcessByProcessId))>
> >  <win32k.sys:45c16 (objects/gdiobj.c:1219 (GDIOBJ_SetOwnership))>
> >  
>  ^ Honestly I can't see how can you ever get ProcessId == -1 from this
>  line of code (assuming it's the correct line):
>     Status = PsLookupProcessByProcessId((HANDLE)((ULONG_PTR)PrevProcId
>  & ~0x1), &OldProcess);
>  Since a "binary and" with 0xfffffffe is performed, the result can
>  never be 0xfffffffff.
> >   <win32k.sys:6840 (eng/surface.c:466
> > (EngDeleteSurface))>
> >  <win32k.sys:52456 (objects/text.c:1922 (NtGdiExtTextOut))>
> >  <win32k.sys:539ef (objects/text.c:2770 (NtGdiTextOut))>
> >  <ntoskrnl.exe:3fb2 (D:\DOKUME~1\hb\LOKALE~1\Temp/ccgPaaaa.s:178
> >  (KiSystemService))>
> >  <gdi32.dll:99bc (objects/text.c:45 (TextOutW))>
> >  
>  Regards,
>  Filip
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  
M:\Sandbox\ros_work\reactos>set SVN_EDITOR=notepad
M:\Sandbox\ros_work\reactos>d:\programme\subversion\bin\svn.exe diff
ntoskrnl\ob\handle.c ntoskrnl\ex\handle.c
Index: ntoskrnl/ob/handle.c
===================================================================
--- ntoskrnl/ob/handle.c        (revision 14161)
+++ ntoskrnl/ob/handle.c        (working copy)
@@ -160,6 +160,7 @@
   POBJECT_HEADER ObjectHeader;
   LONG ExTargetHandle;
   LONG ExSourceHandle = HANDLE_TO_EX_HANDLE(SourceHandle);
+  ULONG NewHandleCount;
   PAGED_CODE();
@@ -194,8 +195,8 @@
      1 here, we're in big trouble... it would've been safe to increment and
      check the handle count without using interlocked functions because the
      entry is locked, which means the handle count can't change. */
-  InterlockedIncrement(&ObjectHeader->HandleCount);
-  ASSERT(ObjectHeader->HandleCount >= 2);
+  NewHandleCount = InterlockedIncrement(&ObjectHeader->HandleCount);
+  ASSERT(NewHandleCount >= 2);
   ExUnlockHandleTableEntry(SourceProcess->ObjectTable,
                            SourceHandleEntry);
@@ -323,7 +324,8 @@
      }
    /* Check for magic handle first */
-   if (SourceHandle == NtCurrentThread())
+   if (SourceHandle == NtCurrentThread() ||
+       SourceHandle == NtCurrentProcess())
      {
        PVOID ObjectBody;
@@ -425,10 +427,7 @@
     ObjectHeader = EX_HTE_TO_HDR(HandleTableEntry);
     if(InterlockedIncrement(&ObjectHeader->HandleCount) == 1)
     {
-      ObReferenceObjectByPointer(HEADER_TO_BODY(ObjectHeader),
-                                0,
-                                NULL,
-                                UserMode);
+      ObReferenceObject(HEADER_TO_BODY(ObjectHeader));
     }
   }
@@ -555,10 +554,7 @@
    {
      if(InterlockedIncrement(&ObjectHeader->HandleCount) == 1)
      {
-      ObReferenceObjectByPointer(ObjectBody,
-                                0,
-                                NULL,
-                                UserMode);
+       ObReferenceObject(ObjectBody);
      }
      *HandleReturn = EX_HANDLE_TO_HANDLE(ExHandle);
@@ -740,6 +736,7 @@
                                  HandleEntry);
         KeLeaveCriticalRegion();
+        ObDereferenceObject(ObjectBody);
         return(STATUS_OBJECT_TYPE_MISMATCH);
      }
@@ -756,6 +753,7 @@
        if (!(GrantedAccess & DesiredAccess) &&
            !((~GrantedAccess) & DesiredAccess))
          {
+             ObDereferenceObject(ObjectBody);
             CHECKPOINT;
             return(STATUS_ACCESS_DENIED);
          }
Index: ntoskrnl/ex/handle.c
===================================================================
--- ntoskrnl/ex/handle.c        (revision 14161)
+++ ntoskrnl/ex/handle.c        (working copy)
@@ -718,7 +718,9 @@
   }
   else
   {
-    DPRINT("Looking up invalid handle 0x%x\n", Handle);
+    DPRINT1("Looking up invalid handle 0x%x\n", Handle);
+    KeRosDumpStackFrames(NULL, 15);
+
   }
   return Entry;