Re:[ros-svn] [ion] 12748: Fix incorrect LPC Object export...we export pointers not the object itself! Also remove more TIME stuf

> Fix incorrect LPC Object export...we export pointers not the object itself!

Why?

Aren't DATA symbols exported by DLLs pointers?

Emanuele

____________________________________________________________
Libero ADSL: 3 mesi gratis e navighi a 1.2 Mega. E poi hai l'Adsl senza limiti a meno di 1 euro al giorno. 
Abbonati subito senza costi di attivazione su http://www.libero.it

[Filip Navara]

[e-mail] wrote:
> > Fix incorrect LPC Object export...we export pointers not the object itself!
>
> Why?
>
> Aren't DATA symbols exported by DLLs pointers?

Yes, they are! Alex's change is WRONG!

- Filip

[Hartmut Birr]

Filip Navara schrieb:
> [e-mail] wrote:
> > > Fix incorrect LPC Object export...we export pointers not the object 
> > > itself!
> >
> > Why?
> >
> > Aren't DATA symbols exported by DLLs pointers?
>
> Yes, they are! Alex's change is WRONG!
> Filip

I think that Alex's changes are correct. If Alex's changes are wrong, 
all other type object initialisations are also wrong. There is only one 
little bug, the EXPORTED/IMPORTED definition is missing.

- Hartmut

Index: ntoskrnl/lpc/port.c
===================================================================
--- ntoskrnl/lpc/port.c (Revision 12753)
+++ ntoskrnl/lpc/port.c (Arbeitskopie)
@@ -21,7 +21,7 @@

/* GLOBALS 
*******************************************************************/

-POBJECT_TYPE   LpcPortObjectType = 0;
+POBJECT_TYPE   EXPORTED LpcPortObjectType = NULL;
 ULONG          LpcpNextMessageId = 0; /* 0 is not a valid ID */
 FAST_MUTEX     LpcpLock; /* global internal sync in LPC facility */

Index: ntoskrnl/include/internal/port.h
===================================================================
--- ntoskrnl/include/internal/port.h    (Revision 12753)
+++ ntoskrnl/include/internal/port.h    (Arbeitskopie)
@@ -140,7 +140,13 @@
 NTSTATUS
 NiInitPort (VOID);

-extern POBJECT_TYPE    LpcPortObjectType;
+#ifdef __NTOSKRNL__
+extern POBJECT_TYPE EXPORTED LpcPortObjectType;
+#else
+/* Usually not necessary, because nobody should include an internal 
header from ntoskrnl. */
+extern POBJECT_TYPE IMPORTED LpcPortObjectType;
+#endif
+
 extern ULONG           LpcpNextMessageId;
 extern FAST_MUTEX      LpcpLock;

[Filip Navara]

Hartmut Birr wrote:
> I think that Alex's changes are correct. If Alex's changes are wrong, 
> all other type object initialisations are also wrong. There is only 
> one little bug, the EXPORTED/IMPORTED definition is missing.

Hmm, you might be right...

POBJECT_TYPE _LpcPortObjectType; /* <- Debug symbols */
extern NTOSAPI POBJECT_TYPE LpcPortObjectType; /* <- W32API headers */

POBJECT_TYPE _IoFileObjectType; /* <- Debug symbols */
extern POBJECT_TYPE *IoFileObjectType; /* <- Official DDK headers */
extern NTOSAPI POBJECT_TYPE IoFileObjectType; /* <- W32API headers */

- Filip

[Filip Navara]

Filip Navara wrote:
> Hartmut Birr wrote:
> > I think that Alex's changes are correct. If Alex's changes are wrong, 
> > all other type object initialisations are also wrong. There is only 
> > one little bug, the EXPORTED/IMPORTED definition is missing.
>
> Hmm, you might be right...

I have searched the net a bit and looked into my books.

