https://git.reactos.org/?p=reactos.git;a=commitdiff;h=918e33197024163fd6e27…
commit 918e33197024163fd6e2781eeed3bb6b7be82270
Author: Timo Kreuzer <timo.kreuzer(a)reactos.org>
AuthorDate: Sun Apr 9 19:10:38 2023 +0300
Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org>
CommitDate: Sat Jul 29 14:00:44 2023 +0300
[NTOS:Mm] Fix race condition in _MmSetPageEntrySectionSegment
The function updates the entry in the section page table and updates the section association rmaps for it. In the page-in path, when the new section association is set before the entry is updated, a concurrent attempt to unmap the page would find an inconsistent entry, where there is an rmap, but the section page table entry is still an MM_WAIT_ENTRY.
---
ntoskrnl/cache/section/sptab.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/ntoskrnl/cache/section/sptab.c b/ntoskrnl/cache/section/sptab.c
index 5149ea3b76e..83f7c5f542a 100644
--- a/ntoskrnl/cache/section/sptab.c
+++ b/ntoskrnl/cache/section/sptab.c
@@ -223,8 +223,17 @@ _MmSetPageEntrySectionSegment(PMM_SECTION_SEGMENT Segment,
if (PFN_FROM_SSE(Entry) != PFN_FROM_SSE(OldEntry))
{
MmDeleteSectionAssociation(PFN_FROM_SSE(OldEntry));
+
+ /* This has to be done before setting the new section association
+ to prevent a race condition with the paging out path */
+ PageTable->PageEntries[PageIndex] = Entry;
+
MmSetSectionAssociation(PFN_FROM_SSE(Entry), Segment, Offset);
}
+ else
+ {
+ PageTable->PageEntries[PageIndex] = Entry;
+ }
}
else
{
@@ -232,6 +241,7 @@ _MmSetPageEntrySectionSegment(PMM_SECTION_SEGMENT Segment,
* We're switching to a valid entry from an invalid one.
* Add the Rmap and take a ref on the segment.
*/
+ PageTable->PageEntries[PageIndex] = Entry;
MmSetSectionAssociation(PFN_FROM_SSE(Entry), Segment, Offset);
if (Offset->QuadPart >= (Segment->LastPage << PAGE_SHIFT))
@@ -242,6 +252,7 @@ _MmSetPageEntrySectionSegment(PMM_SECTION_SEGMENT Segment,
{
/* We're switching to an invalid entry from a valid one */
MmDeleteSectionAssociation(PFN_FROM_SSE(OldEntry));
+ PageTable->PageEntries[PageIndex] = Entry;
if (Offset->QuadPart == ((Segment->LastPage - 1ULL) << PAGE_SHIFT))
{
@@ -256,8 +267,11 @@ _MmSetPageEntrySectionSegment(PMM_SECTION_SEGMENT Segment,
}
}
}
+ else
+ {
+ PageTable->PageEntries[PageIndex] = Entry;
+ }
- PageTable->PageEntries[PageIndex] = Entry;
return STATUS_SUCCESS;
}
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=477792856e42be09b12fd…
commit 477792856e42be09b12fd3e3ce557657c6cc63c2
Author: Timo Kreuzer <timo.kreuzer(a)reactos.org>
AuthorDate: Sat Apr 1 19:18:45 2023 +0300
Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org>
CommitDate: Sat Jul 29 14:00:44 2023 +0300
[NTOS:Mm] Replace YieldProcessor() with KeDelayExecutionThread()
These are used in the paging path, when the page is currently in the process of being read from or written to the disk. While YieldProcessor() provides the chance to switch context to the other paging thread, it only does so, once the current thread's quantum has expired. On a single CPU system this effectively leads to busy waiting for the rest of the quantum. On SMP systems this could succeed earlier, thus reducing latency, but it would still contribute to high CPU usage, while wait [...]
Using KeDelayExecutionThread() will instantly allow another thread to run, providing enough time to complete the IO operation.
---
ntoskrnl/mm/section.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/ntoskrnl/mm/section.c b/ntoskrnl/mm/section.c
index 49c0e0e8f4f..33584a02230 100644
--- a/ntoskrnl/mm/section.c
+++ b/ntoskrnl/mm/section.c
@@ -1477,7 +1477,7 @@ MmAlterViewAttributes(PMMSUPPORT AddressSpace,
break;
MmUnlockSectionSegment(Segment);
MmUnlockAddressSpace(AddressSpace);
- YieldProcessor();
+ KeDelayExecutionThread(KernelMode, FALSE, &TinyTime);
MmLockAddressSpace(AddressSpace);
MmLockSectionSegment(Segment);
}
@@ -1608,7 +1608,7 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
if (SwapEntry == MM_WAIT_ENTRY)
{
MmUnlockAddressSpace(AddressSpace);
- YieldProcessor();
+ KeDelayExecutionThread(KernelMode, FALSE, &TinyTime);
MmLockAddressSpace(AddressSpace);
return STATUS_MM_RESTART_OPERATION;
}
@@ -1789,7 +1789,7 @@ MmNotPresentFaultSectionView(PMMSUPPORT AddressSpace,
{
MmUnlockSectionSegment(Segment);
MmUnlockAddressSpace(AddressSpace);
- YieldProcessor();
+ KeDelayExecutionThread(KernelMode, FALSE, &TinyTime);
MmLockAddressSpace(AddressSpace);
return STATUS_MM_RESTART_OPERATION;
}
@@ -3442,7 +3442,7 @@ MmFreeSectionPage(PVOID Context, MEMORY_AREA* MemoryArea, PVOID Address,
MmUnlockSectionSegment(Segment);
MmUnlockAddressSpace(AddressSpace);
- YieldProcessor();
+ KeDelayExecutionThread(KernelMode, FALSE, &TinyTime);
MmLockAddressSpace(AddressSpace);
MmLockSectionSegment(Segment);
@@ -5202,7 +5202,7 @@ MmMakePagesDirty(
{
MmUnlockSectionSegment(Segment);
MmUnlockAddressSpace(AddressSpace);
- YieldProcessor();
+ KeDelayExecutionThread(KernelMode, FALSE, &TinyTime);
MmLockAddressSpace(AddressSpace);
MmLockSectionSegment(Segment);
Entry = MmGetPageEntrySectionSegment(Segment, &SegmentOffset);