From 543797fee3b88b652e167bb75a100212333b6a45 Mon Sep 17 00:00:00 2001 From: Timo Kreuzer Date: Tue, 26 Nov 2013 21:38:02 +0000 Subject: [PATCH] [NTOSKRNL] - Remove the BoundaryAddressMultiple parameter from MmCreateMemoryArea (wasn't used) and give it instead a Granularity parameter - Use the Granularity parameter in MmMapViewOfSegment to make sure that full sections are allocated on a MM_ALLOCATION_GRANULARITY aligned address. - Check for overflow and unaligned image base in MmMapViewOfSection when mapping image sections - Return proper status code on failure svn path=/trunk/; revision=61108 --- reactos/ntoskrnl/cache/section/data.c | 5 +--- reactos/ntoskrnl/cc/view.c | 8 ++---- reactos/ntoskrnl/include/internal/mm.h | 2 +- reactos/ntoskrnl/mm/ARM3/procsup.c | 4 +-- reactos/ntoskrnl/mm/ARM3/vadnode.c | 17 ++++++----- reactos/ntoskrnl/mm/arm/stubs.c | 8 ++---- reactos/ntoskrnl/mm/i386/pagepae.c | 6 ++-- reactos/ntoskrnl/mm/marea.c | 11 +------- reactos/ntoskrnl/mm/mminit.c | 28 +++++++++--------- reactos/ntoskrnl/mm/section.c | 39 ++++++++++++++++++-------- 10 files changed, 60 insertions(+), 68 deletions(-) diff --git a/reactos/ntoskrnl/cache/section/data.c b/reactos/ntoskrnl/cache/section/data.c index b496b57acea..0156ec69306 100644 --- a/reactos/ntoskrnl/cache/section/data.c +++ b/reactos/ntoskrnl/cache/section/data.c @@ -627,9 +627,6 @@ _MiMapViewOfSegment(PMMSUPPORT AddressSpace, { PMEMORY_AREA MArea; NTSTATUS Status; - PHYSICAL_ADDRESS BoundaryAddressMultiple; - - BoundaryAddressMultiple.QuadPart = 0; Status = MmCreateMemoryArea(AddressSpace, MEMORY_AREA_CACHE, @@ -639,7 +636,7 @@ _MiMapViewOfSegment(PMMSUPPORT AddressSpace, &MArea, FALSE, AllocationType, - BoundaryAddressMultiple); + PAGE_SIZE); if (!NT_SUCCESS(Status)) { diff --git a/reactos/ntoskrnl/cc/view.c b/reactos/ntoskrnl/cc/view.c index 40e8f44acd6..46c14a504fa 100644 --- a/reactos/ntoskrnl/cc/view.c +++ b/reactos/ntoskrnl/cc/view.c @@ -611,13 +611,11 @@ CcRosCreateCacheSegment ( #ifdef CACHE_BITMAP ULONG StartingOffset; #endif - PHYSICAL_ADDRESS BoundaryAddressMultiple; ASSERT(Bcb); DPRINT("CcRosCreateCacheSegment()\n"); - BoundaryAddressMultiple.QuadPart = 0; if (FileOffset >= Bcb->FileSize.u.LowPart) { CacheSeg = NULL; @@ -742,7 +740,7 @@ CcRosCreateCacheSegment ( (PMEMORY_AREA*)¤t->MemoryArea, FALSE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); MmUnlockAddressSpace(MmGetKernelAddressSpace()); if (!NT_SUCCESS(Status)) { @@ -1370,12 +1368,10 @@ CcInitView ( #ifdef CACHE_BITMAP PMEMORY_AREA marea; PVOID Buffer; - PHYSICAL_ADDRESS BoundaryAddressMultiple; #endif DPRINT("CcInitView()\n"); #ifdef CACHE_BITMAP - BoundaryAddressMultiple.QuadPart = 0; CiCacheSegMappingRegionHint = 0; CiCacheSegMappingRegionBase = NULL; @@ -1389,7 +1385,7 @@ CcInitView ( &marea, FALSE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); MmUnlockAddressSpace(MmGetKernelAddressSpace()); if (!NT_SUCCESS(Status)) { diff --git a/reactos/ntoskrnl/include/internal/mm.h b/reactos/ntoskrnl/include/internal/mm.h index 11b428b420b..40019a925e9 100644 --- a/reactos/ntoskrnl/include/internal/mm.h +++ b/reactos/ntoskrnl/include/internal/mm.h @@ -512,7 +512,7 @@ MmCreateMemoryArea( PMEMORY_AREA *Result, BOOLEAN FixedAddress, ULONG AllocationFlags, - PHYSICAL_ADDRESS BoundaryAddressMultiple OPTIONAL + ULONG AllocationGranularity ); PMEMORY_AREA diff --git a/reactos/ntoskrnl/mm/ARM3/procsup.c b/reactos/ntoskrnl/mm/ARM3/procsup.c index 8bb17d02e6f..2e90165b049 100644 --- a/reactos/ntoskrnl/mm/ARM3/procsup.c +++ b/reactos/ntoskrnl/mm/ARM3/procsup.c @@ -30,9 +30,7 @@ MiRosTakeOverSharedUserPage(IN PEPROCESS Process) { NTSTATUS Status; PMEMORY_AREA MemoryArea; - PHYSICAL_ADDRESS BoundaryAddressMultiple; PVOID AllocatedBase = (PVOID)MM_SHARED_USER_DATA_VA; - BoundaryAddressMultiple.QuadPart = 0; Status = MmCreateMemoryArea(&Process->Vm, MEMORY_AREA_OWNED_BY_ARM3, @@ -42,7 +40,7 @@ MiRosTakeOverSharedUserPage(IN PEPROCESS Process) &MemoryArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(NT_SUCCESS(Status)); } diff --git a/reactos/ntoskrnl/mm/ARM3/vadnode.c b/reactos/ntoskrnl/mm/ARM3/vadnode.c index c6608c55c57..402b4c69f48 100644 --- a/reactos/ntoskrnl/mm/ARM3/vadnode.c +++ b/reactos/ntoskrnl/mm/ARM3/vadnode.c @@ -24,19 +24,19 @@ CHAR MmReadWrite[32] = { - MM_NO_ACCESS_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, + MM_NO_ACCESS_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, - MM_NO_ACCESS_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, + MM_NO_ACCESS_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, - MM_NO_ACCESS_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, + MM_NO_ACCESS_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, - MM_NO_ACCESS_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, + MM_NO_ACCESS_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_ONLY_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, MM_READ_WRITE_ALLOWED, }; @@ -129,11 +129,10 @@ MiInsertNode(IN PMM_AVL_TABLE Table, { NTSTATUS Status; PMEMORY_AREA MemoryArea; - PHYSICAL_ADDRESS BoundaryAddressMultiple; SIZE_T Size; PEPROCESS Process = CONTAINING_RECORD(Table, EPROCESS, VadRoot); PVOID AllocatedBase = (PVOID)(Vad->StartingVpn << PAGE_SHIFT); - BoundaryAddressMultiple.QuadPart = 0; + Size = ((Vad->EndingVpn + 1) - Vad->StartingVpn) << PAGE_SHIFT; Status = MmCreateMemoryArea(&Process->Vm, MEMORY_AREA_OWNED_BY_ARM3, @@ -143,7 +142,7 @@ MiInsertNode(IN PMM_AVL_TABLE Table, &MemoryArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(NT_SUCCESS(Status)); /* Check if this is VM VAD */ @@ -635,7 +634,7 @@ MiCheckSecuredVad(IN PMMVAD Vad, /* This is allowed */ ProtectionMask = 0; } - + /* ARM3 doesn't support this yet */ ASSERT(Vad->u2.VadFlags2.MultipleSecured == 0); @@ -655,7 +654,7 @@ MiCheckSecuredVad(IN PMMVAD Vad, /* ARM3 doesn't have read-only VADs yet */ ASSERT(Vad->u2.VadFlags2.ReadOnly == 0); - + /* Check if read-write protections are allowed */ if (MmReadWrite[ProtectionMask] < MM_READ_WRITE_ALLOWED) { diff --git a/reactos/ntoskrnl/mm/arm/stubs.c b/reactos/ntoskrnl/mm/arm/stubs.c index c7916d6c107..2d43dfc27db 100644 --- a/reactos/ntoskrnl/mm/arm/stubs.c +++ b/reactos/ntoskrnl/mm/arm/stubs.c @@ -779,14 +779,12 @@ NTAPI MiInitPageDirectoryMap(VOID) { MEMORY_AREA* MemoryArea = NULL; - PHYSICAL_ADDRESS BoundaryAddressMultiple; PVOID BaseAddress; NTSTATUS Status; // // Create memory area for the PTE area // - BoundaryAddressMultiple.QuadPart = 0; BaseAddress = (PVOID)PTE_BASE; Status = MmCreateMemoryArea(MmGetKernelAddressSpace(), MEMORY_AREA_OWNED_BY_ARM3, @@ -796,7 +794,7 @@ MiInitPageDirectoryMap(VOID) &MemoryArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(NT_SUCCESS(Status)); // @@ -811,7 +809,7 @@ MiInitPageDirectoryMap(VOID) &MemoryArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(NT_SUCCESS(Status)); // @@ -826,7 +824,7 @@ MiInitPageDirectoryMap(VOID) &MemoryArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(NT_SUCCESS(Status)); } diff --git a/reactos/ntoskrnl/mm/i386/pagepae.c b/reactos/ntoskrnl/mm/i386/pagepae.c index 0375e755ac6..f7a4c428e7f 100644 --- a/reactos/ntoskrnl/mm/i386/pagepae.c +++ b/reactos/ntoskrnl/mm/i386/pagepae.c @@ -2311,13 +2311,11 @@ MiInitPageDirectoryMap(VOID) { MEMORY_AREA* kernel_map_desc = NULL; MEMORY_AREA* hyperspace_desc = NULL; - PHYSICAL_ADDRESS BoundaryAddressMultiple; PVOID BaseAddress; NTSTATUS Status; DPRINT("MiInitPageDirectoryMap()\n"); - BoundaryAddressMultiple.QuadPart = 0; BaseAddress = (PVOID)PAGETABLE_MAP; Status = MmCreateMemoryArea(MmGetKernelAddressSpace(), MEMORY_AREA_SYSTEM, @@ -2327,7 +2325,7 @@ MiInitPageDirectoryMap(VOID) &kernel_map_desc, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); if (!NT_SUCCESS(Status)) { ASSERT(FALSE); @@ -2341,7 +2339,7 @@ MiInitPageDirectoryMap(VOID) &hyperspace_desc, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); if (!NT_SUCCESS(Status)) { ASSERT(FALSE); diff --git a/reactos/ntoskrnl/mm/marea.c b/reactos/ntoskrnl/mm/marea.c index 0423c055a59..0109d19d4c0 100644 --- a/reactos/ntoskrnl/mm/marea.c +++ b/reactos/ntoskrnl/mm/marea.c @@ -984,10 +984,8 @@ MmCreateMemoryArea(PMMSUPPORT AddressSpace, PMEMORY_AREA *Result, BOOLEAN FixedAddress, ULONG AllocationFlags, - PHYSICAL_ADDRESS BoundaryAddressMultiple) + ULONG Granularity) { - PVOID EndAddress; - ULONG Granularity; ULONG_PTR tmpLength; PMEMORY_AREA MemoryArea; @@ -997,7 +995,6 @@ MmCreateMemoryArea(PMMSUPPORT AddressSpace, Type, BaseAddress, *BaseAddress, Length, AllocationFlags, FixedAddress, Result); - Granularity = PAGE_SIZE; if ((*BaseAddress) == 0 && !FixedAddress) { tmpLength = (ULONG_PTR)MM_ROUND_UP(Length, Granularity); @@ -1030,12 +1027,6 @@ MmCreateMemoryArea(PMMSUPPORT AddressSpace, return STATUS_ACCESS_VIOLATION; } - if (BoundaryAddressMultiple.QuadPart != 0) - { - EndAddress = ((char*)(*BaseAddress)) + tmpLength-1; - ASSERT(((ULONG_PTR)*BaseAddress/BoundaryAddressMultiple.QuadPart) == ((DWORD_PTR)EndAddress/BoundaryAddressMultiple.QuadPart)); - } - if (MmLocateMemoryAreaByRegion(AddressSpace, *BaseAddress, tmpLength) != NULL) diff --git a/reactos/ntoskrnl/mm/mminit.c b/reactos/ntoskrnl/mm/mminit.c index 98283ffef96..910f3973dae 100644 --- a/reactos/ntoskrnl/mm/mminit.c +++ b/reactos/ntoskrnl/mm/mminit.c @@ -44,10 +44,8 @@ NTAPI MiInitSystemMemoryAreas() { PVOID BaseAddress; - PHYSICAL_ADDRESS BoundaryAddressMultiple; PMEMORY_AREA MArea; NTSTATUS Status; - BoundaryAddressMultiple.QuadPart = 0; // // Create the memory area to define the loader mappings @@ -61,7 +59,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); // @@ -76,7 +74,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); // @@ -91,7 +89,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); // @@ -106,7 +104,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); // @@ -121,7 +119,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); // @@ -136,7 +134,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); // @@ -151,7 +149,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); // @@ -167,7 +165,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); // @@ -182,7 +180,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); #ifndef _M_AMD64 // @@ -197,7 +195,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); #endif // @@ -212,7 +210,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); // @@ -227,7 +225,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); #if defined(_X86_) @@ -243,7 +241,7 @@ MiInitSystemMemoryAreas() &MArea, TRUE, 0, - BoundaryAddressMultiple); + PAGE_SIZE); ASSERT(Status == STATUS_SUCCESS); #endif } diff --git a/reactos/ntoskrnl/mm/section.c b/reactos/ntoskrnl/mm/section.c index fc029246fdd..243a837bd76 100644 --- a/reactos/ntoskrnl/mm/section.c +++ b/reactos/ntoskrnl/mm/section.c @@ -200,7 +200,7 @@ NTSTATUS NTAPI PeFmtCreateSection(IN CONST VOID * FileHeader, ULONG cbHeadersSize = 0; ULONG nSectionAlignment; ULONG nFileAlignment; - ULONG ImageBase; + ULONG_PTR ImageBase; const IMAGE_DOS_HEADER * pidhDosHeader; const IMAGE_NT_HEADERS32 * pinhNtHeader; const IMAGE_OPTIONAL_HEADER32 * piohOptHeader; @@ -457,7 +457,7 @@ l_ReadHeaderFromFile: break; } - +#ifdef _WIN64 /* PE64 */ case IMAGE_NT_OPTIONAL_HDR64_MAGIC: { @@ -535,6 +535,7 @@ l_ReadHeaderFromFile: break; } +#endif // _WIN64 } /* [1], section 3.4.2 */ @@ -2816,7 +2817,8 @@ MmCreatePageFileSection(PROS_SECTION_OBJECT *SectionObject, if (UMaximumSize == NULL) { - return(STATUS_UNSUCCESSFUL); + DPRINT1("MmCreatePageFileSection: (UMaximumSize == NULL)\n"); + return(STATUS_INVALID_PARAMETER); } MaximumSize = *UMaximumSize; @@ -2834,6 +2836,7 @@ MmCreatePageFileSection(PROS_SECTION_OBJECT *SectionObject, (PVOID*)(PVOID)&Section); if (!NT_SUCCESS(Status)) { + DPRINT1("MmCreatePageFileSection: failed to create object (0x%lx)\n", Status); return(Status); } @@ -3901,7 +3904,7 @@ MmMapViewOfSegment(PMMSUPPORT AddressSpace, { PMEMORY_AREA MArea; NTSTATUS Status; - PHYSICAL_ADDRESS BoundaryAddressMultiple; + ULONG Granularity; if (Segment->WriteCopy) { @@ -3920,7 +3923,10 @@ MmMapViewOfSegment(PMMSUPPORT AddressSpace, Protect = PAGE_EXECUTE_READWRITE; } - BoundaryAddressMultiple.QuadPart = 0; + if (*BaseAddress == NULL) + Granularity = MM_ALLOCATION_GRANULARITY; + else + Granularity = PAGE_SIZE; #ifdef NEWCC if (Segment->Flags & MM_DATAFILE_SEGMENT) { @@ -3938,7 +3944,7 @@ MmMapViewOfSegment(PMMSUPPORT AddressSpace, &MArea, FALSE, AllocationType, - BoundaryAddressMultiple); + Granularity); if (!NT_SUCCESS(Status)) { DPRINT1("Mapping between 0x%p and 0x%p failed (%X).\n", @@ -4471,9 +4477,19 @@ MmMapViewOfSection(IN PVOID SectionObject, ImageSectionObject->ImageInformation.ImageFileSize = (ULONG)ImageSize; /* Check for an illegal base address */ - if ((ImageBase + ImageSize) > (ULONG_PTR)MmHighestUserAddress) + if (((ImageBase + ImageSize) > (ULONG_PTR)MmHighestUserAddress) || + ((ImageBase + ImageSize) < ImageSize)) { - ImageBase = PAGE_ROUND_DOWN((ULONG_PTR)MmHighestUserAddress - ImageSize); + NT_ASSERT(*BaseAddress == NULL); + ImageBase = ALIGN_DOWN_BY((ULONG_PTR)MmHighestUserAddress - ImageSize, + MM_VIRTMEM_GRANULARITY); + NotAtBase = TRUE; + } + else if (ImageBase != ALIGN_DOWN_BY(ImageBase, MM_VIRTMEM_GRANULARITY)) + { + NT_ASSERT(*BaseAddress == NULL); + ImageBase = ALIGN_DOWN_BY(ImageBase, MM_VIRTMEM_GRANULARITY); + NotAtBase = TRUE; } /* Check there is enough space to map the section at that point. */ @@ -4484,14 +4500,14 @@ MmMapViewOfSection(IN PVOID SectionObject, if ((*BaseAddress) != NULL) { MmUnlockAddressSpace(AddressSpace); - return(STATUS_UNSUCCESSFUL); + return(STATUS_CONFLICTING_ADDRESSES); } /* Otherwise find a gap to map the image. */ - ImageBase = (ULONG_PTR)MmFindGap(AddressSpace, PAGE_ROUND_UP(ImageSize), PAGE_SIZE, FALSE); + ImageBase = (ULONG_PTR)MmFindGap(AddressSpace, PAGE_ROUND_UP(ImageSize), MM_VIRTMEM_GRANULARITY, FALSE); if (ImageBase == 0) { MmUnlockAddressSpace(AddressSpace); - return(STATUS_UNSUCCESSFUL); + return(STATUS_CONFLICTING_ADDRESSES); } /* Remember that we loaded image at a different base address */ NotAtBase = TRUE; @@ -4599,6 +4615,7 @@ MmMapViewOfSection(IN PVOID SectionObject, } MmUnlockAddressSpace(AddressSpace); + NT_ASSERT(*BaseAddress == ALIGN_DOWN_POINTER_BY(*BaseAddress, MM_VIRTMEM_GRANULARITY)); if (NotAtBase) Status = STATUS_IMAGE_NOT_AT_BASE;