Re: [ros-dev] [ros-diffs] 07/18: [WIN32K:NTUSER] Get rid of the cached window station Name member, and instead just use the name stored in the NT Object's header. CORE-11933 and PR #621.
10 Dec
2018
10 Dec
'18
11:49 a.m.
Hello Timo,<br>
<br>
Let's make a deal: while I fix this in win32k, you may try to find a way to fix that
one too in the SAC driver line 1192:<br>
<br>
https://git.reactos.org/?p=reactos.git;a=blob;f=drivers/sac/driver/util.c;h…
<br>
I used the object macros because I didn't want to have the (huge) overhead of calling
ObQueryNameString() together with allocating memory buffers just to retrieve the necessary
names that already exist somewhere. (Also we note that WinStation objects, by their
nature, really wrap closely around the NT objects and that's why I didn't see any
inconvenience in using the helper macros for retrieving said information).<br>
<br>
Best,<br>
Hermes<span style="font-family:arial,helvetica,sans-serif;
font-size:12px"></span>
<div class="gl_quote" style="margin-top: 20px; padding-top:
5px;">De : "Timo Kreuzer"<br>
A : ros-dev(a)reactos.xn--org,"herms-7mb Bélusca-Maïto"<br>
Envoyé: lundi 10 décembre 2018 01:44<br>
Objet : Re: [ros-diffs] 07/18: [WIN32K:NTUSER] Get rid of the cached window station Name
member, and instead just use the name stored in the NT Object's header. CORE-11933 and
PR #621.<br>
<div class="gl_quoted"><br>
Why did you remove an abstraction and create additional dependencies on<br>
internal implementation details?<br>
<br>
Win32k is not supposed to directly access internal kernel structures!<br>
The headers and macros shouldn't even be in NDK.<br>
<br>
Please revert or fix this. And while you are at it, put an "#ifdef<br>
_NTOSKRNL_" around the stuff in NDK to prevent people from using it.<br>
<br>
Thanks,<br>
Timo<br>
<br>
<br>
Am 19.08.2018 um 22:16 schrieb Hermès Bélusca-Maïto:<br>
>
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=43e2ab208a2d3d50b12b4…
><br>
> commit 43e2ab208a2d3d50b12b4689347f57ca83568dd9<br>
> Author: Hermès Bélusca-Maïto<br>
> AuthorDate: Sun Jun 17 19:40:32 2018 +0200<br>
> Commit: Hermès Bélusca-Maïto<br>
> CommitDate: Sun Aug 19 22:18:32 2018 +0200<br>
><br>
> [WIN32K:NTUSER] Get rid of the cached window station Name member, and instead
just use the name stored in the NT Object's header.<br>
> CORE-11933 and PR #621.<br>
><br>
> - Remove the related hack-FIXMEs;<br>
> - Adjust NtUserGetObjectInformation() in accordance.<br>
> - Retrieve the window-station/desktop object type string in
NtUserGetObjectInformation()<br>
> also from the NT Object's header.<br>
><br>
> Also simplify the UOI_FLAGS case of NtUserGetObjectInformation() by
reading<br>
> the handle inheritance information directly from the
OBJECT_HANDLE_INFORMATION<br>
> structure returned by ObReferenceObjectByHandle().<br>
> ---<br>
> win32ss/user/ntuser/sysparams.c | 3 +-<br>
> win32ss/user/ntuser/winsta.c | 106
+++++++++++++++++++---------------------<br>
> win32ss/user/ntuser/winsta.h | 1 -<br>
> 3 files changed, 51 insertions(+), 59 deletions(-)<br>
><br>
> diff --git a/win32ss/user/ntuser/sysparams.c
b/win32ss/user/ntuser/sysparams.c<br>
> index 7eedc028de..d0badba00e 100644<br>
> --- a/win32ss/user/ntuser/sysparams.c<br>
> +++ b/win32ss/user/ntuser/sysparams.c<br>
> @@ -33,7 +33,8 @@ BOOL g_PaintDesktopVersion = FALSE;<br>
> } \<br>
> else \<br>
> { \<br>
> - ERR("NtUserSystemParametersInfo requires interactive window station
(current is %wZ)\n", &GetW32ProcessInfo()->prpwinsta->Name);
\<br>
> + ERR("NtUserSystemParametersInfo requires interactive window station
(current is %wZ)\n", \<br>
> +
&(OBJECT_HEADER_TO_NAME_INFO(OBJECT_TO_OBJECT_HEADER(GetW32ProcessInfo()->prpwinsta))->Name));
\<br>
> } \<br>
> EngSetLastError(err); \<br>
> return 0; \<br>
> diff --git a/win32ss/user/ntuser/winsta.c
b/win32ss/user/ntuser/winsta.c<br>
> index f373b1cedf..ba1b1eb57d 100644<br>
> --- a/win32ss/user/ntuser/winsta.c<br>
> +++ b/win32ss/user/ntuser/winsta.c<br>
> @@ -114,8 +114,6 @@ IntWinStaObjectDelete(<br>
><br>
> RtlDestroyAtomTable(WinSta->AtomTable);<br>
><br>
> - RtlFreeUnicodeString(&WinSta->Name);<br>
> -<br>
> return STATUS_SUCCESS;<br>
> }<br>
><br>
> @@ -449,8 +447,6 @@ IntCreateWindowStation(<br>
> RtlZeroMemory(WindowStationObject, sizeof(WINSTATION_OBJECT));<br>
><br>
>
InitializeListHead(&WindowStationObject->DesktopListHead);<br>
> - WindowStationObject->Name =
*ObjectAttributes->ObjectName;<br>
> - ObjectAttributes->ObjectName = NULL; // FIXME! (see
NtUserCreateWindowStation())<br>
> WindowStationObject->dwSessionId =
NtCurrentPeb()->SessionId;<br>
> Status = RtlCreateAtomTable(37,
&WindowStationObject->AtomTable);<br>
> if (!NT_SUCCESS(Status))<br>
> @@ -491,7 +487,7 @@ IntCreateWindowStation(<br>
> }<br>
><br>
> TRACE("IntCreateWindowStation created object 0x%p with name %wZ handle
0x%p\n",<br>
> - WindowStationObject, &WindowStationObject->Name,
WindowStation);<br>
> + WindowStationObject, ObjectAttributes->ObjectName,
WindowStation);<br>
><br>
> *phWinSta = WindowStation;<br>
> return STATUS_SUCCESS;<br>
> @@ -582,23 +578,7 @@ NtUserCreateWindowStation(<br>
> return NULL;<br>
> }<br>
><br>
> - WindowStationName.Length = wcslen(ServiceWinStaName) *
sizeof(WCHAR);<br>
> - WindowStationName.MaximumLength =<br>
> - WindowStationName.Length + sizeof(UNICODE_NULL);<br>
> - WindowStationName.Buffer =<br>
> - ExAllocatePoolWithTag(PagedPool,<br>
> - WindowStationName.MaximumLength,<br>
> - TAG_STRING);<br>
> - if (!WindowStationName.Buffer)<br>
> - {<br>
> - Status = STATUS_NO_MEMORY;<br>
> - ERR("Impossible to build a valid window station name, Status
0x%08lx\n", Status);<br>
> - SetLastNtError(Status);<br>
> - return NULL;<br>
> - }<br>
> - RtlStringCbCopyW(WindowStationName.Buffer,<br>
> - WindowStationName.MaximumLength,<br>
> - ServiceWinStaName);<br>
> + RtlInitUnicodeString(&WindowStationName, ServiceWinStaName);<br>
> LocalObjectAttributes.ObjectName = &WindowStationName;<br>
> AccessMode = KernelMode;<br>
> }<br>
> @@ -615,12 +595,7 @@ NtUserCreateWindowStation(<br>
> Unknown5,<br>
> Unknown6);<br>
><br>
> - // FIXME! Because in some situations we store the allocated window station
name<br>
> - // inside the window station, we must not free it now! We know this fact
when<br>
> - // IntCreateWindowStation() sets LocalObjectAttributes.ObjectName to
NULL.<br>
> - // This hack must be removed once we just use the stored Ob name
instead<br>
> - // (in which case we will always free the allocated name here).<br>
> - if (LocalObjectAttributes.ObjectName)<br>
> + if ((AccessMode == UserMode) &&
LocalObjectAttributes.ObjectName)<br>
> ExFreePoolWithTag(LocalObjectAttributes.ObjectName->Buffer,
TAG_STRING);<br>
><br>
> if (NT_SUCCESS(Status))<br>
> @@ -802,7 +777,11 @@ NtUserGetObjectInformation(<br>
> NTSTATUS Status;<br>
> PWINSTATION_OBJECT WinStaObject = NULL;<br>
> PDESKTOP DesktopObject = NULL;<br>
> + POBJECT_HEADER ObjectHeader;<br>
> + POBJECT_HEADER_NAME_INFO NameInfo;<br>
> + OBJECT_HANDLE_INFORMATION HandleInfo;<br>
> USEROBJECTFLAGS ObjectFlags;<br>
> + PUNICODE_STRING pStrNameU = NULL;<br>
> PVOID pvData = NULL;<br>
> SIZE_T nDataSize = 0;<br>
><br>
> @@ -820,13 +799,13 @@ NtUserGetObjectInformation(<br>
> _SEH2_END;<br>
><br>
> /* Try window station */<br>
> - TRACE("Trying to open window station %p\n", hObject);<br>
> + TRACE("Trying to open window station 0x%p\n", hObject);<br>
> Status = ObReferenceObjectByHandle(hObject,<br>
> 0,<br>
> ExWindowStationObjectType,<br>
> UserMode,<br>
> (PVOID*)&WinStaObject,<br>
> - NULL);<br>
> + &HandleInfo);<br>
><br>
> if (Status == STATUS_OBJECT_TYPE_MISMATCH)<br>
> {<br>
> @@ -852,23 +831,8 @@ NtUserGetObjectInformation(<br>
> {<br>
> case UOI_FLAGS:<br>
> {<br>
> - OBJECT_HANDLE_ATTRIBUTE_INFORMATION HandleInfo;<br>
> - ULONG BytesWritten;<br>
> -<br>
> ObjectFlags.fReserved = FALSE;<br>
> -<br>
> - /* Check whether this handle is inheritable */<br>
> - Status = ZwQueryObject(hObject,<br>
> - ObjectHandleFlagInformation,<br>
> - &HandleInfo,<br>
> - sizeof(OBJECT_HANDLE_ATTRIBUTE_INFORMATION),<br>
> - &BytesWritten);<br>
> - if (!NT_SUCCESS(Status))<br>
> - {<br>
> - ERR("ZwQueryObject failed, Status 0x%08lx\n", Status);<br>
> - break;<br>
> - }<br>
> - ObjectFlags.fInherit = HandleInfo.Inherit;<br>
> + ObjectFlags.fInherit = !!(HandleInfo.HandleAttributes &
OBJ_INHERIT);<br>
><br>
> ObjectFlags.dwFlags = 0;<br>
> if (WinStaObject != NULL)<br>
> @@ -893,11 +857,24 @@ NtUserGetObjectInformation(<br>
><br>
> case UOI_NAME:<br>
> {<br>
> - // FIXME: Use either ObQueryNameString() or read directly that name inside the
Object section!<br>
> if (WinStaObject != NULL)<br>
> {<br>
> - pvData = WinStaObject->Name.Buffer;<br>
> - nDataSize = WinStaObject->Name.Length + sizeof(WCHAR);<br>
> + ObjectHeader = OBJECT_TO_OBJECT_HEADER(WinStaObject);<br>
> + NameInfo = OBJECT_HEADER_TO_NAME_INFO(ObjectHeader);<br>
> +<br>
> + if (NameInfo && (NameInfo->Name.Length >
0))<br>
> + {<br>
> + /* Named window station */<br>
> + pStrNameU = &NameInfo->Name;<br>
> + nDataSize = pStrNameU->Length + sizeof(UNICODE_NULL);<br>
> + }<br>
> + else<br>
> + {<br>
> + /* Unnamed window station (should never happen!) */<br>
> + ASSERT(FALSE);<br>
> + pStrNameU = NULL;<br>
> + nDataSize = sizeof(UNICODE_NULL);<br>
> + }<br>
> Status = STATUS_SUCCESS;<br>
> }<br>
> else if (DesktopObject != NULL)<br>
> @@ -917,14 +894,16 @@ NtUserGetObjectInformation(<br>
> {<br>
> if (WinStaObject != NULL)<br>
> {<br>
> - pvData = L"WindowStation";<br>
> - nDataSize = sizeof(L"WindowStation");<br>
> + ObjectHeader = OBJECT_TO_OBJECT_HEADER(WinStaObject);<br>
> + pStrNameU = &ObjectHeader->Type->Name;<br>
> + nDataSize = pStrNameU->Length + sizeof(UNICODE_NULL);<br>
> Status = STATUS_SUCCESS;<br>
> }<br>
> else if (DesktopObject != NULL)<br>
> {<br>
> - pvData = L"Desktop";<br>
> - nDataSize = sizeof(L"Desktop");<br>
> + ObjectHeader = OBJECT_TO_OBJECT_HEADER(DesktopObject);<br>
> + pStrNameU = &ObjectHeader->Type->Name;<br>
> + nDataSize = pStrNameU->Length + sizeof(UNICODE_NULL);<br>
> Status = STATUS_SUCCESS;<br>
> }<br>
> else<br>
> @@ -954,10 +933,25 @@ Exit:<br>
> *nLengthNeeded = nDataSize;<br>
><br>
> /* Try to copy data to caller */<br>
> - if (Status == STATUS_SUCCESS)<br>
> + if (Status == STATUS_SUCCESS && (nDataSize >
0))<br>
> {<br>
> TRACE("Trying to copy data to caller (len = %lu, len needed = %lu)\n",
nLength, nDataSize);<br>
> - RtlCopyMemory(pvInformation, pvData, nDataSize);<br>
> + if (pvData)<br>
> + {<br>
> + /* Copy the data */<br>
> + RtlCopyMemory(pvInformation, pvData, nDataSize);<br>
> + }<br>
> + else if (pStrNameU)<br>
> + {<br>
> + /* Copy and NULL-terminate the string */<br>
> + RtlCopyMemory(pvInformation, pStrNameU->Buffer,
pStrNameU->Length);<br>
> + ((PWCHAR)pvInformation)[pStrNameU->Length / sizeof(WCHAR)] =
UNICODE_NULL;<br>
> + }<br>
> + else<br>
> + {<br>
> + /* Zero the memory */<br>
> + RtlZeroMemory(pvInformation, nDataSize);<br>
> + }<br>
> }<br>
> }<br>
> _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)<br>
> @@ -1026,8 +1020,6 @@ NtUserSetObjectInformation(<br>
> }<br>
><br>
><br>
> -<br>
> -<br>
> HWINSTA FASTCALL<br>
> UserGetProcessWindowStation(VOID)<br>
> {<br>
> diff --git a/win32ss/user/ntuser/winsta.h
b/win32ss/user/ntuser/winsta.h<br>
> index 085f3bcb26..19b1479ec0 100644<br>
> --- a/win32ss/user/ntuser/winsta.h<br>
> +++ b/win32ss/user/ntuser/winsta.h<br>
> @@ -15,7 +15,6 @@ typedef struct _WINSTATION_OBJECT<br>
> {<br>
> DWORD dwSessionId;<br>
><br>
> - UNICODE_STRING Name;<br>
> LIST_ENTRY DesktopListHead;<br>
> PRTL_ATOM_TABLE AtomTable;<br>
> HANDLE ShellWindow;<br>
><br>
><br>
</div>
<div class="gl_quoted"> </div>
</div>
2166
days inactive
2166
days old
0 comments
1 participants
participants (1)
-
hermes.belusca＠sfr.fr