Ros-diffs October 2023

ros-diffs@reactos.org

21 participants
140 discussions

[reactos] 01/01: [REGEDIT] Use <strsafe.h> functions strictly (#5825)

by Katayama Hirofumi MZ

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8cef980ab96b7027120f7… commit 8cef980ab96b7027120f759ccd467bbaa278302b Author: Katayama Hirofumi MZ <katayama.hirofumi.mz(a)gmail.com> AuthorDate: Thu Oct 26 10:09:31 2023 +0900 Commit: GitHub <noreply(a)github.com> CommitDate: Thu Oct 26 10:09:31 2023 +0900 [REGEDIT] Use <strsafe.h> functions strictly (#5825) Use safer functions for buffer manipulation to avoid buffer overflow/overrun. CORE-18876 --- base/applications/regedit/childwnd.c | 9 +++++---- base/applications/regedit/edit.c | 26 ++++++++++++++------------ base/applications/regedit/find.c | 21 ++++++++++----------- base/applications/regedit/framewnd.c | 8 +++++--- base/applications/regedit/security.c | 3 +-- base/applications/regedit/txtproc.c | 5 +++-- 6 files changed, 38 insertions(+), 34 deletions(-) diff --git a/base/applications/regedit/childwnd.c b/base/applications/regedit/childwnd.c index 67a88bc90a5..d77ed1899b3 100644 --- a/base/applications/regedit/childwnd.c +++ b/base/applications/regedit/childwnd.c @@ -177,7 +177,7 @@ static void SuggestKeys(HKEY hRootKey, LPCWSTR pszKeyPath, LPWSTR pszSuggestions RegCloseKey(hOtherKey); bFound = TRUE; - wcscpy(szLastFound, szBuffer); + StringCbCopyW(szLastFound, sizeof(szLastFound), szBuffer); pszKeyPath = szLastFound; } } @@ -258,10 +258,11 @@ UpdateAddress(HTREEITEM hItem, HKEY hRootKey, LPCWSTR pszPath, BOOL bSelectNone) if (fullPath) { /* set (correct) the address bar text */ - if (keyPath[0] != L'\0') - swprintf(fullPath, L"%s%s%s", rootName, keyPath[0]==L'\\'?L"":L"\\", keyPath); + if (keyPath[0] != UNICODE_NULL) + StringCbPrintfW(fullPath, cbFullPath, L"%s%s%s", rootName, + ((keyPath[0] == L'\\') ? L"" : L"\\"), keyPath); else - fullPath = wcscpy(fullPath, rootName); + StringCbCopyW(fullPath, cbFullPath, rootName); SendMessageW(hStatusBar, SB_SETTEXTW, 0, (LPARAM)fullPath); SendMessageW(g_pChildWnd->hAddressBarWnd, WM_SETTEXT, 0, (LPARAM)fullPath); diff --git a/base/applications/regedit/edit.c b/base/applications/regedit/edit.c index d84361c7a0a..8805ccb35e7 100644 --- a/base/applications/regedit/edit.c +++ b/base/applications/regedit/edit.c @@ -50,10 +50,10 @@ void error(HWND hwnd, INT resId, ...) hInstance = GetModuleHandle(0); if (!LoadStringW(hInstance, IDS_ERROR, title, ARRAY_SIZE(title))) - wcscpy(title, L"Error"); + StringCbCopyW(title, sizeof(title), L"Error"); if (!LoadStringW(hInstance, resId, errfmt, ARRAY_SIZE(errfmt))) - wcscpy(errfmt, L"Unknown error string!"); + StringCbCopyW(errfmt, sizeof(errfmt), L"Unknown error string!"); va_start(ap, resId); _vsnwprintf(errstr, ARRAY_SIZE(errstr), errfmt, ap); @@ -66,7 +66,7 @@ static void error_code_messagebox(HWND hwnd, DWORD error_code) { WCHAR title[256]; if (!LoadStringW(hInst, IDS_ERROR, title, ARRAY_SIZE(title))) - wcscpy(title, L"Error"); + StringCbCopyW(title, sizeof(title), L"Error"); ErrorMessageBox(hwnd, title, error_code); } @@ -81,13 +81,13 @@ void warning(HWND hwnd, INT resId, ...) hInstance = GetModuleHandle(0); if (!LoadStringW(hInstance, IDS_WARNING, title, ARRAY_SIZE(title))) - wcscpy(title, L"Warning"); + StringCbCopyW(title, sizeof(title), L"Warning"); if (!LoadStringW(hInstance, resId, errfmt, ARRAY_SIZE(errfmt))) - wcscpy(errfmt, L"Unknown error string!"); + StringCbCopyW(errfmt, sizeof(errfmt), L"Unknown error string!"); va_start(ap, resId); - _vsnwprintf(errstr, ARRAY_SIZE(errstr), errfmt, ap); + StringCbVPrintfW(errstr, sizeof(errstr), errfmt, ap); va_end(ap); MessageBoxW(hwnd, errstr, title, MB_OK | MB_ICONSTOP); @@ -304,7 +304,7 @@ INT_PTR CALLBACK modify_dword_dlgproc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LP SetDlgItemTextW(hwndDlg, IDC_VALUE_NAME, buffer); } CheckRadioButton (hwndDlg, IDC_FORMAT_HEX, IDC_FORMAT_DEC, IDC_FORMAT_HEX); - swprintf(ValueString, L"%lx", dwordValueData); + StringCbPrintfW(ValueString, sizeof(ValueString), L"%lx", dwordValueData); SetDlgItemTextW(hwndDlg, IDC_VALUE_DATA, ValueString); SendMessage(GetDlgItem(hwndDlg, IDC_VALUE_DATA), EM_SETSEL, 0, -1); SetFocus(GetDlgItem(hwndDlg, IDC_VALUE_DATA)); @@ -327,7 +327,7 @@ INT_PTR CALLBACK modify_dword_dlgproc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LP } } } - swprintf(ValueString, L"%lx", Value); + StringCbPrintfW(ValueString, sizeof(ValueString), L"%lx", Value); SetDlgItemTextW(hwndDlg, IDC_VALUE_DATA, ValueString); return TRUE; } @@ -347,7 +347,7 @@ INT_PTR CALLBACK modify_dword_dlgproc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LP } } } - swprintf(ValueString, L"%lu", Value); + StringCbPrintfW(ValueString, sizeof(ValueString), L"%lu", Value); SetDlgItemTextW(hwndDlg, IDC_VALUE_DATA, ValueString); return TRUE; } @@ -1491,6 +1491,7 @@ LONG RenameKey(HKEY hKey, LPCWSTR lpSubKey, LPCWSTR lpNewName) LPCWSTR s; LPWSTR lpNewSubKey = NULL; LONG Ret = 0; + SIZE_T cbNewSubKey; if (!lpSubKey) return Ret; @@ -1499,11 +1500,12 @@ LONG RenameKey(HKEY hKey, LPCWSTR lpSubKey, LPCWSTR lpNewName) if (s) { s++; - lpNewSubKey = (LPWSTR) HeapAlloc(GetProcessHeap(), 0, (s - lpSubKey + wcslen(lpNewName) + 1) * sizeof(WCHAR)); + cbNewSubKey = (s - lpSubKey + wcslen(lpNewName) + 1) * sizeof(WCHAR); + lpNewSubKey = (LPWSTR) HeapAlloc(GetProcessHeap(), 0, cbNewSubKey); if (lpNewSubKey != NULL) { - memcpy(lpNewSubKey, lpSubKey, (s - lpSubKey) * sizeof(WCHAR)); - wcscpy(lpNewSubKey + (s - lpSubKey), lpNewName); + StringCbCopyNW(lpNewSubKey, cbNewSubKey, lpSubKey, (s - lpSubKey) * sizeof(WCHAR)); + StringCbCatW(lpNewSubKey, cbNewSubKey, lpNewName); lpNewName = lpNewSubKey; } else diff --git a/base/applications/regedit/find.c b/base/applications/regedit/find.c index 444f4a25952..9405271a916 100644 --- a/base/applications/regedit/find.c +++ b/base/applications/regedit/find.c @@ -144,7 +144,7 @@ BOOL RegFindRecurse( if(wcslen(pszSubKey) >= _countof(szSubKey)) return FALSE; - wcscpy(szSubKey, pszSubKey); + StringCbCopyW(szSubKey, sizeof(szSubKey), pszSubKey); hSubKey = NULL; lResult = RegOpenKeyExW(hKey, szSubKey, 0, KEY_ALL_ACCESS, &hSubKey); @@ -304,14 +304,14 @@ BOOL RegFindRecurse( ppszFoundValueName)) { LPWSTR psz = *ppszFoundSubKey; - *ppszFoundSubKey = malloc( - (wcslen(szSubKey) + wcslen(psz) + 2) * sizeof(WCHAR)); + SIZE_T cbFoundSubKey = (wcslen(szSubKey) + wcslen(psz) + 2) * sizeof(WCHAR); + *ppszFoundSubKey = malloc(cbFoundSubKey); if (*ppszFoundSubKey == NULL) goto err; if (szSubKey[0]) { - wcscpy(*ppszFoundSubKey, szSubKey); - wcscat(*ppszFoundSubKey, s_backslash); + StringCbCopyW(*ppszFoundSubKey, cbFoundSubKey, szSubKey); + StringCbCatW(*ppszFoundSubKey, cbFoundSubKey, s_backslash); } else **ppszFoundSubKey = 0; @@ -368,7 +368,7 @@ BOOL RegFindWalk( ppszFoundValueName)) return TRUE; - wcscpy(szSubKey, pszSubKey); + StringCbCopyW(szSubKey, sizeof(szSubKey), pszSubKey); while(szSubKey[0] != 0) { if (DoEvents()) @@ -460,15 +460,14 @@ BOOL RegFindWalk( ppszFoundSubKey, ppszFoundValueName)) { LPWSTR psz = *ppszFoundSubKey; - *ppszFoundSubKey = malloc( - (wcslen(szSubKey) + wcslen(psz) + 2) * - sizeof(WCHAR)); + SIZE_T cbFoundSubKey = (wcslen(szSubKey) + wcslen(psz) + 2) * sizeof(WCHAR); + *ppszFoundSubKey = malloc(cbFoundSubKey); if (*ppszFoundSubKey == NULL) goto err; if (szSubKey[0]) { - wcscpy(*ppszFoundSubKey, szSubKey); - wcscat(*ppszFoundSubKey, s_backslash); + StringCbCopyW(*ppszFoundSubKey, cbFoundSubKey, szSubKey); + StringCbCatW(*ppszFoundSubKey, cbFoundSubKey, s_backslash); } else **ppszFoundSubKey = 0; diff --git a/base/applications/regedit/framewnd.c b/base/applications/regedit/framewnd.c index 6b5b1c28cfb..6f382a072cd 100644 --- a/base/applications/regedit/framewnd.c +++ b/base/applications/regedit/framewnd.c @@ -138,7 +138,7 @@ static void OnMenuSelect(HWND hWnd, UINT nItemID, UINT nFlags, HMENU hSysMenu) { WCHAR str[100]; - wcscpy(str, L""); + str[0] = UNICODE_NULL; if (nFlags & MF_POPUP) { if (hSysMenu != GetMenu(hWnd)) @@ -814,6 +814,7 @@ BOOL CopyKeyName(HWND hWnd, HKEY hRootKey, LPCWSTR keyName) WCHAR szBuffer[512]; HGLOBAL hGlobal; LPWSTR s; + SIZE_T cbGlobal; if (!OpenClipboard(hWnd)) goto done; @@ -825,12 +826,13 @@ BOOL CopyKeyName(HWND hWnd, HKEY hRootKey, LPCWSTR keyName) if (!GetKeyName(szBuffer, ARRAY_SIZE(szBuffer), hRootKey, keyName)) goto done; - hGlobal = GlobalAlloc(GMEM_MOVEABLE, (wcslen(szBuffer) + 1) * sizeof(WCHAR)); + cbGlobal = (wcslen(szBuffer) + 1) * sizeof(WCHAR); + hGlobal = GlobalAlloc(GMEM_MOVEABLE, cbGlobal); if (!hGlobal) goto done; s = GlobalLock(hGlobal); - wcscpy(s, szBuffer); + StringCbCopyW(s, cbGlobal, szBuffer); GlobalUnlock(hGlobal); SetClipboardData(CF_UNICODETEXT, hGlobal); diff --git a/base/applications/regedit/security.c b/base/applications/regedit/security.c index 71251583e9c..0ea90092d91 100644 --- a/base/applications/regedit/security.c +++ b/base/applications/regedit/security.c @@ -782,8 +782,7 @@ CRegKeySecurity_fnConstructor(LPWSTR lpRegKey, obj->ObjectInfo = *ObjectInfo; obj->Btn = Btn; obj->hRootKey = hRootKey; - wcscpy(obj->szRegKey, - lpRegKey); + StringCbCopyW(obj->szRegKey, sizeof(obj->szRegKey), lpRegKey); } else SetLastError(ERROR_NOT_ENOUGH_MEMORY); diff --git a/base/applications/regedit/txtproc.c b/base/applications/regedit/txtproc.c index f5e3ea041c2..092930e6f7a 100644 --- a/base/applications/regedit/txtproc.c +++ b/base/applications/regedit/txtproc.c @@ -194,8 +194,9 @@ static WCHAR * txt_build_subkey_path(LPCWSTR path, DWORD path_len, LPCWSTR subkey_name, DWORD subkey_len) { WCHAR *subkey_path; - subkey_path = malloc((path_len + subkey_len + 2) * sizeof(WCHAR)); - swprintf(subkey_path, L"%s\\%s", path, subkey_name); + SIZE_T cb_subkey_path = (path_len + subkey_len + 2) * sizeof(WCHAR); + subkey_path = malloc(cb_subkey_path); + StringCbPrintfW(subkey_path, cb_subkey_path, L"%s\\%s", path, subkey_name); return subkey_path; }

10 months, 3 weeks

[reactos] 01/01: [SYSSETUP] syssetup.rc improve URLs (#5810)

by Joachim Henze

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=9360423cc22d1b5a8f064… commit 9360423cc22d1b5a8f064895ba2390edc6dea045 Author: Joachim Henze <joachim.henze(a)reactos.org> AuthorDate: Tue Oct 24 22:59:28 2023 +0200 Commit: GitHub <noreply(a)github.com> CommitDate: Tue Oct 24 20:59:28 2023 +0000 [SYSSETUP] syssetup.rc improve URLs (#5810) Reformat those URLs in a way that allows new lines to be added, or existing lines to be removed or changed, without the need to touch any neighbor line. This will also allow git blame to work much better in the future. Fix outdated URLs: http://www.icu-project.org -> http://icu.unicode.org https://fedorahosted.org/liberation-fonts -> http://pagure.io/liberation-fonts I checked: all of the URLs do still work as good as before. * Update ext2fsd links in syssetup.rc and media/doc --- dll/win32/syssetup/syssetup.rc | 17 ++++++++++++++++- media/doc/3rd Party Files.txt | 3 ++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/dll/win32/syssetup/syssetup.rc b/dll/win32/syssetup/syssetup.rc index 77cec6efc3b..206e9a0c206 100644 --- a/dll/win32/syssetup/syssetup.rc +++ b/dll/win32/syssetup/syssetup.rc @@ -33,7 +33,22 @@ IDI_ICON5 ICON "resources/5.ico" STRINGTABLE BEGIN - IDS_ACKPROJECTS "Wine - http://www.winehq.org\nFreeType - http://www.freetype.org\nSYSLINUX - http://syslinux.zytor.com\nMinGW - http://www.mingw.org\nBochs - http://bochs.sourceforge.net\nMesa3D - http://www.mesa3d.org\nadns - http://www.gnu.org/software/adns\nICU - http://www.icu-project.org/\nGraphApp - http://enchantia.com/software/graphapp/\nExt2 - http://www.ext2fsd.com/\nGNU FreeFont - http://savannah.gnu.org/projects/freefont/\nDejaVu Fonts - http://dejavu.sourceforge.net\nLi [...] + IDS_ACKPROJECTS "Wine - http://winehq.org\ +\nFreeType - http://freetype.org\ +\nSYSLINUX - http://syslinux.zytor.com\ +\nMinGW - http://mingw.org\ +\nBochs - http://bochs.sourceforge.net\ +\nMesa3D - http://mesa3d.org\ +\nadns - http://gnu.org/software/adns\ +\nICU - http://icu.unicode.org\ +\nGraphApp - http://enchantia.com/software/graphapp\ +\nExt2 - http://sourceforge.net/projects/ext2fsd\ +\nGNU FreeFont - http://savannah.gnu.org/projects/freefont\ +\nDejaVu Fonts - http://dejavu.sourceforge.net\ +\nLiberation(tm) Fonts - http://pagure.io/liberation-fonts\ +\nBtrfs - https://github.com/maharmstone/btrfs\ +\nTango Desktop Project - http://tango.freedesktop.org/Tango_Desktop_Project\ +" END IDR_GPL RT_TEXT "COPYING" diff --git a/media/doc/3rd Party Files.txt b/media/doc/3rd Party Files.txt index f1633295bfa..b39199a5cd6 100644 --- a/media/doc/3rd Party Files.txt +++ b/media/doc/3rd Party Files.txt @@ -93,7 +93,8 @@ Path: drivers/filesystems/ext2 Path: sdk/lib/fslib/ext2lib Used Version: 0.69 License: GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later.html) -URL: http://www.ext2fsd.com +URL: https://web.archive.org/web/20210516213924/http://www.ext2fsd.com +URL: https://sourceforge.net/projects/ext2fsd Title: NFSv4.1 Client for Windows Path: base/services/nfsd

10 months, 3 weeks

[reactos] 09/09: [NTOS:MM] Add ASSERTs for VAD table locking

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=d831bc4fab38300e36765… commit d831bc4fab38300e36765cf591059c8fd5752926 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Tue Oct 10 02:41:19 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Tue Oct 24 21:45:27 2023 +0300 [NTOS:MM] Add ASSERTs for VAD table locking --- ntoskrnl/mm/ARM3/vadnode.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/ntoskrnl/mm/ARM3/vadnode.c b/ntoskrnl/mm/ARM3/vadnode.c index 050e1c92235..53b8fa83dbd 100644 --- a/ntoskrnl/mm/ARM3/vadnode.c +++ b/ntoskrnl/mm/ARM3/vadnode.c @@ -43,6 +43,74 @@ CHAR MmReadWrite[32] = /* FUNCTIONS ******************************************************************/ +extern MM_AVL_TABLE MiRosKernelVadRoot; + +#if DBG + +static +VOID +MiDbgAssertIsLockedForRead(_In_ PMM_AVL_TABLE Table) +{ + if (Table == &MmSectionBasedRoot) + { + /* Need to hold MmSectionBasedMutex */ + ASSERT(MmSectionBasedMutex.Owner == KeGetCurrentThread()); + } + else if (Table == &MiRosKernelVadRoot) + { + /* Need to hold either the system working-set lock or + the idle process' AddressCreationLock */ + ASSERT(PsGetCurrentThread()->OwnsSystemWorkingSetExclusive || + PsGetCurrentThread()->OwnsSystemWorkingSetShared || + (PsIdleProcess->AddressCreationLock.Owner == KeGetCurrentThread())); + } + else + { + /* Need to hold either the process working-set lock or + the current process' AddressCreationLock */ + PEPROCESS Process = CONTAINING_RECORD(Table, EPROCESS, VadRoot); + ASSERT(MI_WS_OWNER(Process) || + (Process->AddressCreationLock.Owner == KeGetCurrentThread())); + } +} + +static +VOID +MiDbgAssertIsLockedForWrite(_In_ PMM_AVL_TABLE Table) +{ + if (Table == &MmSectionBasedRoot) + { + /* Need to hold MmSectionBasedMutex */ + ASSERT(MmSectionBasedMutex.Owner == KeGetCurrentThread()); + } + else if (Table == &MiRosKernelVadRoot) + { + /* Need to hold both the system working-set lock exclusive and + the idle process' AddressCreationLock */ + ASSERT(PsGetCurrentThread()->OwnsSystemWorkingSetExclusive); + ASSERT(PsIdleProcess->AddressCreationLock.Owner == KeGetCurrentThread()); + } + else + { + /* Need to hold both the process working-set lock exclusive and + the current process' AddressCreationLock */ + PEPROCESS Process = CONTAINING_RECORD(Table, EPROCESS, VadRoot); + ASSERT(Process == PsGetCurrentProcess()); + ASSERT(PsGetCurrentThread()->OwnsProcessWorkingSetExclusive); + ASSERT(Process->AddressCreationLock.Owner == KeGetCurrentThread()); + } +} + +#define ASSERT_LOCKED_FOR_READ(Table) MiDbgAssertIsLockedForRead(Table) +#define ASSERT_LOCKED_FOR_WRITE(Table) MiDbgAssertIsLockedForWrite(Table) + +#else // DBG + +#define ASSERT_LOCKED_FOR_READ(Table) +#define ASSERT_LOCKED_FOR_WRITE(Table) + +#endif // DBG + PMMVAD NTAPI MiLocateAddress(IN PVOID VirtualAddress) @@ -52,6 +120,8 @@ MiLocateAddress(IN PVOID VirtualAddress) PMM_AVL_TABLE Table = &PsGetCurrentProcess()->VadRoot; TABLE_SEARCH_RESULT SearchResult; + ASSERT_LOCKED_FOR_READ(Table); + /* Start with the the hint */ FoundVad = (PMMVAD)Table->NodeHint; if (!FoundVad) return NULL; @@ -69,6 +139,8 @@ MiLocateAddress(IN PVOID VirtualAddress) /* We found it, update the hint */ ASSERT(FoundVad != NULL); ASSERT((Vpn >= FoundVad->StartingVpn) && (Vpn <= FoundVad->EndingVpn)); + + /* We allow this (atomic) update without exclusive lock, because it's a hint only */ Table->NodeHint = FoundVad; return FoundVad; } @@ -82,6 +154,8 @@ MiCheckForConflictingNode(IN ULONG_PTR StartVpn, { PMMADDRESS_NODE ParentNode, CurrentNode; + ASSERT_LOCKED_FOR_READ(Table); + /* If the tree is empty, there is no conflict */ if (Table->NumberGenericTableElements == 0) return TableEmptyTree; @@ -132,6 +206,8 @@ MiInsertNode(IN PMM_AVL_TABLE Table, { PMMVAD_LONG Vad; + ASSERT_LOCKED_FOR_WRITE(Table); + /* Insert it into the tree */ RtlpInsertAvlTreeNode(Table, NewNode, Parent, Result); @@ -186,6 +262,8 @@ MiInsertVad(IN PMMVAD Vad, TABLE_SEARCH_RESULT Result; PMMADDRESS_NODE Parent = NULL; + ASSERT_LOCKED_FOR_WRITE(VadRoot); + /* Validate the VAD and set it as the current hint */ ASSERT(Vad->EndingVpn >= Vad->StartingVpn); VadRoot->NodeHint = Vad; @@ -348,6 +426,8 @@ MiInsertBasedSection(IN PSECTION Section) PMMADDRESS_NODE Parent = NULL; ASSERT(Section->Address.EndingVpn >= Section->Address.StartingVpn); + ASSERT_LOCKED_FOR_WRITE(&MmSectionBasedRoot); + /* Find the parent VAD and where this child should be inserted */ Result = RtlpFindAvlTableNodeOrParent(&MmSectionBasedRoot, (PVOID)Section->Address.StartingVpn, &Parent); ASSERT(Result != TableFoundNode); @@ -362,6 +442,8 @@ MiRemoveNode(IN PMMADDRESS_NODE Node, { PMMVAD_LONG Vad; + ASSERT_LOCKED_FOR_WRITE(Table); + /* Call the AVL code */ RtlpDeleteAvlTreeNode(Table, Node); @@ -509,6 +591,8 @@ MiFindEmptyAddressRangeInTree(IN SIZE_T Length, ULONG_PTR PageCount, AlignmentVpn, LowVpn, HighestVpn; ASSERT(Length != 0); + ASSERT_LOCKED_FOR_READ(Table); + /* Calculate page numbers for the length, alignment, and starting address */ PageCount = BYTES_TO_PAGES(Length); AlignmentVpn = Alignment >> PAGE_SHIFT; @@ -605,6 +689,8 @@ MiFindEmptyAddressRangeDownTree(IN SIZE_T Length, ULONG_PTR LowVpn, HighVpn, AlignmentVpn; PFN_NUMBER PageCount; + ASSERT_LOCKED_FOR_READ(Table); + /* Sanity checks */ ASSERT(BoundaryAddress); ASSERT(BoundaryAddress <= ((ULONG_PTR)MM_HIGHEST_VAD_ADDRESS)); @@ -719,6 +805,8 @@ MiFindEmptyAddressRangeDownBasedTree(IN SIZE_T Length, PMMADDRESS_NODE Node, LowestNode; ULONG_PTR LowVpn, BestVpn; + ASSERT_LOCKED_FOR_READ(Table); + /* Sanity checks */ ASSERT(Table == &MmSectionBasedRoot); ASSERT(BoundaryAddress);

10 months, 4 weeks

[reactos] 08/09: [NTOS:MM] Lock kernel address space in MiInitSystemMemoryAreas

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=23589e9bf14afe5763a1f… commit 23589e9bf14afe5763a1fede921b1f67ef949246 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Tue Oct 10 02:33:53 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Tue Oct 24 21:45:27 2023 +0300 [NTOS:MM] Lock kernel address space in MiInitSystemMemoryAreas This is required to satisfy VAD locking rules. --- ntoskrnl/mm/mminit.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ntoskrnl/mm/mminit.c b/ntoskrnl/mm/mminit.c index 3e58ad9bbc8..009383992b7 100644 --- a/ntoskrnl/mm/mminit.c +++ b/ntoskrnl/mm/mminit.c @@ -1,4 +1,4 @@ -/* +/* * PROJECT: ReactOS Kernel * LICENSE: GPL - See COPYING in the top level directory * FILE: ntoskrnl/mm/mminit.c @@ -68,6 +68,8 @@ MiInitSystemMemoryAreas(VOID) // Create all the static memory areas. // + MmLockAddressSpace(MmGetKernelAddressSpace()); + #ifdef _M_AMD64 // Reserved range FFFF800000000000 - FFFFF68000000000 MiCreateArm3StaticMemoryArea((PVOID)MI_REAL_SYSTEM_RANGE_START, PTE_BASE - MI_REAL_SYSTEM_RANGE_START, FALSE); @@ -118,6 +120,8 @@ MiInitSystemMemoryAreas(VOID) // KUSER_SHARED_DATA MiCreateArm3StaticMemoryArea((PVOID)KI_USER_SHARED_DATA, PAGE_SIZE, FALSE); #endif /* _X86_ */ + + MmUnlockAddressSpace(MmGetKernelAddressSpace()); } CODE_SEG("INIT")

10 months, 4 weeks

[reactos] 07/09: [NTOS:MM] Attach to the target process in MmMapViewOfSection

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=07ad8c4c11d72f9603cef… commit 07ad8c4c11d72f9603cef8ad667664a7ce9a44f6 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Mon Oct 9 21:45:01 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Tue Oct 24 21:45:27 2023 +0300 [NTOS:MM] Attach to the target process in MmMapViewOfSection This is required to satisfy VAD locking rules. --- ntoskrnl/mm/section.c | 49 +++++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/ntoskrnl/mm/section.c b/ntoskrnl/mm/section.c index 35c36d45cfa..ff77ef77077 100644 --- a/ntoskrnl/mm/section.c +++ b/ntoskrnl/mm/section.c @@ -4008,6 +4008,8 @@ MmMapViewOfSection(IN PVOID SectionObject, PMMSUPPORT AddressSpace; NTSTATUS Status = STATUS_SUCCESS; BOOLEAN NotAtBase = FALSE; + BOOLEAN IsAttached = FALSE; + KAPC_STATE ApcState; if (MiIsRosSectionObject(SectionObject) == FALSE) { @@ -4031,6 +4033,12 @@ MmMapViewOfSection(IN PVOID SectionObject, return STATUS_INVALID_PAGE_PROTECTION; } + if (PsGetCurrentProcess() != Process) + { + KeStackAttachProcess(&Process->Pcb, &ApcState); + IsAttached = TRUE; + } + /* FIXME: We should keep this, but it would break code checking equality */ Protect &= ~PAGE_NOCACHE; @@ -4097,15 +4105,15 @@ MmMapViewOfSection(IN PVOID SectionObject, /* Fail if the user requested a fixed base address. */ if ((*BaseAddress) != NULL) { - MmUnlockAddressSpace(AddressSpace); - return STATUS_CONFLICTING_ADDRESSES; + Status = STATUS_CONFLICTING_ADDRESSES; + goto Exit; } /* Otherwise find a gap to map the image. */ ImageBase = (ULONG_PTR)MmFindGap(AddressSpace, PAGE_ROUND_UP(ImageSize), MM_VIRTMEM_GRANULARITY, FALSE); if (ImageBase == 0) { - MmUnlockAddressSpace(AddressSpace); - return STATUS_CONFLICTING_ADDRESSES; + Status = STATUS_CONFLICTING_ADDRESSES; + goto Exit; } /* Remember that we loaded image at a different base address */ NotAtBase = TRUE; @@ -4136,8 +4144,7 @@ MmMapViewOfSection(IN PVOID SectionObject, MmUnlockSectionSegment(&SectionSegments[i]); } - MmUnlockAddressSpace(AddressSpace); - return Status; + goto Exit; } } @@ -4160,22 +4167,22 @@ MmMapViewOfSection(IN PVOID SectionObject, if ((Protect & (PAGE_READWRITE|PAGE_EXECUTE_READWRITE)) && !(Section->InitialPageProtection & (PAGE_READWRITE|PAGE_EXECUTE_READWRITE))) { - MmUnlockAddressSpace(AddressSpace); - return STATUS_SECTION_PROTECTION; + Status = STATUS_SECTION_PROTECTION; + goto Exit; } /* check for read access */ if ((Protect & (PAGE_READONLY|PAGE_WRITECOPY|PAGE_EXECUTE_READ|PAGE_EXECUTE_WRITECOPY)) && !(Section->InitialPageProtection & (PAGE_READONLY|PAGE_READWRITE|PAGE_WRITECOPY|PAGE_EXECUTE_READ|PAGE_EXECUTE_READWRITE|PAGE_EXECUTE_WRITECOPY))) { - MmUnlockAddressSpace(AddressSpace); - return STATUS_SECTION_PROTECTION; + Status = STATUS_SECTION_PROTECTION; + goto Exit; } /* check for execute access */ if ((Protect & (PAGE_EXECUTE|PAGE_EXECUTE_READ|PAGE_EXECUTE_READWRITE|PAGE_EXECUTE_WRITECOPY)) && !(Section->InitialPageProtection & (PAGE_EXECUTE|PAGE_EXECUTE_READ|PAGE_EXECUTE_READWRITE|PAGE_EXECUTE_WRITECOPY))) { - MmUnlockAddressSpace(AddressSpace); - return STATUS_SECTION_PROTECTION; + Status = STATUS_SECTION_PROTECTION; + goto Exit; } if (SectionOffset == NULL) @@ -4189,8 +4196,8 @@ MmMapViewOfSection(IN PVOID SectionObject, if ((ViewOffset % PAGE_SIZE) != 0) { - MmUnlockAddressSpace(AddressSpace); - return STATUS_MAPPED_ALIGNMENT; + Status = STATUS_MAPPED_ALIGNMENT; + goto Exit; } if ((*ViewSize) == 0) @@ -4219,18 +4226,24 @@ MmMapViewOfSection(IN PVOID SectionObject, MmUnlockSectionSegment(Segment); if (!NT_SUCCESS(Status)) { - MmUnlockAddressSpace(AddressSpace); - return Status; + goto Exit; } } - MmUnlockAddressSpace(AddressSpace); - if (NotAtBase) Status = STATUS_IMAGE_NOT_AT_BASE; else Status = STATUS_SUCCESS; +Exit: + + MmUnlockAddressSpace(AddressSpace); + + if (IsAttached) + { + KeUnstackDetachProcess(&ApcState); + } + return Status; }

10 months, 4 weeks

[reactos] 06/09: [NTOS:MM] Fix MmFreeMemoryArea

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=601bb49c0d276447fd643… commit 601bb49c0d276447fd6435a8f70cc0f190db681a Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Mon Oct 9 22:24:01 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Tue Oct 24 21:45:27 2023 +0300 [NTOS:MM] Fix MmFreeMemoryArea - Stay attached while deleting the VAD node - Acquire the appropriate working set lock when deleting a VAD node - Both are needed for locking correctness --- ntoskrnl/mm/marea.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/ntoskrnl/mm/marea.c b/ntoskrnl/mm/marea.c index ab8bff4a18e..281226c4afc 100644 --- a/ntoskrnl/mm/marea.c +++ b/ntoskrnl/mm/marea.c @@ -300,8 +300,7 @@ MmFreeMemoryArea( PEPROCESS CurrentProcess = PsGetCurrentProcess(); PEPROCESS Process = MmGetAddressSpaceOwner(AddressSpace); - if (Process != NULL && - Process != CurrentProcess) + if ((Process != NULL) && (Process != CurrentProcess)) { KeAttachProcess(&Process->Pcb); } @@ -337,12 +336,6 @@ MmFreeMemoryArea( } } - if (Process != NULL && - Process != CurrentProcess) - { - KeDetachProcess(); - } - //if (MemoryArea->VadNode.StartingVpn < (ULONG_PTR)MmSystemRangeStart >> PAGE_SHIFT if (MemoryArea->Vad) { @@ -357,14 +350,23 @@ MmFreeMemoryArea( ASSERT(MemoryArea->VadNode.u.VadFlags.Spare != 0); if (((PMMVAD)MemoryArea->Vad)->u.VadFlags.Spare == 1) { + MiLockProcessWorkingSet(PsGetCurrentProcess(), PsGetCurrentThread()); MiRemoveNode((PMMADDRESS_NODE)&MemoryArea->VadNode, &Process->VadRoot); + MiUnlockProcessWorkingSet(PsGetCurrentProcess(), PsGetCurrentThread()); } MemoryArea->Vad = NULL; } else { + MiLockWorkingSet(PsGetCurrentThread(), &MmSystemCacheWs); MiRemoveNode((PMMADDRESS_NODE)&MemoryArea->VadNode, &MiRosKernelVadRoot); + MiUnlockWorkingSet(PsGetCurrentThread(), &MmSystemCacheWs); + } + + if ((Process != NULL) && (Process != CurrentProcess)) + { + KeDetachProcess(); } }

10 months, 4 weeks

[reactos] 05/09: Revert "[NTOS:MM/x64] Temporarily release AddressCreationLock in MmCreateVirtualMappingUnsafeEx"

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=de3958dc2b626626d017b… commit de3958dc2b626626d017be102c04eb75b3cce56d Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Tue Oct 10 02:42:11 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Tue Oct 24 21:45:27 2023 +0300 Revert "[NTOS:MM/x64] Temporarily release AddressCreationLock in MmCreateVirtualMappingUnsafeEx" This reverts commit e685b25e35c9d6dfbe2139ac71f7378b1f9e6d0f. --- ntoskrnl/mm/i386/page.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/ntoskrnl/mm/i386/page.c b/ntoskrnl/mm/i386/page.c index eeccef94346..df13cb013e2 100644 --- a/ntoskrnl/mm/i386/page.c +++ b/ntoskrnl/mm/i386/page.c @@ -637,9 +637,6 @@ MmCreateVirtualMappingUnsafeEx( PMMPTE PointerPte; MMPTE TempPte; ULONG_PTR Pte; -#ifdef _M_AMD64 - BOOLEAN LockReleased = FALSE; -#endif DPRINT("MmCreateVirtualMappingUnsafe(%p, %p, %lu, %x)\n", Process, Address, flProtect, Page); @@ -668,15 +665,6 @@ MmCreateVirtualMappingUnsafeEx( if (!MiSynchronizeSystemPde(MiAddressToPde(Address))) MiFillSystemPageDirectory(Address, PAGE_SIZE); #endif - -#ifdef _M_AMD64 - /* This is a temporary hack, because we can incur a recursive page fault when accessing the PDE */ - if (PsIdleProcess->AddressCreationLock.Owner == KeGetCurrentThread()) - { - MmUnlockAddressSpace(MmGetKernelAddressSpace()); - LockReleased = TRUE; - } -#endif } else { @@ -727,15 +715,6 @@ MmCreateVirtualMappingUnsafeEx( MiIncrementPageTableReferences(Address); MiUnlockProcessWorkingSetUnsafe(Process, PsGetCurrentThread()); } -#ifdef _M_AMD64 - else - { - if (LockReleased) - { - MmLockAddressSpace(MmGetKernelAddressSpace()); - } - } -#endif return(STATUS_SUCCESS); }

10 months, 4 weeks

[reactos] 04/09: [NTOS:MM] Fix bugs in MmAccessFault

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=a662bedab8310ac4c15e1… commit a662bedab8310ac4c15e1337e584e5f5835d14f5 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Tue Oct 10 02:10:52 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Tue Oct 24 21:45:27 2023 +0300 [NTOS:MM] Fix bugs in MmAccessFault - Acquire the appropriate working set lock when calling MmLocateMemoryAreaByAddress - Do not access MemoryArea without holding the lock (otherwise it can be pulled away under our feet) - Fix range check for paged pool --- ntoskrnl/mm/mmfault.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/ntoskrnl/mm/mmfault.c b/ntoskrnl/mm/mmfault.c index 68e31ba1281..cfd6d756d15 100644 --- a/ntoskrnl/mm/mmfault.c +++ b/ntoskrnl/mm/mmfault.c @@ -213,6 +213,7 @@ MmAccessFault(IN ULONG FaultCode, { PMEMORY_AREA MemoryArea = NULL; NTSTATUS Status; + BOOLEAN IsArm3Fault = FALSE; /* Cute little hack for ROS */ if ((ULONG_PTR)Address >= (ULONG_PTR)MmSystemRangeStart) @@ -239,19 +240,40 @@ MmAccessFault(IN ULONG FaultCode, /* Is there a ReactOS address space yet? */ if (MmGetKernelAddressSpace()) { - /* Check if this is an ARM3 memory area */ - MemoryArea = MmLocateMemoryAreaByAddress(MmGetKernelAddressSpace(), Address); - if (!(MemoryArea) && (Address <= MM_HIGHEST_USER_ADDRESS)) + if (Address > MM_HIGHEST_USER_ADDRESS) + { + /* Check if this is an ARM3 memory area */ + MiLockWorkingSetShared(PsGetCurrentThread(), &MmSystemCacheWs); + MemoryArea = MmLocateMemoryAreaByAddress(MmGetKernelAddressSpace(), Address); + + if ((MemoryArea != NULL) && (MemoryArea->Type == MEMORY_AREA_OWNED_BY_ARM3)) + { + IsArm3Fault = TRUE; + } + + MiUnlockWorkingSetShared(PsGetCurrentThread(), &MmSystemCacheWs); + } + else { /* Could this be a VAD fault from user-mode? */ + MiLockProcessWorkingSetShared(PsGetCurrentProcess(), PsGetCurrentThread()); MemoryArea = MmLocateMemoryAreaByAddress(MmGetCurrentAddressSpace(), Address); + + if ((MemoryArea != NULL) && (MemoryArea->Type == MEMORY_AREA_OWNED_BY_ARM3)) + { + IsArm3Fault = TRUE; + } + + MiUnlockProcessWorkingSetShared(PsGetCurrentProcess(), PsGetCurrentThread()); } } /* Is this an ARM3 memory area, or is there no address space yet? */ - if (((MemoryArea) && (MemoryArea->Type == MEMORY_AREA_OWNED_BY_ARM3)) || - (!(MemoryArea) && ((ULONG_PTR)Address >= (ULONG_PTR)MmPagedPoolStart)) || - (!MmGetKernelAddressSpace())) + if (IsArm3Fault || + ((MemoryArea == NULL) && + ((ULONG_PTR)Address >= (ULONG_PTR)MmPagedPoolStart) && + ((ULONG_PTR)Address < (ULONG_PTR)MmPagedPoolEnd)) || + (!MmGetKernelAddressSpace())) { /* This is an ARM3 fault */ DPRINT("ARM3 fault %p\n", MemoryArea);

10 months, 4 weeks

[reactos] 03/09: [NTOS:MM] Handle page table faults in MmArmAccessFault

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=84d0586ca40c21ae4d2b3… commit 84d0586ca40c21ae4d2b3a68137bc3e88423d463 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Tue Oct 10 22:37:21 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Tue Oct 24 21:45:27 2023 +0300 [NTOS:MM] Handle page table faults in MmArmAccessFault These faults are handled by ARM³ and we don't need to check for a memory area. They can be recursive faults (e.g. from MiDeleteSystemPageableVm), so we might be holding the WS lock already. Passing it straight to ARM³ allows to acquire the WS lock below to look up the memory area. --- ntoskrnl/mm/mmfault.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ntoskrnl/mm/mmfault.c b/ntoskrnl/mm/mmfault.c index 7937f7b5e5b..68e31ba1281 100644 --- a/ntoskrnl/mm/mmfault.c +++ b/ntoskrnl/mm/mmfault.c @@ -227,11 +227,12 @@ MmAccessFault(IN ULONG FaultCode, #endif } - /* Handle shared user page, which doesn't have a VAD / MemoryArea */ - if (PAGE_ALIGN(Address) == (PVOID)MM_SHARED_USER_DATA_VA) + /* Handle shared user page / page table, which don't have a VAD / MemoryArea */ + if ((PAGE_ALIGN(Address) == (PVOID)MM_SHARED_USER_DATA_VA) || + MI_IS_PAGE_TABLE_ADDRESS(Address)) { /* This is an ARM3 fault */ - DPRINT("ARM3 fault %p\n", MemoryArea); + DPRINT("ARM3 fault %p\n", Address); return MmArmAccessFault(FaultCode, Address, Mode, TrapInformation); }

10 months, 4 weeks

[reactos] 02/09: [NTOS:MM] Fix address space locking in MiProtectVirtualMemory

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=a8b57f0a6ba6061ce8102… commit a8b57f0a6ba6061ce810290b7d8e64ac5b73e7f3 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Mon Oct 9 22:01:56 2023 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Tue Oct 24 21:45:27 2023 +0300 [NTOS:MM] Fix address space locking in MiProtectVirtualMemory --- ntoskrnl/mm/ARM3/virtual.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/ntoskrnl/mm/ARM3/virtual.c b/ntoskrnl/mm/ARM3/virtual.c index e0804520435..ec081c8110d 100644 --- a/ntoskrnl/mm/ARM3/virtual.c +++ b/ntoskrnl/mm/ARM3/virtual.c @@ -2213,6 +2213,9 @@ MiProtectVirtualMemory(IN PEPROCESS Process, PETHREAD Thread = PsGetCurrentThread(); TABLE_SEARCH_RESULT Result; + /* We must be attached */ + ASSERT(Process == PsGetCurrentProcess()); + /* Calculate base address for the VAD */ StartingAddress = (ULONG_PTR)PAGE_ALIGN((*BaseAddress)); EndingAddress = (((ULONG_PTR)*BaseAddress + *NumberOfBytesToProtect - 1) | (PAGE_SIZE - 1)); @@ -2225,11 +2228,22 @@ MiProtectVirtualMemory(IN PEPROCESS Process, return STATUS_INVALID_PAGE_PROTECTION; } + /* Lock the address space and make sure the process isn't already dead */ + AddressSpace = MmGetCurrentAddressSpace(); + MmLockAddressSpace(AddressSpace); + if (Process->VmDeleted) + { + DPRINT1("Process is dying\n"); + Status = STATUS_PROCESS_IS_TERMINATING; + goto FailPath; + } + /* Check for ROS specific memory area */ MemoryArea = MmLocateMemoryAreaByAddress(&Process->Vm, *BaseAddress); if ((MemoryArea) && (MemoryArea->Type != MEMORY_AREA_OWNED_BY_ARM3)) { /* Evil hack */ + MmUnlockAddressSpace(AddressSpace); return MiRosProtectVirtualMemory(Process, BaseAddress, NumberOfBytesToProtect, @@ -2237,16 +2251,6 @@ MiProtectVirtualMemory(IN PEPROCESS Process, OldAccessProtection); } - /* Lock the address space and make sure the process isn't already dead */ - AddressSpace = MmGetCurrentAddressSpace(); - MmLockAddressSpace(AddressSpace); - if (Process->VmDeleted) - { - DPRINT1("Process is dying\n"); - Status = STATUS_PROCESS_IS_TERMINATING; - goto FailPath; - } - /* Get the VAD for this address range, and make sure it exists */ Result = MiCheckForConflictingNode(StartingAddress >> PAGE_SHIFT, EndingAddress >> PAGE_SHIFT,

10 months, 4 weeks

← Newer
1
2
3
4
5
6
7
...
14
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 October 2023