Windows NT Filesystem Internals:
"...at system initialization time, the NT I/O Manager registers all the 
different I/O Manager objects (including the file object structure) with 
the NT Object Manager. The ObCreateObjectType() Object Manager routine 
is used for this purpose. Although this routine is not exposed by the NT 
Executive, it serves to make the NT Object Manager aware of a new object 
type. When invoking this routine, the I/O Manager also supplies the 
functions that must be invoked by the Object Manager to manipulate the 
object being defined. For file object structures, the I/O Manager 
supplies an internal routine called IopCloseFile() to be invoked 
whenever any handle associated with the file object has been closed."

The ObCreateObjectType function has prototype similar to this:

NTSTATUS STDCALL
    ObCreateObjectType(
        IN PUNICODE_STRING ObjectTypeName,
        IN POBJECT_TYPE_INITIALIZER ObjectTypeInitializer,
        IN ULONG Reserved,
        OUT POBJECT_TYPE *ObjectType);

typedef struct _OBJECT_TYPE_INITIALIZER
    {
        WORD Length;
        UCHAR UseDefaultObject;
        UCHAR CaseInsensitive;
        ULONG InvalidAttributes;
        GENERIC_MAPPING GenericMapping;
        ULONG ValidAccessMask;
        UCHAR SecurityRequired;
        UCHAR MaintainHandleCount;
        UCHAR MaintainTypeList;
        POOL_TYPE PoolType;
        ULONG DefaultPagedPoolCharge;
        ULONG DefaultNonPagedPoolCharge;
        /* Prototypes for the functions below are in our headers... */
        PVOID DumpProcedure;
        PVOID OpenProcedure;
        PVOID CloseProcedure;
        PVOID DeleteProcedure;
        PVOID ParseProcedure;
        PVOID SecurityProcedure;
        PVOID QueryNameProcedure;
        PVOID OkayToCloseProcedure;
    } OBJECT_TYPE_INITIALIZER, *POBJECT_TYPE_INITIALIZER;

and it's used like

RtlZeroMemory(&ObjectTypeInitializer, sizeof(OBJECT_TYPE_INITIALIZER));
    ObjectTypeInitializer.Length = sizeof(OBJECT_TYPE_INITIALIZER);
    ...
    RtlInitUnicodeString(&ObjectTypeName, L"Port");
    ObCreateObjectType(&ObjectTypeName, &ObjectTypeInitializer, 0, 
&LpcPortObjectType);

so yes, the exported *ObjectType variables are really of type POBJECT_TYPE.

My apologies to Alex.

Regards,
Filip

[Hartmut Birr]

Filip Navara schrieb:
> Filip Navara wrote:
> > Hartmut Birr wrote:
> > > I think that Alex's changes are correct. If Alex's changes are 
> > > wrong, all other type object initialisations are also wrong. There 
> > > is only one little bug, the EXPORTED/IMPORTED definition is missing.
> >
> > Hmm, you might be right...
>
> I have searched the net a bit and looked into my books.
>
> Windows NT Filesystem Internals:
> "...at system initialization time, the NT I/O Manager registers all 
> the different I/O Manager objects (including the file object 
> structure) with the NT Object Manager. The ObCreateObjectType() Object 
> Manager routine is used for this purpose. Although this routine is not 
> exposed by the NT Executive, it serves to make the NT Object Manager 
> aware of a new object type. When invoking this routine, the I/O 
> Manager also supplies the functions that must be invoked by the Object 
> Manager to manipulate the object being defined. For file object 
> structures, the I/O Manager supplies an internal routine called 
> IopCloseFile() to be invoked whenever any handle associated with the 
> file object has been closed."
>
> The ObCreateObjectType function has prototype similar to this:
>
> and it's used like
>
> RtlZeroMemory(&ObjectTypeInitializer, 
> sizeof(OBJECT_TYPE_INITIALIZER));
>    ObjectTypeInitializer.Length = sizeof(OBJECT_TYPE_INITIALIZER);
>    ...
>    RtlInitUnicodeString(&ObjectTypeName, L"Port");
>    ObCreateObjectType(&ObjectTypeName, &ObjectTypeInitializer, 0, 
> &LpcPortObjectType);
>
> so yes, the exported *ObjectType variables are really of type 
> POBJECT_TYPE.
>
> My apologies to Alex.
>
> Regards,
> Filip
>
> Ros-dev mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-dev

