Ros-diffs May 2022

ros-diffs@reactos.org

31 participants
165 discussions

[reactos] 01/01: [EXPLORER] Implement CDesktopThread::Destroy (#4371)

by Thamatip Chitpong

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=1aa95f389772901970369… commit 1aa95f389772901970369afa3abfb207df6f57f6 Author: Thamatip Chitpong <tangaming123456(a)outlook.com> AuthorDate: Tue May 24 18:58:24 2022 +0700 Commit: GitHub <noreply(a)github.com> CommitDate: Tue May 24 14:58:24 2022 +0300 [EXPLORER] Implement CDesktopThread::Destroy (#4371) Now Explorer exits properly without using ExitProcess hack. Signed-off-by: Thamatip Chitpong <tangaming123456(a)outlook.com> Reviewed-by: Mark Jansen <mark.jansen(a)reactos.org> Reviewed-by: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> Reviewed-by: Stanislav Motylkov <x86corez(a)gmail.com> --- base/shell/explorer/desktop.cpp | 186 +++++++++++++++++++++++++++------------- 1 file changed, 126 insertions(+), 60 deletions(-) diff --git a/base/shell/explorer/desktop.cpp b/base/shell/explorer/desktop.cpp index 94a30ea4cd7..764c9b0f975 100644 --- a/base/shell/explorer/desktop.cpp +++ b/base/shell/explorer/desktop.cpp @@ -22,97 +22,163 @@ class CDesktopThread { - HANDLE m_hEvent; +private: CComPtr<ITrayWindow> m_Tray; + HANDLE m_hInitEvent; + HANDLE m_hThread; - DWORD DesktopThreadProc() - { - CComPtr<IShellDesktopTray> pSdt; - HANDLE hDesktop; - HRESULT hRet; + DWORD DesktopThreadProc(); + static DWORD WINAPI s_DesktopThreadProc(LPVOID lpParameter); - OleInitialize(NULL); +public: + CDesktopThread(); + ~CDesktopThread(); - hRet = m_Tray->QueryInterface(IID_PPV_ARG(IShellDesktopTray, &pSdt)); - if (!SUCCEEDED(hRet)) - return 1; + HRESULT Initialize(ITrayWindow* pTray); + void Destroy(); +}; - hDesktop = _SHCreateDesktop(pSdt); - if (hDesktop == NULL) - return 1; +/*******************************************************************/ - if (!SetEvent(m_hEvent)) - { - /* Failed to notify that we initialized successfully, kill ourselves - to make the main thread wake up! */ - return 1; - } +CDesktopThread::CDesktopThread(): + m_Tray(NULL), + m_hInitEvent(NULL), + m_hThread(NULL) +{ +} - _SHDesktopMessageLoop(hDesktop); +CDesktopThread::~CDesktopThread() +{ + Destroy(); +} - /* FIXME: Properly rundown the main thread! */ - ExitProcess(0); +HRESULT CDesktopThread::Initialize(ITrayWindow* pTray) +{ + HANDLE Handles[2]; - return 0; + if (!pTray || m_Tray) + { + return E_FAIL; } - static DWORD CALLBACK s_DesktopThreadProc(IN OUT LPVOID lpParameter) + m_hInitEvent = CreateEventW(NULL, FALSE, FALSE, NULL); + if (!m_hInitEvent) { - return reinterpret_cast<CDesktopThread*>(lpParameter)->DesktopThreadProc(); + return E_FAIL; } -public: - CDesktopThread() : - m_hEvent(NULL), - m_Tray(NULL) + m_Tray = pTray; + m_hThread = CreateThread(NULL, 0, s_DesktopThreadProc, (LPVOID)this, 0, NULL); + + if (!m_hThread) { + CloseHandle(m_hInitEvent); + m_hInitEvent = NULL; + + m_Tray = NULL; + + return E_FAIL; } - HRESULT Initialize(IN OUT ITrayWindow *pTray) + Handles[0] = m_hThread; + Handles[1] = m_hInitEvent; + + for (;;) { - HANDLE hThread; - HANDLE Handles[2]; + DWORD WaitResult = MsgWaitForMultipleObjects(_countof(Handles), Handles, FALSE, INFINITE, QS_ALLEVENTS); + + if (WaitResult == WAIT_OBJECT_0 + _countof(Handles)) + { + TrayProcessMessages(m_Tray); + } + else if (WaitResult != WAIT_FAILED && WaitResult != WAIT_OBJECT_0) + { + break; + } + else + { + CloseHandle(m_hThread); + m_hThread = NULL; - m_Tray = pTray; + CloseHandle(m_hInitEvent); + m_hInitEvent = NULL; - m_hEvent = CreateEventW(NULL, FALSE, FALSE, NULL); - if (!m_hEvent) - return E_FAIL; + m_Tray = NULL; - hThread = CreateThread(NULL, 0, s_DesktopThreadProc, (PVOID)this, 0, NULL); - if (!hThread) - { - CloseHandle(m_hEvent); return E_FAIL; } + } + return S_OK; +} - Handles[0] = hThread; - Handles[1] = m_hEvent; - - for (;;) +void CDesktopThread::Destroy() +{ + if (m_hThread) + { + DWORD WaitResult = WaitForSingleObject(m_hThread, 0); + if (WaitResult == WAIT_TIMEOUT) { - DWORD WaitResult = MsgWaitForMultipleObjects(_countof(Handles), Handles, FALSE, INFINITE, QS_ALLEVENTS); - if (WaitResult == WAIT_OBJECT_0 + _countof(Handles)) - { - TrayProcessMessages(m_Tray); - } - else if (WaitResult != WAIT_FAILED && WaitResult != WAIT_OBJECT_0) - { - break; - } + /* Send WM_QUIT message to the thread and wait for it to terminate */ + PostThreadMessageW(GetThreadId(m_hThread), WM_QUIT, 0, 0); + WaitForSingleObject(m_hThread, INFINITE); } - CloseHandle(hThread); - CloseHandle(m_hEvent); + CloseHandle(m_hThread); + m_hThread = NULL; + } - return S_OK; + if (m_hInitEvent) + { + CloseHandle(m_hInitEvent); + m_hInitEvent = NULL; } - void Destroy() + m_Tray = NULL; +} + +DWORD CDesktopThread::DesktopThreadProc() +{ + CComPtr<IShellDesktopTray> pSdt; + HANDLE hDesktop; + HRESULT hRet; + DWORD dwResult = 1; + + OleInitialize(NULL); + + hRet = m_Tray->QueryInterface(IID_PPV_ARG(IShellDesktopTray, &pSdt)); + if (!SUCCEEDED(hRet)) { - return; + goto Cleanup; } -}; + + hDesktop = _SHCreateDesktop(pSdt); + if (!hDesktop) + { + goto Cleanup; + } + + if (!SetEvent(m_hInitEvent)) + { + /* Failed to notify that we initialized successfully, kill ourselves + * to make the main thread wake up! */ + goto Cleanup; + } + + _SHDesktopMessageLoop(hDesktop); + dwResult = 0; + +Cleanup: + OleUninitialize(); + return dwResult; +} + +DWORD WINAPI CDesktopThread::s_DesktopThreadProc(LPVOID lpParameter) +{ + CDesktopThread* pDesktopThread = static_cast<CDesktopThread*>(lpParameter); + return pDesktopThread->DesktopThreadProc(); +} + +/*******************************************************************/ HANDLE DesktopCreateWindow(IN OUT ITrayWindow *Tray) @@ -133,5 +199,5 @@ VOID DesktopDestroyShellWindow(IN HANDLE hDesktop) { CDesktopThread* pDesktopThread = reinterpret_cast<CDesktopThread*>(hDesktop); - pDesktopThread->Destroy(); + delete pDesktopThread; }

2 years, 7 months

[reactos] 01/01: [RAPPS] Improve localized display of installation date (#4498)

by Marcus Boillat

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=4900dd3d87531c6ae411d… commit 4900dd3d87531c6ae411dad376188cb9e1b229de Author: Marcus Boillat <marcus.boillat(a)outlook.com> AuthorDate: Tue May 24 11:20:52 2022 +0200 Commit: GitHub <noreply(a)github.com> CommitDate: Tue May 24 12:20:52 2022 +0300 [RAPPS] Improve localized display of installation date (#4498) Parse InstallDate registry key in "YYYYMMDD" format. CORE-17422 Signed-off-by: Marcus Boillat <marcus.boillat(a)gmail.com> Reviewed-by: Mark Jansen <mark.jansen(a)reactos.org> Reviewed-by: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> Reviewed-by: Stanislav Motylkov <x86corez(a)gmail.com> --- base/applications/rapps/installed.cpp | 49 ++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/base/applications/rapps/installed.cpp b/base/applications/rapps/installed.cpp index 3528c2c3aaa..7162f471b50 100644 --- a/base/applications/rapps/installed.cpp +++ b/base/applications/rapps/installed.cpp @@ -45,30 +45,43 @@ void CInstalledApplicationInfo::EnsureDetailsLoaded() GetApplicationRegString(L"Contact", szContact); GetApplicationRegString(L"URLUpdateInfo", szURLUpdateInfo); GetApplicationRegString(L"URLInfoAbout", szURLInfoAbout); - if (GetApplicationRegString(L"InstallDate", szInstallDate) == FALSE) + + DWORD dwInstallTimeStamp; + SYSTEMTIME InstallLocalTime; + if (GetApplicationRegString(L"InstallDate", szInstallDate)) { - // It might be a DWORD (Unix timestamp). try again. - DWORD dwInstallTimeStamp; - if (GetApplicationRegDword(L"InstallDate", &dwInstallTimeStamp)) + ZeroMemory(&InstallLocalTime, sizeof(InstallLocalTime)); + // Check if we have 8 characters to parse the datetime. + // Maybe other formats exist as well? + szInstallDate = szInstallDate.Trim(); + if (szInstallDate.GetLength() == 8) { - FILETIME InstallFileTime; - SYSTEMTIME InstallSystemTime, InstallLocalTime; + InstallLocalTime.wYear = wcstol(szInstallDate.Left(4).GetString(), NULL, 10); + InstallLocalTime.wMonth = wcstol(szInstallDate.Mid(4, 2).GetString(), NULL, 10); + InstallLocalTime.wDay = wcstol(szInstallDate.Mid(6, 2).GetString(), NULL, 10); + } + } + // It might be a DWORD (Unix timestamp). try again. + else if (GetApplicationRegDword(L"InstallDate", &dwInstallTimeStamp)) + { + FILETIME InstallFileTime; + SYSTEMTIME InstallSystemTime; - UnixTimeToFileTime(dwInstallTimeStamp, &InstallFileTime); - FileTimeToSystemTime(&InstallFileTime, &InstallSystemTime); + UnixTimeToFileTime(dwInstallTimeStamp, &InstallFileTime); + FileTimeToSystemTime(&InstallFileTime, &InstallSystemTime); - // convert to localtime - SystemTimeToTzSpecificLocalTime(NULL, &InstallSystemTime, &InstallLocalTime); + // convert to localtime + SystemTimeToTzSpecificLocalTime(NULL, &InstallSystemTime, &InstallLocalTime); + } - // convert to readable date string - int cchTimeStrLen = GetDateFormatW(LOCALE_USER_DEFAULT, 0, &InstallLocalTime, NULL, 0, 0); + // convert to readable date string + int cchTimeStrLen = GetDateFormatW(LOCALE_USER_DEFAULT, 0, &InstallLocalTime, NULL, 0, 0); + + GetDateFormatW( + LOCALE_USER_DEFAULT, // use default locale for current user + 0, &InstallLocalTime, NULL, szInstallDate.GetBuffer(cchTimeStrLen), cchTimeStrLen); + szInstallDate.ReleaseBuffer(); - GetDateFormatW( - LOCALE_USER_DEFAULT, // use default locale for current user - 0, &InstallLocalTime, NULL, szInstallDate.GetBuffer(cchTimeStrLen), cchTimeStrLen); - szInstallDate.ReleaseBuffer(); - } - } GetApplicationRegString(L"InstallLocation", szInstallLocation); GetApplicationRegString(L"InstallSource", szInstallSource); DWORD dwWindowsInstaller = 0;

2 years, 7 months

[reactos] 01/01: [NTOS:PNP] Misc IoInvalidateDeviceState fixes

by Victor Perevertkin

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=505ac6565afcdc150a915… commit 505ac6565afcdc150a915937b6217c3b6f892e28 Author: Victor Perevertkin <victor.perevertkin(a)reactos.org> AuthorDate: Tue May 24 04:57:42 2022 +0300 Commit: Victor Perevertkin <victor.perevertkin(a)reactos.org> CommitDate: Tue May 24 05:04:11 2022 +0300 [NTOS:PNP] Misc IoInvalidateDeviceState fixes - Add a check for correct PDO before doing anything - Process the request only for started devices - Send the request synchronously during the start sequence This makes Windows' i8042prt.sys work on ReactOS. Addendum to cf0bc1c1321047b879edcd19bfbf08138ba9c988 --- ntoskrnl/io/pnpmgr/devaction.c | 152 ++++++++++++++++++++++------------------- ntoskrnl/io/pnpmgr/pnpirp.c | 3 +- ntoskrnl/io/pnpmgr/pnpmgr.c | 7 +- 3 files changed, 87 insertions(+), 75 deletions(-) diff --git a/ntoskrnl/io/pnpmgr/devaction.c b/ntoskrnl/io/pnpmgr/devaction.c index a3b03d5bd2c..8b133ef80f4 100644 --- a/ntoskrnl/io/pnpmgr/devaction.c +++ b/ntoskrnl/io/pnpmgr/devaction.c @@ -1502,6 +1502,72 @@ done: return Status; } +/** + * @brief Processes the IoInvalidateDeviceState request + * + * Sends IRP_MN_QUERY_PNP_DEVICE_STATE request and sets device node's flags + * according to the result. + * Tree reenumeration should be started upon a successful return of the function. + * + * @todo Do not return STATUS_SUCCESS if nothing is changed. + */ +static +NTSTATUS +PiUpdateDeviceState( + _In_ PDEVICE_NODE DeviceNode) +{ + PNP_DEVICE_STATE PnPFlags; + NTSTATUS Status; + + Status = PiIrpQueryPnPDeviceState(DeviceNode, &PnPFlags); + if (!NT_SUCCESS(Status)) + { + return Status; + } + + if (PnPFlags & PNP_DEVICE_NOT_DISABLEABLE) + DeviceNode->UserFlags |= DNUF_NOT_DISABLEABLE; + else + DeviceNode->UserFlags &= ~DNUF_NOT_DISABLEABLE; + + if (PnPFlags & PNP_DEVICE_DONT_DISPLAY_IN_UI) + DeviceNode->UserFlags |= DNUF_DONT_SHOW_IN_UI; + else + DeviceNode->UserFlags &= ~DNUF_DONT_SHOW_IN_UI; + + if (PnPFlags & PNP_DEVICE_REMOVED || PnPFlags & PNP_DEVICE_DISABLED) + { + PiSetDevNodeProblem(DeviceNode, + PnPFlags & PNP_DEVICE_DISABLED + ? CM_PROB_HARDWARE_DISABLED + : CM_PROB_DEVICE_NOT_THERE); + + PiSetDevNodeState(DeviceNode, DeviceNodeAwaitingQueuedRemoval); + } + else if (PnPFlags & PNP_DEVICE_RESOURCE_REQUIREMENTS_CHANGED) + { + // Query resource rebalance + + if (PnPFlags & PNP_DEVICE_FAILED) + DeviceNode->Flags &= DNF_NON_STOPPED_REBALANCE; + else + DeviceNode->Flags |= DNF_NON_STOPPED_REBALANCE; + + // Clear DNF_NO_RESOURCE_REQUIRED just in case (will be set back if needed) + DeviceNode->Flags &= ~DNF_NO_RESOURCE_REQUIRED; + + // This will be caught up later by enumeration + DeviceNode->Flags |= DNF_RESOURCE_REQUIREMENTS_CHANGED; + } + else if (PnPFlags & PNP_DEVICE_FAILED) + { + PiSetDevNodeProblem(DeviceNode, CM_PROB_FAILED_POST_START); + PiSetDevNodeState(DeviceNode, DeviceNodeAwaitingQueuedRemoval); + } + + return STATUS_SUCCESS; +} + static NTSTATUS PiStartDeviceFinal( @@ -1553,7 +1619,7 @@ PiStartDeviceFinal( } // Query the device state (IRP_MN_QUERY_PNP_DEVICE_STATE) - PiQueueDeviceAction(DeviceNode->PhysicalDeviceObject, PiActionQueryState, NULL, NULL); + PiUpdateDeviceState(DeviceNode); DPRINT("Sending GUID_DEVICE_ARRIVAL %wZ\n", &DeviceNode->InstancePath); IopQueueTargetDeviceEvent(&GUID_DEVICE_ARRIVAL, &DeviceNode->InstancePath); @@ -2025,72 +2091,6 @@ IopRemoveDevice(PDEVICE_NODE DeviceNode) return Status; } -/** - * @brief Processes the IoInvalidateDeviceState request - * - * Sends IRP_MN_QUERY_PNP_DEVICE_STATE request and sets device node's flags - * according to the result. - * Tree reenumeration should be started upon a successful return of the function. - * - * @todo Do not return STATUS_SUCCESS if nothing is changed. - */ -static -NTSTATUS -PiUpdateDeviceState( - _In_ PDEVICE_NODE DeviceNode) -{ - PNP_DEVICE_STATE PnPFlags; - NTSTATUS Status; - - Status = PiIrpQueryPnPDeviceState(DeviceNode, &PnPFlags); - if (!NT_SUCCESS(Status)) - { - return Status; - } - - if (PnPFlags & PNP_DEVICE_NOT_DISABLEABLE) - DeviceNode->UserFlags |= DNUF_NOT_DISABLEABLE; - else - DeviceNode->UserFlags &= ~DNUF_NOT_DISABLEABLE; - - if (PnPFlags & PNP_DEVICE_DONT_DISPLAY_IN_UI) - DeviceNode->UserFlags |= DNUF_DONT_SHOW_IN_UI; - else - DeviceNode->UserFlags &= ~DNUF_DONT_SHOW_IN_UI; - - if (PnPFlags & PNP_DEVICE_REMOVED || PnPFlags & PNP_DEVICE_DISABLED) - { - PiSetDevNodeProblem(DeviceNode, - PnPFlags & PNP_DEVICE_DISABLED - ? CM_PROB_HARDWARE_DISABLED - : CM_PROB_DEVICE_NOT_THERE); - - PiSetDevNodeState(DeviceNode, DeviceNodeAwaitingQueuedRemoval); - } - else if (PnPFlags & PNP_DEVICE_RESOURCE_REQUIREMENTS_CHANGED) - { - // Query resource rebalance - - if (PnPFlags & PNP_DEVICE_FAILED) - DeviceNode->Flags &= DNF_NON_STOPPED_REBALANCE; - else - DeviceNode->Flags |= DNF_NON_STOPPED_REBALANCE; - - // Clear DNF_NO_RESOURCE_REQUIRED just in case (will be set back if needed) - DeviceNode->Flags &= ~DNF_NO_RESOURCE_REQUIRED; - - // This will be caught up later by enumeration - DeviceNode->Flags |= DNF_RESOURCE_REQUIREMENTS_CHANGED; - } - else if (PnPFlags & PNP_DEVICE_FAILED) - { - PiSetDevNodeProblem(DeviceNode, CM_PROB_FAILED_POST_START); - PiSetDevNodeState(DeviceNode, DeviceNodeAwaitingQueuedRemoval); - } - - return STATUS_SUCCESS; -} - static NTSTATUS PiEnumerateDevice( @@ -2386,6 +2386,7 @@ PiDevNodeStateMachine( break; case DeviceNodeStartPostWork: DPRINT("DeviceNodeStartPostWork %wZ\n", &currentNode->InstancePath); + // TODO: inspect the status status = PiStartDeviceFinal(currentNode); doProcessAgain = TRUE; break; @@ -2599,13 +2600,20 @@ PipDeviceActionWorker( break; case PiActionQueryState: - // First, do a IRP_MN_QUERY_PNP_DEVICE_STATE request, - // it will update node's flags and then do enumeration if something changed - status = PiUpdateDeviceState(deviceNode); - if (NT_SUCCESS(status)) + // This action is only valid for started devices. If the device is not yet + // started, the PnP manager issues IRP_MN_QUERY_PNP_DEVICE_STATE by itself. + if (deviceNode->State == DeviceNodeStarted) { - PiDevNodeStateMachine(deviceNode); + // Issue a IRP_MN_QUERY_PNP_DEVICE_STATE request: it will update node's flags + // and then do enumeration if something has changed + status = PiUpdateDeviceState(deviceNode); + if (NT_SUCCESS(status)) + { + PiDevNodeStateMachine(deviceNode); + } } + // TODO: Windows may return STATUS_DELETE_PENDING here + status = STATUS_SUCCESS; break; default: diff --git a/ntoskrnl/io/pnpmgr/pnpirp.c b/ntoskrnl/io/pnpmgr/pnpirp.c index 952150b2731..4a6a50c92ef 100644 --- a/ntoskrnl/io/pnpmgr/pnpirp.c +++ b/ntoskrnl/io/pnpmgr/pnpirp.c @@ -288,8 +288,7 @@ PiIrpQueryPnPDeviceState( PAGED_CODE(); ASSERT(DeviceNode); - ASSERT(DeviceNode->State == DeviceNodeResourcesAssigned || - DeviceNode->State == DeviceNodeStartPostWork || + ASSERT(DeviceNode->State == DeviceNodeStartPostWork || DeviceNode->State == DeviceNodeStarted); ULONG_PTR longState; diff --git a/ntoskrnl/io/pnpmgr/pnpmgr.c b/ntoskrnl/io/pnpmgr/pnpmgr.c index e2f7ba98da5..87bf857e9fc 100644 --- a/ntoskrnl/io/pnpmgr/pnpmgr.c +++ b/ntoskrnl/io/pnpmgr/pnpmgr.c @@ -4,7 +4,7 @@ * FILE: ntoskrnl/io/pnpmgr/pnpmgr.c * PURPOSE: Initializes the PnP manager * PROGRAMMERS: Casper S. Hornstrup (chorns(a)users.sourceforge.net) - * Copyright 2007 Herv� Poussineau (hpoussin(a)reactos.org) + * Copyright 2007 Hervé Poussineau (hpoussin(a)reactos.org) */ /* INCLUDES ******************************************************************/ @@ -2418,5 +2418,10 @@ NTAPI IoInvalidateDeviceState( IN PDEVICE_OBJECT DeviceObject) { + if (!IopIsValidPhysicalDeviceObject(DeviceObject)) + { + KeBugCheckEx(PNP_DETECTED_FATAL_ERROR, 0x2, (ULONG_PTR)DeviceObject, 0, 0); + } + PiQueueDeviceAction(DeviceObject, PiActionQueryState, NULL, NULL); }

2 years, 7 months

[reactos] 02/02: [DESKMON] Implement GetMonitorDevInstID function

by Stanislav Motylkov

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=17154c94c1acfedef9ad8… commit 17154c94c1acfedef9ad801e48ad775c92966e51 Author: Stanislav Motylkov <x86corez(a)gmail.com> AuthorDate: Mon May 23 23:54:41 2022 +0300 Commit: Stanislav Motylkov <x86corez(a)gmail.com> CommitDate: Mon May 23 23:54:41 2022 +0300 [DESKMON] Implement GetMonitorDevInstID function - Now our deskmon.cpl extension works on Windows XP and 2003 and does open the monitor device properties correctly. - Also "Properties" button should be disabled when no Device Instance ID found for the monitor. --- dll/shellext/deskmon/CMakeLists.txt | 2 +- dll/shellext/deskmon/deskmon.c | 144 ++++++++++++++++++++++++++++++++++-- 2 files changed, 139 insertions(+), 7 deletions(-) diff --git a/dll/shellext/deskmon/CMakeLists.txt b/dll/shellext/deskmon/CMakeLists.txt index d01e2e09870..2f85372577b 100644 --- a/dll/shellext/deskmon/CMakeLists.txt +++ b/dll/shellext/deskmon/CMakeLists.txt @@ -15,6 +15,6 @@ add_library(deskmon MODULE set_module_type(deskmon win32dll UNICODE) target_link_libraries(deskmon uuid) -add_importlibs(deskmon user32 gdi32 comctl32 ole32 msvcrt kernel32 ntdll) +add_importlibs(deskmon user32 gdi32 comctl32 ole32 setupapi msvcrt kernel32 ntdll) add_pch(deskmon precomp.h SOURCE) add_cd_file(TARGET deskmon DESTINATION reactos/system32 FOR all) diff --git a/dll/shellext/deskmon/deskmon.c b/dll/shellext/deskmon/deskmon.c index 16bdd2372e4..21551a6ceb5 100644 --- a/dll/shellext/deskmon/deskmon.c +++ b/dll/shellext/deskmon/deskmon.c @@ -1,6 +1,8 @@ #include "precomp.h" #include <tchar.h> +#include <winreg.h> +#include <cfgmgr32.h> #define NDEBUG #include <debug.h> @@ -8,6 +10,8 @@ #include "resource.h" static HINSTANCE hInstance; +static BOOL bFoundAdapter; +static DEVINST diAdapter; #ifdef UNICODE typedef INT_PTR (WINAPI *PDEVICEPROPERTIES)(HWND,LPCWSTR,LPCWSTR,BOOL); @@ -17,11 +21,115 @@ typedef INT_PTR (WINAPI *PDEVICEPROPERTIES)(HWND,LPCSTR,LPCSTR,BOOL); #define FUNC_DEVICEPROPERTIES "DevicePropertiesA" #endif +/** + * @brief + * Converts a Hardware ID (DeviceID from EnumDisplayDevices) + * to an unique Device Instance ID. + * + * @param[in] lpDeviceID + * A pointer to a null-terminated string + * containing a Hardware ID concatenated with driver key. + * e.g. "Monitor\Default_Monitor\{4D36E96E-E325-11CE-BFC1-08002BE10318}\0000" + * + * @return + * A pointer to a null-terminated string + * containing an unique Device Instance ID + * or NULL in case of error. + * e.g. "DISPLAY\DEFAULT_MONITOR\4&2ABFAA30&0&00000001&00&02" + * + * @remarks + * The caller must free the returned string with LocalFree. + */ static LPTSTR GetMonitorDevInstID(LPCTSTR lpDeviceID) { - /* FIXME: Implement, allocate returned string with LocalAlloc! */ - return NULL; + CONFIGRET cr; + DEVINST diChild; + TCHAR szProperty[256]; + DWORD dwSize; + LPTSTR lpDevInstId = NULL; + + if (!bFoundAdapter) + return NULL; + + /* Look for child monitor devices of selected video adapter */ + cr = CM_Get_Child(&diChild, diAdapter, 0); + if (cr != CR_SUCCESS) + { + DPRINT1("CM_Get_Child failed: %d\n", cr); + return NULL; + } + + do + { + /* Get Hardware ID for each of them */ + dwSize = sizeof(szProperty) - sizeof(TCHAR); + + cr = CM_Get_DevNode_Registry_Property(diChild, + CM_DRP_HARDWAREID, + NULL, + szProperty, + &dwSize, + 0); + if (cr != CR_SUCCESS) + { + DPRINT1("CM_Get_DevNode_Registry_Property failed: %d\n", cr); + continue; + } + + /* Concatenate with driver key */ + _tcscat(szProperty, TEXT("\\")); + dwSize = sizeof(szProperty) - sizeof(TCHAR); + dwSize -= _tcslen(szProperty) * sizeof(TCHAR); + + cr = CM_Get_DevNode_Registry_Property(diChild, + CM_DRP_DRIVER, + NULL, + szProperty + _tcslen(szProperty), + &dwSize, + 0); + if (cr != CR_SUCCESS) + { + DPRINT1("CM_Get_DevNode_Registry_Property failed: %d\n", cr); + continue; + } + + /* If the strings match, this is our monitor device node */ + if (_tcscmp(szProperty, lpDeviceID) == 0) + { + cr = CM_Get_Device_ID_Size(&dwSize, + diChild, + 0); + if (cr != CR_SUCCESS) + { + DPRINT1("CM_Get_Device_ID_Size failed: %d\n", cr); + break; + } + + lpDevInstId = LocalAlloc(LMEM_FIXED, + (dwSize + 1) * sizeof(TCHAR)); + if (lpDevInstId == NULL) + { + DPRINT1("LocalAlloc failed\n"); + break; + } + + cr = CM_Get_Device_ID(diChild, + lpDevInstId, + dwSize + 1, + 0); + if (cr != CR_SUCCESS) + { + DPRINT1("CM_Get_Device_ID failed: %d\n", cr); + LocalFree((HLOCAL)lpDevInstId); + lpDevInstId = NULL; + } + + break; + } + } while (CM_Get_Sibling(&diChild, diChild, 0) == CR_SUCCESS); + + return lpDevInstId; } static VOID @@ -44,9 +152,9 @@ ShowMonitorProperties(PDESKMONITOR This) if (pDeviceProperties != NULL) { pDeviceProperties(This->hwndDlg, - NULL, - This->SelMonitor->dd.DeviceID, - FALSE); + NULL, + lpDevInstID, + FALSE); } FreeLibrary(hDevMgr); @@ -61,6 +169,7 @@ static VOID UpdateMonitorSelection(PDESKMONITOR This) { INT i; + LPTSTR lpDevInstID = NULL; if (This->dwMonitorCount > 1) { @@ -83,9 +192,15 @@ UpdateMonitorSelection(PDESKMONITOR This) else This->SelMonitor = This->Monitors; + if (This->SelMonitor != NULL) + lpDevInstID = GetMonitorDevInstID(This->SelMonitor->dd.DeviceID); + EnableWindow(GetDlgItem(This->hwndDlg, IDC_MONITORPROPERTIES), - This->SelMonitor != NULL); + lpDevInstID != NULL && lpDevInstID[0] != TEXT('\0')); + + if (lpDevInstID != NULL) + LocalFree((HLOCAL)lpDevInstID); } static VOID @@ -230,6 +345,8 @@ static VOID InitMonitorDialog(PDESKMONITOR This) { PDESKMONINFO pmi, pminext, *pmilink; + LPTSTR lpDeviceId; + CONFIGRET cr; DISPLAY_DEVICE dd; BOOL bRet; INT i; @@ -248,6 +365,21 @@ InitMonitorDialog(PDESKMONITOR This) This->SelMonitor = NULL; This->dwMonitorCount = 0; + bFoundAdapter = FALSE; + lpDeviceId = QueryDeskCplString(This->pdtobj, + RegisterClipboardFormat(DESK_EXT_DISPLAYID)); + + if (lpDeviceId != NULL && lpDeviceId[0] != TEXT('\0')) + { + cr = CM_Locate_DevNode(&diAdapter, + lpDeviceId, + CM_LOCATE_DEVNODE_NORMAL); + bFoundAdapter = (cr == CR_SUCCESS); + + if (!bFoundAdapter) + DPRINT1("CM_Locate_DevNode failed: %d\n", cr); + } + if (This->lpDisplayDevice != NULL) LocalFree((HLOCAL)This->lpDisplayDevice);

2 years, 7 months

[reactos] 01/02: [DESK] Add debug prints to pCDevSettings_GetDeviceInstanceId

by Stanislav Motylkov

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=fcba0968bef3e3ec529be… commit fcba0968bef3e3ec529bebb5a97445a44c7068ba Author: Stanislav Motylkov <x86corez(a)gmail.com> AuthorDate: Mon May 23 23:50:47 2022 +0300 Commit: Stanislav Motylkov <x86corez(a)gmail.com> CommitDate: Mon May 23 23:50:47 2022 +0300 [DESK] Add debug prints to pCDevSettings_GetDeviceInstanceId Addendum to 3900cf88. --- dll/cpl/desk/devsett.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dll/cpl/desk/devsett.c b/dll/cpl/desk/devsett.c index 27e75d6d349..dc801f4c624 100644 --- a/dll/cpl/desk/devsett.c +++ b/dll/cpl/desk/devsett.c @@ -199,27 +199,40 @@ pCDevSettings_GetDeviceInstanceId(const WCHAR *pszDevice) DevInfo = SetupDiGetClassDevsW(NULL, pszDevice, NULL, DIGCF_ALLCLASSES | DIGCF_PRESENT); if (DevInfo == INVALID_HANDLE_VALUE) + { + DPRINT1("SetupDiGetClassDevsW(\"%ws\") failed: %d\n", pszDevice, GetLastError()); return NULL; + } ZeroMemory(&InfoData, sizeof(InfoData)); InfoData.cbSize = sizeof(InfoData); /* Try to enumerate the first matching device */ if (!SetupDiEnumDeviceInfo(DevInfo, 0, &InfoData)) + { + DPRINT1("SetupDiEnumDeviceInfo failed: %d\n", GetLastError()); return NULL; + } if (SetupDiGetDeviceInstanceId(DevInfo, &InfoData, NULL, 0, &BufLen) || GetLastError() != ERROR_INSUFFICIENT_BUFFER) + { + DPRINT1("SetupDiGetDeviceInstanceId failed: %d\n", GetLastError()); return NULL; + } lpDevInstId = LocalAlloc(LMEM_FIXED, (BufLen + 1) * sizeof(WCHAR)); if (lpDevInstId == NULL) + { + DPRINT1("LocalAlloc failed\n"); return NULL; + } if (!SetupDiGetDeviceInstanceId(DevInfo, &InfoData, lpDevInstId, BufLen, NULL)) { + DPRINT1("SetupDiGetDeviceInstanceId failed: %d\n", GetLastError()); LocalFree((HLOCAL)lpDevInstId); lpDevInstId = NULL; }

2 years, 7 months

[reactos] 07/07: [NTOS:SE] Replace a bunch of RtlCopyLuid() calls into direct assignations (#4523)

by Hermès Bélusca-Maïto

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=985468d08a82d9e1eac69… commit 985468d08a82d9e1eac6902656518829987667df Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Mon May 23 01:56:32 2022 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon May 23 19:30:37 2022 +0200 [NTOS:SE] Replace a bunch of RtlCopyLuid() calls into direct assignations (#4523) Nowadays' compilers support such direct structure assignations, and our existing codebase already uses that in other places. --- ntoskrnl/se/token.c | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c index 0c5296fe114..e773358b356 100644 --- a/ntoskrnl/se/token.c +++ b/ntoskrnl/se/token.c @@ -1047,8 +1047,7 @@ SepDuplicateToken( } /* Copy the immutable fields */ - RtlCopyLuid(&AccessToken->TokenSource.SourceIdentifier, - &Token->TokenSource.SourceIdentifier); + AccessToken->TokenSource.SourceIdentifier = Token->TokenSource.SourceIdentifier; RtlCopyMemory(AccessToken->TokenSource.SourceName, Token->TokenSource.SourceName, sizeof(Token->TokenSource.SourceName)); @@ -1062,7 +1061,7 @@ SepDuplicateToken( SepAcquireTokenLockShared(Token); AccessToken->SessionId = Token->SessionId; - RtlCopyLuid(&AccessToken->ModifiedId, &Token->ModifiedId); + AccessToken->ModifiedId = Token->ModifiedId; AccessToken->TokenFlags = Token->TokenFlags & ~TOKEN_SESSION_NOT_REFERENCED; @@ -1097,7 +1096,7 @@ SepDuplicateToken( AccessToken->CreateMethod = TOKEN_DUPLICATE_METHOD; #endif - /* Assign the data that reside in the TOKEN's variable information area */ + /* Assign the data that reside in the token's variable information area */ AccessToken->VariableLength = VariableLength; EndMem = (PVOID)&AccessToken->VariablePart; @@ -1264,7 +1263,7 @@ SepDuplicateToken( // other data, the code will require adaptations. // - /* Now allocate the TOKEN's dynamic information area and set the data */ + /* Now allocate the token's dynamic information area and set the data */ AccessToken->DynamicAvailable = 0; // Unused memory in the dynamic area. AccessToken->DynamicPart = NULL; if (Token->DynamicPart && Token->DefaultDacl) @@ -1810,8 +1809,7 @@ SepCreateToken( /* Zero out the buffer and initialize the token */ RtlZeroMemory(AccessToken, TotalSize); - RtlCopyLuid(&AccessToken->TokenId, &TokenId); - + AccessToken->TokenId = TokenId; AccessToken->TokenType = TokenType; AccessToken->ImpersonationLevel = ImpersonationLevel; @@ -1820,19 +1818,18 @@ SepCreateToken( if (!NT_SUCCESS(Status)) goto Quit; - RtlCopyLuid(&AccessToken->TokenSource.SourceIdentifier, - &TokenSource->SourceIdentifier); + AccessToken->TokenSource.SourceIdentifier = TokenSource->SourceIdentifier; RtlCopyMemory(AccessToken->TokenSource.SourceName, TokenSource->SourceName, sizeof(TokenSource->SourceName)); AccessToken->ExpirationTime = *ExpirationTime; - RtlCopyLuid(&AccessToken->ModifiedId, &ModifiedId); + AccessToken->ModifiedId = ModifiedId; AccessToken->TokenFlags = TokenFlags & ~TOKEN_SESSION_NOT_REFERENCED; /* Copy and reference the logon session */ - RtlCopyLuid(&AccessToken->AuthenticationId, AuthenticationId); + AccessToken->AuthenticationId = *AuthenticationId; Status = SepRmReferenceLogonSession(&AccessToken->AuthenticationId); if (!NT_SUCCESS(Status)) { @@ -1878,7 +1875,7 @@ SepCreateToken( AccessToken->CreateMethod = TOKEN_CREATE_METHOD; #endif - /* Assign the data that reside in the TOKEN's variable information area */ + /* Assign the data that reside in the token's variable information area */ AccessToken->VariableLength = VariableLength; EndMem = (PVOID)&AccessToken->VariablePart; @@ -1960,7 +1957,7 @@ SepCreateToken( AccessToken->PrimaryGroup = AccessToken->UserAndGroups[PrimaryGroupIndex].Sid; AccessToken->DefaultOwnerIndex = DefaultOwnerIndex; - /* Now allocate the TOKEN's dynamic information area and set the data */ + /* Now allocate the token's dynamic information area and set the data */ AccessToken->DynamicAvailable = 0; // Unused memory in the dynamic area. AccessToken->DynamicPart = NULL; if (DefaultDacl != NULL) @@ -2181,16 +2178,14 @@ SepPerformTokenFiltering( AccessToken->ImpersonationLevel = Token->ImpersonationLevel; /* Copy the immutable fields */ - RtlCopyLuid(&AccessToken->TokenSource.SourceIdentifier, - &Token->TokenSource.SourceIdentifier); + AccessToken->TokenSource.SourceIdentifier = Token->TokenSource.SourceIdentifier; RtlCopyMemory(AccessToken->TokenSource.SourceName, Token->TokenSource.SourceName, sizeof(Token->TokenSource.SourceName)); - RtlCopyLuid(&AccessToken->AuthenticationId, &Token->AuthenticationId); - RtlCopyLuid(&AccessToken->ParentTokenId, &Token->TokenId); - RtlCopyLuid(&AccessToken->OriginatingLogonSession, - &Token->OriginatingLogonSession); + AccessToken->AuthenticationId = Token->AuthenticationId; + AccessToken->ParentTokenId = Token->TokenId; + AccessToken->OriginatingLogonSession = Token->OriginatingLogonSession; AccessToken->ExpirationTime = Token->ExpirationTime; @@ -4100,8 +4095,7 @@ NtQueryInformationToken( { if (TokenInformationLength >= RequiredLength) { - RtlCopyLuid(&to->OriginatingLogonSession, - &Token->AuthenticationId); + to->OriginatingLogonSession = Token->AuthenticationId; } else { @@ -5436,9 +5430,7 @@ NtAdjustGroupsToken( Quit: /* Allocate a new ID for the token as we made changes */ if (ChangesMade) - { ExAllocateLocallyUniqueId(&Token->ModifiedId); - } /* Unlock and dereference the token */ SepReleaseTokenLock(Token);

2 years, 7 months

[reactos] 06/07: [NTOS:SE] SepPerformTokenFiltering(): Fix corruption of DynamicPart (#4523)

by Hermès Bélusca-Maïto

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=487d8601f9700a04f302a… commit 487d8601f9700a04f302a37c4fe399eab92da23e Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Fri May 20 02:33:00 2022 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon May 23 19:30:36 2022 +0200 [NTOS:SE] SepPerformTokenFiltering(): Fix corruption of DynamicPart (#4523) The problem is that EndMem is changed to point to the DynamicPart of the token, but the code after that expects it to still point into the VariablePart instead. Problem fixed by moving the insertion of RestrictedSids much sooner (where the original ones are also being copied). --- ntoskrnl/se/token.c | 122 ++++++++++++++++++++++++++-------------------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c index 65018f60f3f..0c5296fe114 100644 --- a/ntoskrnl/se/token.c +++ b/ntoskrnl/se/token.c @@ -1277,8 +1277,8 @@ SepDuplicateToken( Status = STATUS_INSUFFICIENT_RESOURCES; goto Quit; } - EndMem = (PVOID)AccessToken->DynamicPart; + EndMem = (PVOID)AccessToken->DynamicPart; AccessToken->DefaultDacl = EndMem; RtlCopyMemory(AccessToken->DefaultDacl, @@ -1973,8 +1973,8 @@ SepCreateToken( Status = STATUS_INSUFFICIENT_RESOURCES; goto Quit; } - EndMem = (PVOID)AccessToken->DynamicPart; + EndMem = (PVOID)AccessToken->DynamicPart; AccessToken->DefaultDacl = EndMem; RtlCopyMemory(AccessToken->DefaultDacl, @@ -2306,6 +2306,64 @@ SepPerformTokenFiltering( } } + /* + * Insert the restricted SIDs into the token on + * the request by the caller. + */ + if (RestrictedSidsIntoToken != NULL) + { + for (RestrictedSidsInList = 0; RestrictedSidsInList < RestrictedSidsCount; RestrictedSidsInList++) + { + /* Did the caller assign attributes to the restricted SIDs? */ + if (RestrictedSidsIntoToken[RestrictedSidsInList].Attributes != 0) + { + /* There mustn't be any attributes, bail out */ + DPRINT1("SepPerformTokenFiltering(): There mustn't be any attributes to restricted SIDs!\n"); + Status = STATUS_INVALID_PARAMETER; + goto Quit; + } + } + + /* + * Ensure that the token can hold the restricted SIDs + * (the variable length is calculated at the beginning + * of the routine call). + */ + ASSERT(VariableLength >= RestrictedSidsLength); + + /* + * Now let's begin inserting the restricted SIDs into the filtered + * access token from the list the caller gave us. + */ + AccessToken->RestrictedSidCount = RestrictedSidsCount; + AccessToken->RestrictedSids = EndMem; + EndMem = (PVOID)((ULONG_PTR)EndMem + RestrictedSidsLength); + VariableLength -= RestrictedSidsLength; + + RtlCopyMemory(AccessToken->RestrictedSids, + RestrictedSidsIntoToken, + AccessToken->RestrictedSidCount * sizeof(SID_AND_ATTRIBUTES)); + + /* + * As we've copied the restricted SIDs into + * the token, we must assign them the following + * combination of attributes SE_GROUP_ENABLED, + * SE_GROUP_ENABLED_BY_DEFAULT and SE_GROUP_MANDATORY. + * With such attributes we estabilish that restricting + * SIDs into the token are enabled for access checks. + */ + for (RestrictedSidsInToken = 0; RestrictedSidsInToken < AccessToken->RestrictedSidCount; RestrictedSidsInToken++) + { + AccessToken->RestrictedSids[RestrictedSidsInToken].Attributes |= (SE_GROUP_ENABLED | SE_GROUP_ENABLED_BY_DEFAULT | SE_GROUP_MANDATORY); + } + + /* + * As we added restricted SIDs into the token, mark + * it as restricted. + */ + AccessToken->TokenFlags |= TOKEN_IS_RESTRICTED; + } + /* Search for the primary group */ Status = SepFindPrimaryGroupAndDefaultOwner(AccessToken, Token->PrimaryGroup, @@ -2517,64 +2575,6 @@ SepPerformTokenFiltering( } } - /* - * Insert the restricted SIDs into the token on - * the request by the caller. - */ - if (RestrictedSidsIntoToken != NULL) - { - for (RestrictedSidsInList = 0; RestrictedSidsInList < RestrictedSidsCount; RestrictedSidsInList++) - { - /* Did the caller assign attributes to the restricted SIDs? */ - if (RestrictedSidsIntoToken[RestrictedSidsInList].Attributes != 0) - { - /* There mustn't be any attributes, bail out */ - DPRINT1("SepPerformTokenFiltering(): There mustn't be any attributes to restricted SIDs!\n"); - Status = STATUS_INVALID_PARAMETER; - goto Quit; - } - } - - /* - * Ensure that the token can hold the restricted SIDs - * (the variable length is calculated at the beginning - * of the routine call). - */ - ASSERT(VariableLength >= RestrictedSidsLength); - - /* - * Now let's begin inserting the restricted SIDs into the filtered - * access token from the list the caller gave us. - */ - AccessToken->RestrictedSidCount = RestrictedSidsCount; - AccessToken->RestrictedSids = EndMem; - EndMem = (PVOID)((ULONG_PTR)EndMem + RestrictedSidsLength); - VariableLength -= RestrictedSidsLength; - - RtlCopyMemory(AccessToken->RestrictedSids, - RestrictedSidsIntoToken, - AccessToken->RestrictedSidCount * sizeof(SID_AND_ATTRIBUTES)); - - /* - * As we've copied the restricted SIDs into - * the token, we must assign them the following - * combination of attributes SE_GROUP_ENABLED, - * SE_GROUP_ENABLED_BY_DEFAULT and SE_GROUP_MANDATORY. - * With such attributes we estabilish that restricting - * SIDs into the token are enabled for access checks. - */ - for (RestrictedSidsInToken = 0; RestrictedSidsInToken < AccessToken->RestrictedSidCount; RestrictedSidsInToken++) - { - AccessToken->RestrictedSids[RestrictedSidsInToken].Attributes |= (SE_GROUP_ENABLED | SE_GROUP_ENABLED_BY_DEFAULT | SE_GROUP_MANDATORY); - } - - /* - * As we added restricted SIDs into the token, mark - * it as restricted. - */ - AccessToken->TokenFlags |= TOKEN_IS_RESTRICTED; - } - /* We've finally filtered the token, return it to the caller */ *FilteredToken = AccessToken; Status = STATUS_SUCCESS; @@ -5045,7 +5045,7 @@ SepAdjustGroups( PAGED_CODE(); - /* Ensure that the token we get is not plain garbage */ + /* Ensure that the token we get is valid */ ASSERT(Token); /* Initialize the counters and begin the work */

2 years, 7 months

[reactos] 05/07: [NTOS:SE] NtAdjustGroupsToken(): Avoid double-free on ObReferenceObjectByHandle failure path (#4523)

by Hermès Bélusca-Maïto

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=96761885432e19d403592… commit 96761885432e19d4035922178e56c4c49f844153 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun May 22 18:33:46 2022 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon May 23 19:30:36 2022 +0200 [NTOS:SE] NtAdjustGroupsToken(): Avoid double-free on ObReferenceObjectByHandle failure path (#4523) --- ntoskrnl/se/token.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c index aad1321abd6..65018f60f3f 100644 --- a/ntoskrnl/se/token.c +++ b/ntoskrnl/se/token.c @@ -5256,7 +5256,6 @@ NtAdjustGroupsToken( ULONG NewStateSize = 0; PSID_AND_ATTRIBUTES CapturedGroups = NULL; BOOLEAN ChangesMade = FALSE; - BOOLEAN LockAndReferenceAcquired = FALSE; PAGED_CODE(); @@ -5363,12 +5362,11 @@ NtAdjustGroupsToken( TRUE); } - goto Quit; + return Status; } /* Lock the token */ SepAcquireTokenLockExclusive(Token); - LockAndReferenceAcquired = TRUE; /* Count the number of groups to be changed */ Status = SepAdjustGroups(Token, @@ -5442,13 +5440,9 @@ Quit: ExAllocateLocallyUniqueId(&Token->ModifiedId); } - /* Have we successfully acquired the lock and referenced the token before? */ - if (LockAndReferenceAcquired) - { - /* Unlock and dereference the token */ - SepReleaseTokenLock(Token); - ObDereferenceObject(Token); - } + /* Unlock and dereference the token */ + SepReleaseTokenLock(Token); + ObDereferenceObject(Token); /* Release the captured groups */ if (CapturedGroups != NULL)

2 years, 7 months

[reactos] 04/07: [NTOS:SE] Fix locking in SepDuplicateToken() and SepPerformTokenFiltering() (#4523)

by Hermès Bélusca-Maïto

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=3370652dfc2f1d0c37e06… commit 3370652dfc2f1d0c37e067b31ee37af67a6320c8 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Sun May 22 18:11:18 2022 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon May 23 19:30:35 2022 +0200 [NTOS:SE] Fix locking in SepDuplicateToken() and SepPerformTokenFiltering() (#4523) Shared locking must be used on the source token as it is accessed for reading only. This fixes in particular the kmtest SeTokenFiltering that would hang otherwise on a (wrong) exclusive locking. - SepPerformTokenFiltering(): Always shared-lock the source token. Its callers (NtFilterToken and SeFilterToken) just need to sanitize and put the parameters in correct form before calling this helper function. - Sync comments in NtFilterToken() with SeFilterToken(). --- ntoskrnl/se/token.c | 66 +++++++++++++++++++++++------------------------------ 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c index 96e972f2c5b..aad1321abd6 100644 --- a/ntoskrnl/se/token.c +++ b/ntoskrnl/se/token.c @@ -1059,7 +1059,7 @@ SepDuplicateToken( AccessToken->OriginatingLogonSession = Token->OriginatingLogonSession; /* Lock the source token and copy the mutable fields */ - SepAcquireTokenLockExclusive(Token); + SepAcquireTokenLockShared(Token); AccessToken->SessionId = Token->SessionId; RtlCopyLuid(&AccessToken->ModifiedId, &Token->ModifiedId); @@ -1286,23 +1286,20 @@ SepDuplicateToken( Token->DefaultDacl->AclSize); } - /* Unlock the source token */ - SepReleaseTokenLock(Token); - - /* Return the token */ + /* Return the token to the caller */ *NewAccessToken = AccessToken; Status = STATUS_SUCCESS; Quit: if (!NT_SUCCESS(Status)) { - /* Unlock the source token */ - SepReleaseTokenLock(Token); - /* Dereference the token, the delete procedure will clean it up */ ObDereferenceObject(AccessToken); } + /* Unlock the source token */ + SepReleaseTokenLock(Token); + return Status; } @@ -2086,8 +2083,8 @@ SepPerformTokenFiltering( _In_ KPROCESSOR_MODE PreviousMode, _Out_ PTOKEN *FilteredToken) { - PTOKEN AccessToken; NTSTATUS Status; + PTOKEN AccessToken; PVOID EndMem; ULONG RestrictedSidsLength; ULONG PrivilegesLength; @@ -2100,10 +2097,12 @@ SepPerformTokenFiltering( BOOLEAN WantPrivilegesDisabled; BOOLEAN FoundPrivilege; BOOLEAN FoundGroup; + PAGED_CODE(); - /* Ensure that the token we get is not garbage */ + /* Ensure that the source token is valid, and lock it */ ASSERT(Token); + SepAcquireTokenLockShared(Token); /* Assume the caller doesn't want privileges disabled */ WantPrivilegesDisabled = FALSE; @@ -2159,6 +2158,9 @@ SepPerformTokenFiltering( if (!NT_SUCCESS(Status)) { DPRINT1("SepPerformTokenFiltering(): Failed to create the filtered token object (Status 0x%lx)\n", Status); + + /* Unlock the source token and bail out */ + SepReleaseTokenLock(Token); return Status; } @@ -2168,10 +2170,7 @@ SepPerformTokenFiltering( /* Set up a lock for the new token */ Status = SepCreateTokenLock(AccessToken); if (!NT_SUCCESS(Status)) - { - ObDereferenceObject(AccessToken); - return Status; - } + goto Quit; /* Allocate new IDs for the token */ ExAllocateLocallyUniqueId(&AccessToken->TokenId); @@ -2576,7 +2575,7 @@ SepPerformTokenFiltering( AccessToken->TokenFlags |= TOKEN_IS_RESTRICTED; } - /* We've finally filtered the token, give it to the caller */ + /* We've finally filtered the token, return it to the caller */ *FilteredToken = AccessToken; Status = STATUS_SUCCESS; DPRINT("SepPerformTokenFiltering(): The token has been filtered!\n"); @@ -2584,10 +2583,13 @@ SepPerformTokenFiltering( Quit: if (!NT_SUCCESS(Status)) { - /* Dereference the token */ + /* Dereference the created token */ ObDereferenceObject(AccessToken); } + /* Unlock the source token */ + SepReleaseTokenLock(Token); + return Status; } @@ -2903,12 +2905,6 @@ SepCreateSystemAnonymousLogonTokenNoEveryone(VOID) * operations and that the access token has been filtered. STATUS_INVALID_PARAMETER * is returned if one or more of the parameter are not valid. A failure NTSTATUS code * is returned otherwise. - * - * @remarks - * WARNING -- The caller IS RESPONSIBLE for locking the existing access token - * before attempting to do any kind of filtering operation into - * the token. The lock MUST BE RELEASED after this kernel routine - * has finished doing its work. */ NTSTATUS NTAPI @@ -2925,6 +2921,7 @@ SeFilterToken( ULONG PrivilegesCount = 0; ULONG SidsCount = 0; ULONG RestrictedSidsCount = 0; + PAGED_CODE(); /* Begin copying the counters */ @@ -2969,11 +2966,11 @@ SeFilterToken( NULL); if (!NT_SUCCESS(Status)) { - DPRINT1("SeFilterToken(): Failed to insert the token (Status 0x%lx)\n", Status); + DPRINT1("SeFilterToken(): Failed to insert the filtered token (Status 0x%lx)\n", Status); return Status; } - /* Give it to the caller */ + /* Return it to the caller */ *FilteredToken = AccessToken; return Status; } @@ -6661,7 +6658,7 @@ NtFilterToken( } _SEH2_END; - /* Reference the token and do the job */ + /* Reference the token */ Status = ObReferenceObjectByHandle(ExistingTokenHandle, TOKEN_DUPLICATE, SeTokenObjectType, @@ -6674,9 +6671,6 @@ NtFilterToken( return Status; } - /* Lock the token */ - SepAcquireTokenLockExclusive(Token); - /* Capture the group SIDs */ if (SidsToDisable != NULL) { @@ -6734,7 +6728,7 @@ NtFilterToken( } } - /* Call the internal API so that it can filter the token for us */ + /* Call the internal API */ Status = SepPerformTokenFiltering(Token, CapturedPrivileges, CapturedSids, @@ -6751,7 +6745,7 @@ NtFilterToken( goto Quit; } - /* We got our filtered token, insert it to the handle */ + /* Insert the filtered token and retrieve a handle to it */ Status = ObInsertObject(FilteredToken, NULL, HandleInfo.GrantedAccess, @@ -6760,11 +6754,11 @@ NtFilterToken( &FilteredTokenHandle); if (!NT_SUCCESS(Status)) { - DPRINT1("NtFilterToken(): Failed to insert the filtered token object into the handle (Status 0x%lx)\n", Status); + DPRINT1("NtFilterToken(): Failed to insert the filtered token (Status 0x%lx)\n", Status); goto Quit; } - /* And give it to the caller once we're done */ + /* And return it to the caller once we're done */ _SEH2_TRY { *NewTokenHandle = FilteredTokenHandle; @@ -6777,17 +6771,15 @@ NtFilterToken( _SEH2_END; Quit: - /* Unlock and dereference the token */ - SepReleaseTokenLock(Token); + /* Dereference the token */ ObDereferenceObject(Token); - /* Release all the stuff we've captured */ + /* Release all the captured data */ if (CapturedSids != NULL) { SeReleaseSidAndAttributesArray(CapturedSids, PreviousMode, TRUE); - CapturedSids = NULL; } if (CapturedPrivileges != NULL) @@ -6795,7 +6787,6 @@ Quit: SeReleaseLuidAndAttributesArray(CapturedPrivileges, PreviousMode, TRUE); - CapturedPrivileges = NULL; } if (CapturedRestrictedSids != NULL) @@ -6803,7 +6794,6 @@ Quit: SeReleaseSidAndAttributesArray(CapturedRestrictedSids, PreviousMode, TRUE); - CapturedRestrictedSids = NULL; } return Status;

2 years, 7 months

[reactos] 03/07: [NTOS:SE] SepPerformTokenFiltering(): Remove useless SEH handling (#4523)

by Hermès Bélusca-Maïto

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b33911b93dacbc0a1dcdf… commit b33911b93dacbc0a1dcdf9869d5fb71b077a7617 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Fri May 20 02:26:21 2022 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Mon May 23 19:30:34 2022 +0200 [NTOS:SE] SepPerformTokenFiltering(): Remove useless SEH handling (#4523) This function is either called inter-kernel (in which case, all parameters must be valid, and if not, we have to bugcheck), or, it is called with **captured** parameters (from NtFilterToken) and those latter ones are now expected to be valid and reside in kernel-mode. Finally, data copied between token structures reside in kernel-mode only and again are expected to be valid (if not, we bugcheck). --- ntoskrnl/se/token.c | 132 +++++++++++----------------------------------------- 1 file changed, 26 insertions(+), 106 deletions(-) diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c index f215f4c1fe2..96e972f2c5b 100644 --- a/ntoskrnl/se/token.c +++ b/ntoskrnl/se/token.c @@ -2254,27 +2254,9 @@ SepPerformTokenFiltering( EndMem = (PVOID)((ULONG_PTR)EndMem + PrivilegesLength); VariableLength -= PrivilegesLength; - if (PreviousMode != KernelMode) - { - _SEH2_TRY - { - RtlCopyMemory(AccessToken->Privileges, - Token->Privileges, - AccessToken->PrivilegeCount * sizeof(LUID_AND_ATTRIBUTES)); - } - _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) - { - Status = _SEH2_GetExceptionCode(); - _SEH2_YIELD(goto Quit); - } - _SEH2_END; - } - else - { - RtlCopyMemory(AccessToken->Privileges, - Token->Privileges, - AccessToken->PrivilegeCount * sizeof(LUID_AND_ATTRIBUTES)); - } + RtlCopyMemory(AccessToken->Privileges, + Token->Privileges, + AccessToken->PrivilegeCount * sizeof(LUID_AND_ATTRIBUTES)); } /* Copy the user and groups */ @@ -2287,39 +2269,17 @@ SepPerformTokenFiltering( EndMem = &AccessToken->UserAndGroups[AccessToken->UserAndGroupCount]; VariableLength -= ((ULONG_PTR)EndMem - (ULONG_PTR)AccessToken->UserAndGroups); - if (PreviousMode != KernelMode) - { - _SEH2_TRY - { - Status = RtlCopySidAndAttributesArray(AccessToken->UserAndGroupCount, - Token->UserAndGroups, - VariableLength, - AccessToken->UserAndGroups, - EndMem, - &EndMem, - &VariableLength); - } - _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) - { - Status = _SEH2_GetExceptionCode(); - _SEH2_YIELD(goto Quit); - } - _SEH2_END; - } - else + Status = RtlCopySidAndAttributesArray(AccessToken->UserAndGroupCount, + Token->UserAndGroups, + VariableLength, + AccessToken->UserAndGroups, + EndMem, + &EndMem, + &VariableLength); + if (!NT_SUCCESS(Status)) { - Status = RtlCopySidAndAttributesArray(AccessToken->UserAndGroupCount, - Token->UserAndGroups, - VariableLength, - AccessToken->UserAndGroups, - EndMem, - &EndMem, - &VariableLength); - if (!NT_SUCCESS(Status)) - { - DPRINT1("SepPerformTokenFiltering(): Failed to copy the groups into token (Status 0x%lx)\n", Status); - goto Quit; - } + DPRINT1("SepPerformTokenFiltering(): Failed to copy the groups into token (Status 0x%lx)\n", Status); + goto Quit; } } @@ -2333,39 +2293,17 @@ SepPerformTokenFiltering( EndMem = &AccessToken->RestrictedSids[AccessToken->RestrictedSidCount]; VariableLength -= ((ULONG_PTR)EndMem - (ULONG_PTR)AccessToken->RestrictedSids); - if (PreviousMode != KernelMode) - { - _SEH2_TRY - { - Status = RtlCopySidAndAttributesArray(AccessToken->RestrictedSidCount, - Token->RestrictedSids, - VariableLength, - AccessToken->RestrictedSids, - EndMem, - &EndMem, - &VariableLength); - } - _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) - { - Status = _SEH2_GetExceptionCode(); - _SEH2_YIELD(goto Quit); - } - _SEH2_END; - } - else + Status = RtlCopySidAndAttributesArray(AccessToken->RestrictedSidCount, + Token->RestrictedSids, + VariableLength, + AccessToken->RestrictedSids, + EndMem, + &EndMem, + &VariableLength); + if (!NT_SUCCESS(Status)) { - Status = RtlCopySidAndAttributesArray(AccessToken->RestrictedSidCount, - Token->RestrictedSids, - VariableLength, - AccessToken->RestrictedSids, - EndMem, - &EndMem, - &VariableLength); - if (!NT_SUCCESS(Status)) - { - DPRINT1("SepPerformTokenFiltering(): Failed to copy the restricted SIDs into token (Status 0x%lx)\n", Status); - goto Quit; - } + DPRINT1("SepPerformTokenFiltering(): Failed to copy the restricted SIDs into token (Status 0x%lx)\n", Status); + goto Quit; } } @@ -2614,27 +2552,9 @@ SepPerformTokenFiltering( EndMem = (PVOID)((ULONG_PTR)EndMem + RestrictedSidsLength); VariableLength -= RestrictedSidsLength; - if (PreviousMode != KernelMode) - { - _SEH2_TRY - { - RtlCopyMemory(AccessToken->RestrictedSids, - RestrictedSidsIntoToken, - AccessToken->RestrictedSidCount * sizeof(SID_AND_ATTRIBUTES)); - } - _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) - { - Status = _SEH2_GetExceptionCode(); - _SEH2_YIELD(goto Quit); - } - _SEH2_END; - } - else - { - RtlCopyMemory(AccessToken->RestrictedSids, - RestrictedSidsIntoToken, - AccessToken->RestrictedSidCount * sizeof(SID_AND_ATTRIBUTES)); - } + RtlCopyMemory(AccessToken->RestrictedSids, + RestrictedSidsIntoToken, + AccessToken->RestrictedSidCount * sizeof(SID_AND_ATTRIBUTES)); /* * As we've copied the restricted SIDs into

2 years, 7 months

← Newer
1
2
3
4
5
6
7
8
...
17
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-diffs May 2022