[ros-diffs] [hbelusca] 60298: [NTOS] - Fix Job object session ID setting and comparison; fix a list initialization. - Correct some comments. - As Alex noticed it 7 years and 2 months ago, in revision 23197, th...

Author: hbelusca
Date: Sat Sep 21 22:59:24 2013
New Revision: 60298

URL: http://svn.reactos.org/svn/reactos?rev=60298&view=rev
Log:
[NTOS]
- Fix Job object session ID setting and comparison; fix a list initialization.
- Correct some comments.
- As Alex noticed it 7 years and 2 months ago, in revision 23197, the
ProcessSessionInformation case in the NtSetInformationProcess API doesn't set a new
session ID for the given process anymore (checked by myself too), because it is set once
and for all at process creation time and is stored inside the Process->Session
structure managed by MM. Therefore fake changing it: we just return success if the
user-defined value is the same as the session ID of the process, and otherwise we fail.

Modified:
    trunk/reactos/ntoskrnl/mm/ARM3/procsup.c
    trunk/reactos/ntoskrnl/ps/job.c
    trunk/reactos/ntoskrnl/ps/query.c

Modified: trunk/reactos/ntoskrnl/mm/ARM3/procsup.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/procsup.c…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/procsup.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/procsup.c	[iso-8859-1] Sat Sep 21 22:59:24 2013
@@ -1576,7 +1576,7 @@
 
     /* Get the session ID */
     SessionId = SessionGlobal->SessionId;
-    DPRINT1("Last process in sessino %lu going down!!!\n", SessionId);
+    DPRINT1("Last process in session %lu going down!!!\n", SessionId);
 
     /* Free the session page tables */
 #ifndef _M_AMD64

Modified: trunk/reactos/ntoskrnl/ps/job.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ps/job.c?rev=6029…
==============================================================================
--- trunk/reactos/ntoskrnl/ps/job.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ps/job.c	[iso-8859-1] Sat Sep 21 22:59:24 2013
@@ -169,8 +169,7 @@
                 ExAcquireRundownProtection(&Process->RundownProtect);
                 if(NT_SUCCESS(Status))
                 {
-                     // FIXME: This is broken
-                    if(Process->Job == NULL && PtrToUlong(Process->Session)
== Job->SessionId)
+                    if(Process->Job == NULL && PsGetProcessSessionId(Process)
== Job->SessionId)
                     {
                         /* Just store the pointer to the job object in the process,
we'll
                         assign it later. The reason we can't do this here is that
locking
@@ -272,9 +271,12 @@
         the list before it even gets added! */
         Job->JobLinks.Flink = NULL;
 
-        /* setup the job object */
+        /* setup the job object - FIXME: More to do! */
+        InitializeListHead(&Job->JobSetLinks);
         InitializeListHead(&Job->ProcessListHead);
-        Job->SessionId = PtrToUlong(CurrentProcess->Session); /* inherit the
session id from the caller, FIXME: broken */
+
+        /* inherit the session id from the caller */
+        Job->SessionId = PsGetProcessSessionId(CurrentProcess);
 
         Status = ExInitializeResource(&Job->JobLock);
         if(!NT_SUCCESS(Status))

Modified: trunk/reactos/ntoskrnl/ps/query.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ps/query.c?rev=60…
==============================================================================
--- trunk/reactos/ntoskrnl/ps/query.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ps/query.c	[iso-8859-1] Sat Sep 21 22:59:24 2013
@@ -1169,7 +1169,7 @@
             /* Getting VDM powers requires the SeTcbPrivilege */
             if (!SeSinglePrivilegeCheck(SeTcbPrivilege, PreviousMode))
             {
-                /* Bail out */
+                /* We don't hold the privilege, bail out */
                 Status = STATUS_PRIVILEGE_NOT_HELD;
                 DPRINT1("Need TCB privilege\n");
                 break;
@@ -1213,7 +1213,7 @@
             /* Setting the error port requires the SeTcbPrivilege */
             if (!SeSinglePrivilegeCheck(SeTcbPrivilege, PreviousMode))
             {
-                /* Can't set the session ID, bail out. */
+                /* We don't hold the privilege, bail out */
                 Status = STATUS_PRIVILEGE_NOT_HELD;
                 break;
             }
@@ -1332,10 +1332,12 @@
             /* Setting the session id requires the SeTcbPrivilege */
             if (!SeSinglePrivilegeCheck(SeTcbPrivilege, PreviousMode))
             {
-                /* Can't set the session ID, bail out. */
+                /* We don't hold the privilege, bail out */
                 Status = STATUS_PRIVILEGE_NOT_HELD;
                 break;
             }
+
+#if 0 // OLD AND DEPRECATED CODE!!!!
 
             /* FIXME - update the session id for the process token */
             //Status = PsLockProcess(Process, FALSE);
@@ -1372,6 +1374,27 @@
 
             /* Unlock the process */
             //PsUnlockProcess(Process);
+
+#endif
+
+            /*
+             * Since we cannot change the session ID of the given
+             * process anymore because it is set once and for all
+             * at process creation time and because it is stored
+             * inside the Process->Session structure managed by MM,
+             * we fake changing it: we just return success if the
+             * user-defined value is the same as the session ID of
+             * the process, and otherwise we fail.
+             */
+            if (SessionInfo.SessionId == PsGetProcessSessionId(Process))
+            {
+                Status = STATUS_SUCCESS;
+            }
+            else
+            {
+                Status = STATUS_ACCESS_DENIED;
+            }
+
             break;
 
         case ProcessPriorityClass:
@@ -1612,6 +1635,7 @@
             /* Setting 'break on termination' requires the SeDebugPrivilege */
             if (!SeSinglePrivilegeCheck(SeDebugPrivilege, PreviousMode))
             {
+                /* We don't hold the privilege, bail out */
                 Status = STATUS_PRIVILEGE_NOT_HELD;
                 break;
             }
@@ -1837,7 +1861,7 @@
             /* Only TCB can do this */
             if (!SeSinglePrivilegeCheck(SeTcbPrivilege, PreviousMode))
             {
-                /* Fail */
+                /* We don't hold the privilege, bail out */
                 DPRINT1("Need TCB to set IOPL\n");
                 Status = STATUS_PRIVILEGE_NOT_HELD;
                 break;
@@ -2366,6 +2390,7 @@
             /* Setting 'break on termination' requires the SeDebugPrivilege */
             if (!SeSinglePrivilegeCheck(SeDebugPrivilege, PreviousMode))
             {
+                /* We don't hold the privilege, bail out */
                 Status = STATUS_PRIVILEGE_NOT_HELD;
                 break;
             }