Hi,

I've add some code to my test driver:

Status = ZwCreateEvent(&hEvent, EVENT_ALL_ACCESS, NULL, 
NotificationEvent, FALSE);
  DPRINT1("%x\n", Status);
  if (NT_SUCCESS(Status))
  {
     Status = ObReferenceObjectByHandle(hEvent, EVENT_ALL_ACCESS, 
(POBJECT_TYPE)ExEventObjectType, KernelMode, (PVOID*)&Event, NULL);
     DPRINT1("%x\n", Status);
     if (NT_SUCCESS(Status))
     {
        ObDereferenceObject((PVOID)Event);
     }
     Status = ObReferenceObjectByHandle(hEvent, EVENT_ALL_ACCESS, 
(POBJECT_TYPE)&ExEventObjectType, KernelMode, (PVOID*)&Event, NULL);
     DPRINT1("%x\n", Status);
     if (NT_SUCCESS(Status))
     {
        ObDereferenceObject((PVOID)Event);
     }
     ZwClose(hEvent);
  }

The result on W2K is:

(memtest.c:185) 0
(memtest.c:189) 0
(memtest.c: 195) c0000024 -> STATUS_OBJECT_TYPE_MISMATCH

This means Alex's changes and the other object type initialisations are 
correct.

- Hartmut

[Emanuele Aliberti]

Hi Hartmut:
> I've add some code to my test driver:
>
> Status = ZwCreateEvent(&hEvent, EVENT_ALL_ACCESS, NULL, 
> NotificationEvent, FALSE);
>  DPRINT1("%x\n", Status);
>  if (NT_SUCCESS(Status))
>  {
>     Status = ObReferenceObjectByHandle(hEvent, EVENT_ALL_ACCESS, 
> (POBJECT_TYPE)ExEventObjectType, KernelMode, (PVOID*)&Event, NULL);
>     DPRINT1("%x\n", Status);
>     if (NT_SUCCESS(Status))
>     {
>        ObDereferenceObject((PVOID)Event);
>     }
>     Status = ObReferenceObjectByHandle(hEvent, EVENT_ALL_ACCESS, 
> (POBJECT_TYPE)&ExEventObjectType, KernelMode, (PVOID*)&Event, NULL);
>     DPRINT1("%x\n", Status);
>     if (NT_SUCCESS(Status))
>     {
>        ObDereferenceObject((PVOID)Event);
>     }
>     ZwClose(hEvent);
>  }
>
> The result on W2K is:
>
> (memtest.c:185) 0
> (memtest.c:189) 0
> (memtest.c: 195) c0000024 -> STATUS_OBJECT_TYPE_MISMATCH
>
> This means Alex's changes and the other object type initialisations 
> are correct.

Compiled with DDK headers or w32api's ones?

Before making LpcPortObjectType static, which you and Filip show is 
actually wrong with respect to DDK headers, my first try was allocating 
the *OBJECT_TYPE calling

ExAllocatePoolWithTag(NonPagedPool,sizeof(OBJECT_TYPE),TAG_OBJECT_TYPE);

which should be fine.

Emanuele

[Alex Ionescu]

Filip Navara wrote:
> My apologies to Alex.

Apoligies accepted.
> Regards,
> Filip

Best regards,
Alex Ionescu

[Emanuele Aliberti]

Alex Ionescu wrote:
> Apoligies accepted.

This, once again, suggests to add detailed comments to code changes when 
committing, or fixing a commit one assumes is wrong. I, first of all, 
had to say I had changed the type of LpcPortObjectType because of the 
w32api header and the fact that DATA symbols are pointers. When I saw 
the excerpts from the DDK headers posted by Filip, I realized I was 
fooled by a wrong definition. I should have checked that first! I assume 
these two definitions *are* different, aren't they?

extern POBJECT_TYPE *IoFileObjectType; /* <- Official DDK headers */
extern NTOSAPI POBJECT_TYPE IoFileObjectType; /* <- W32API headers */

Alex, you say, in another reply, we will build soon ros using the w32api 
headers. If so, should this be fixed?

Emanuele Aliberti

[Alex Ionescu]

Emanuele Aliberti wrote:
> Alex Ionescu wrote:
> > Apoligies accepted.
>
> This, once again, suggests to add detailed comments to code changes 
> when committing, or fixing a commit one assumes is wrong.

I assumed it was clear to anyone checking that *all* the other exported 
objects were *POINTERS*. It also makes sense that NT would export a 
pointer, not the actual structure. Since when does an app export a whole 
object?
> I, first of all, had to say I had changed the type of 
> LpcPortObjectType because of the w32api header and the fact that DATA 
> symbols are pointers.

DATA  symbols are pointers, and that's exactly what I've done. In the 
old code, the LpcPortObjectType was exported as a whole type instead.
> When I saw the excerpts from the DDK headers posted by Filip, I 
> realized I was fooled by a wrong definition. I should have checked 
> that first! I assume these two definitions *are* different, aren't they?
>
> extern POBJECT_TYPE *IoFileObjectType; /* <- Official DDK headers */
> extern NTOSAPI POBJECT_TYPE IoFileObjectType; /* <- W32API headers */

Slightly different in the way you access the object, but both export the 
pointer.
> Alex, you say, in another reply, we will build soon ros using the 
> w32api headers. If so, should this be fixed?

Eventually, yes, but right now I just want it to be able to build.

Best regards,
Alex Ionescu

[Mike Nordell]

Alex Ionescu wrote:
> Emanuele Aliberti wrote:

<snip>
> > extern POBJECT_TYPE *IoFileObjectType; /* <- Official DDK headers */
> > extern NTOSAPI POBJECT_TYPE IoFileObjectType; /* <- W32API headers */
>
> Slightly different in the way you access the object, but both export the
> pointer.

1. POBJECT_TYPE*, i.e. OBJECT_TYPE**
2. POBJECT_TYPE, i.e. OBJECT_TYPE*

There's a vital pointer indirection difference between the two.

/Mike

[Filip Navara]

Filip Navara wrote:
> [e-mail] wrote:
> > > Fix incorrect LPC Object export...we export pointers not the object 
> > > itself!
> >
> > Why?
> >
> > Aren't DATA symbols exported by DLLs pointers?
>
> Yes, they are! Alex's change is WRONG!

The attached patch reverts the incorrect changes in Alex's patch.

Index: ps/process.c
===================================================================
--- ps/process.c	(revision 12754)
+++ ps/process.c	(working copy)
@@ -745,7 +745,7 @@
      {
    Status = ObReferenceObjectByHandle(DebugPort,
    				   PORT_ALL_ACCESS,
-					   LpcPortObjectType,
+					   &LpcPortObjectType,
    				   UserMode,
    				   (PVOID*)&pDebugPort,
    				   NULL);   
@@ -767,7 +767,7 @@
      {
    Status = ObReferenceObjectByHandle(ExceptionPort,
    				   PORT_ALL_ACCESS,
-					   LpcPortObjectType,
+					   &LpcPortObjectType,
    				   UserMode,
    				   (PVOID*)&pExceptionPort,
    				   NULL);   
Index: ps/kill.c
===================================================================
--- ps/kill.c	(revision 12754)
+++ ps/kill.c	(working copy)
@@ -448,7 +448,7 @@

Status = ObReferenceObjectByHandle(PortHandle,
    			      PORT_ALL_ACCESS,
-				      LpcPortObjectType,
+				      &LpcPortObjectType,
    			      KeGetCurrentThread()->PreviousMode,
    			      (PVOID*)&TerminationPort,
    			      NULL);   
Index: include/internal/port.h
===================================================================
--- include/internal/port.h	(revision 12754)
+++ include/internal/port.h	(working copy)
@@ -140,7 +140,7 @@
 NTSTATUS
 NiInitPort (VOID);

-extern POBJECT_TYPE	LpcPortObjectType;
+extern OBJECT_TYPE	LpcPortObjectType;
 extern ULONG		LpcpNextMessageId;
 extern FAST_MUTEX	LpcpLock;

Index: lpc/create.c
===================================================================
--- lpc/create.c	(revision 12754)
+++ lpc/create.c	(working copy)
@@ -134,7 +134,7 @@

/* Ask Ob to create the object */
   Status = ObCreateObject (ExGetPreviousMode(),
-			   LpcPortObjectType,
+			   &LpcPortObjectType,
    		   ObjectAttributes,
    		   ExGetPreviousMode(),
    		   NULL,
Index: lpc/port.c
===================================================================
--- lpc/port.c	(revision 12754)
+++ lpc/port.c	(working copy)
@@ -21,7 +21,7 @@

/* GLOBALS *******************************************************************/

-POBJECT_TYPE	LpcPortObjectType = 0;
+OBJECT_TYPE	LpcPortObjectType;
 ULONG		LpcpNextMessageId = 0; /* 0 is not a valid ID */
 FAST_MUTEX	LpcpLock; /* global internal sync in LPC facility */

@@ -37,32 +37,28 @@
 NTSTATUS INIT_FUNCTION
 NiInitPort (VOID)
 {
-    /* Allocate Memory for the LPC Object */
-    LpcPortObjectType = ExAllocatePool(NonPagedPool, sizeof(OBJECT_TYPE));
-    RtlZeroMemory (LpcPortObjectType, sizeof (OBJECT_TYPE));
+   RtlRosInitUnicodeStringFromLiteral(&LpcPortObjectType.TypeName,L"Port");

-    RtlRosInitUnicodeStringFromLiteral(&LpcPortObjectType->TypeName,L"Port");
-   
-   LpcPortObjectType->Tag = TAG('L', 'P', 'R', 'T');
-   LpcPortObjectType->MaxObjects = ULONG_MAX;
-   LpcPortObjectType->MaxHandles = ULONG_MAX;
-   LpcPortObjectType->TotalObjects = 0;
-   LpcPortObjectType->TotalHandles = 0;
-   LpcPortObjectType->PagedPoolCharge = 0;
-   LpcPortObjectType->NonpagedPoolCharge = sizeof(EPORT);
-   LpcPortObjectType->Mapping = &ExpPortMapping;
-   LpcPortObjectType->Dump = NULL;
-   LpcPortObjectType->Open = NULL;
-   LpcPortObjectType->Close = NiClosePort;
-   LpcPortObjectType->Delete = NiDeletePort;
-   LpcPortObjectType->Parse = NULL;
-   LpcPortObjectType->Security = NULL;
-   LpcPortObjectType->QueryName = NULL;
-   LpcPortObjectType->OkayToClose = NULL;
-   LpcPortObjectType->Create = NiCreatePort;
-   LpcPortObjectType->DuplicationNotify = NULL;
+   LpcPortObjectType.Tag = TAG('L', 'P', 'R', 'T');
+   LpcPortObjectType.MaxObjects = ULONG_MAX;
+   LpcPortObjectType.MaxHandles = ULONG_MAX;
+   LpcPortObjectType.TotalObjects = 0;
+   LpcPortObjectType.TotalHandles = 0;
+   LpcPortObjectType.PagedPoolCharge = 0;
+   LpcPortObjectType.NonpagedPoolCharge = sizeof(EPORT);
+   LpcPortObjectType.Mapping = &ExpPortMapping;
+   LpcPortObjectType.Dump = NULL;
+   LpcPortObjectType.Open = NULL;
+   LpcPortObjectType.Close = NiClosePort;
+   LpcPortObjectType.Delete = NiDeletePort;
+   LpcPortObjectType.Parse = NULL;
+   LpcPortObjectType.Security = NULL;
+   LpcPortObjectType.QueryName = NULL;
+   LpcPortObjectType.OkayToClose = NULL;
+   LpcPortObjectType.Create = NiCreatePort;
+   LpcPortObjectType.DuplicationNotify = NULL;

-   ObpCreateTypeObject(LpcPortObjectType);
+   ObpCreateTypeObject(&LpcPortObjectType);

LpcpNextMessageId = 0;

Index: lpc/query.c
===================================================================
--- lpc/query.c	(revision 12754)
+++ lpc/query.c	(working copy)
@@ -50,7 +50,7 @@

Status = ObReferenceObjectByHandle (PortHandle,
    			      PORT_ALL_ACCESS,   /* AccessRequired */
-				      LpcPortObjectType,
+				      &LpcPortObjectType,
    			      UserMode,
    			      (PVOID *) & Port,
    			      NULL);
Index: lpc/complete.c
===================================================================
--- lpc/complete.c	(revision 12754)
+++ lpc/complete.c	(working copy)
@@ -46,7 +46,7 @@
    */
   Status = ObReferenceObjectByHandle (hServerSideCommPort,
    			      PORT_ALL_ACCESS,
-				      LpcPortObjectType,
+				      &LpcPortObjectType,
    			      UserMode,
    			      (PVOID*)&ReplyPort,
    			      NULL);
Index: lpc/connect.c
===================================================================
--- lpc/connect.c	(revision 12754)
+++ lpc/connect.c	(working copy)
@@ -66,7 +66,7 @@
    * Create a port to represent our side of the connection
    */
   Status = ObCreateObject (KernelMode,
-			   LpcPortObjectType,
+			   &LpcPortObjectType,
    		   NULL,
    		   KernelMode,
    		   NULL,
@@ -333,7 +333,7 @@
                                     0,
                                     NULL,
                                     PORT_ALL_ACCESS,  /* DesiredAccess */
-                                    LpcPortObjectType,
+                                    &LpcPortObjectType,
                                     UserMode,
                                     NULL,
                                     (PVOID*)&NamedPort);
@@ -547,7 +547,7 @@

Status = ObReferenceObjectByHandle(NamedPortHandle,
    			     PORT_ALL_ACCESS,
-				     LpcPortObjectType,
+				     &LpcPortObjectType,
    			     UserMode,
    			     (PVOID*)&NamedPort,
    			     NULL);
@@ -563,7 +563,7 @@
   if (AcceptIt)
     {
       Status = ObCreateObject(ExGetPreviousMode(),
-			      LpcPortObjectType,
+			      &LpcPortObjectType,
    		      NULL,
    		      ExGetPreviousMode(),
    		      NULL,
Index: lpc/send.c
===================================================================
--- lpc/send.c	(revision 12754)
+++ lpc/send.c	(working copy)
@@ -180,7 +180,7 @@

Status = ObReferenceObjectByHandle(PortHandle,
    			      PORT_ALL_ACCESS,
-				      LpcPortObjectType,
+				      &LpcPortObjectType,
    			      UserMode,
    			      (PVOID*)&Port,
    			      NULL);
@@ -231,7 +231,7 @@

Status = ObReferenceObjectByHandle(PortHandle,
    			      PORT_ALL_ACCESS, 
-				      LpcPortObjectType,
+				      &LpcPortObjectType,
    			      UserMode,
    			      (PVOID*)&Port,
    			      NULL);
Index: lpc/reply.c
===================================================================
--- lpc/reply.c	(revision 12754)
+++ lpc/reply.c	(working copy)
@@ -88,7 +88,7 @@

Status = ObReferenceObjectByHandle(PortHandle,
    			      PORT_ALL_ACCESS,   /* AccessRequired */
-				      LpcPortObjectType,
+				      &LpcPortObjectType,
    			      UserMode,
    			      (PVOID*)&Port,
    			      NULL);
@@ -154,7 +154,7 @@

Status = ObReferenceObjectByHandle(PortHandle,
    			      PORT_ALL_ACCESS,
-				      LpcPortObjectType,
+				      &LpcPortObjectType,
    			      UserMode,
    			      (PVOID*)&Port,
    			      NULL);