Ros-diffs June 2021

ros-diffs@reactos.org

19 participants
228 discussions

[reactos] 04/06: [HAL:APIC] Remove obsolete hacks
by Timo Kreuzer 17 Jun '21

17 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=235d7472ba424351d2b1e… commit 235d7472ba424351d2b1ea17a0ad7281a1f16b71 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Thu Jun 3 19:29:25 2021 +0200 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Jun 17 23:27:44 2021 +0200 [HAL:APIC] Remove obsolete hacks These hacks were used to work around broken VBox. VBox works fine without the hacks these days. --- hal/halx86/apic/apic.c | 11 +---------- hal/halx86/apic/apictrap.S | 28 ---------------------------- 2 files changed, 1 insertion(+), 38 deletions(-) diff --git a/hal/halx86/apic/apic.c b/hal/halx86/apic/apic.c index 65a6d2e4d20..25bd4e4b4c3 100644 --- a/hal/halx86/apic/apic.c +++ b/hal/halx86/apic/apic.c @@ -16,8 +16,6 @@ #define NDEBUG #include <debug.h> -void __cdecl HackEoi(void); - #ifndef _M_AMD64 //#define APIC_LAZY_IRQL //FIXME: Disabled due to bug. #endif @@ -155,8 +153,7 @@ FORCEINLINE VOID ApicSendEOI(void) { - //ApicWrite(APIC_EOI, 0); - HackEoi(); + ApicWrite(APIC_EOI, 0); } FORCEINLINE @@ -174,15 +171,9 @@ ApicGetCurrentIrql(VOID) #ifdef _M_AMD64 return (KIRQL)__readcr8(); #elif defined(APIC_LAZY_IRQL) - // HACK: some magic to Sync VBox's APIC registers - ApicRead(APIC_VER); - /* Return the field in the PCR */ return (KIRQL)__readfsbyte(FIELD_OFFSET(KPCR, Irql)); #else - // HACK: some magic to Sync VBox's APIC registers - ApicRead(APIC_VER); - /* Read the TPR and convert it to an IRQL */ return TprToIrql(ApicRead(APIC_TPR)); #endif diff --git a/hal/halx86/apic/apictrap.S b/hal/halx86/apic/apictrap.S index 52f1dabec0f..381f5492023 100644 --- a/hal/halx86/apic/apictrap.S +++ b/hal/halx86/apic/apictrap.S @@ -22,23 +22,6 @@ PUBLIC ApicSpuriousService ApicSpuriousService: iretq -PUBLIC HackEoi -HackEoi: - xor rax, rax - mov ax, ss - push rax - push rsp - pushfq - mov ax, cs - push rax - lea rax, HackEoiReturn[rip] - push rax - mov dword ptr [HEX(0FFFFFFFFFFFE00B0)], 0 - iretq -HackEoiReturn: - add rsp, 8 // esp was changed by the iret to the pushed value - ret - #else #include <ks386.inc> #include <internal/i386/asmmacro.S> @@ -54,17 +37,6 @@ PUBLIC _ApicSpuriousService _ApicSpuriousService: iret -// VBox APIC needs an iret more or less directly following the EOI -PUBLIC _HackEoi -_HackEoi: - pushfd - push cs - push offset OnlyOnePersonKnowsHowToHackAroundVBoxBugsAndThatIsNotYou // !! - mov dword ptr ds:[HEX(0FFFE00B0)], 0 - iretd -OnlyOnePersonKnowsHowToHackAroundVBoxBugsAndThatIsNotYou: - ret - #endif

1 0

[reactos] 03/06: [HAL:APIC] Code fixes
by Timo Kreuzer 17 Jun '21

17 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b7a149fcec7f3626e671f… commit b7a149fcec7f3626e671f1da658589f8c219503f Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Sat Jun 5 15:39:22 2021 +0200 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Jun 17 23:27:44 2021 +0200 [HAL:APIC] Code fixes * Fix some broken code * Add some ASSERTs * Use ApicWriteIORedirectionEntry where appropriate * Use KeQueryInterruptHandler/KeRegisterInterruptHandler to save/restore the old handler instead of saving the KIDTENTRY * Move HalpProfileInterruptHandler to apictimer.c and implement it * Use READ/WRITE_REGISTER macros * Add some symbolic names --- hal/halx86/apic/apic.c | 40 +++++++++++++++++++++++----------------- hal/halx86/apic/apicp.h | 8 ++++++-- hal/halx86/apic/apictimer.c | 7 +++++++ hal/halx86/apic/rtctimer.c | 7 ------- hal/halx86/apic/tsc.c | 10 ++++------ 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/hal/halx86/apic/apic.c b/hal/halx86/apic/apic.c index 2811cc45535..65a6d2e4d20 100644 --- a/hal/halx86/apic/apic.c +++ b/hal/halx86/apic/apic.c @@ -94,8 +94,9 @@ ULONG IOApicRead(UCHAR Register) { /* Select the register, then do the read */ - *(volatile UCHAR *)(IOAPIC_BASE + IOAPIC_IOREGSEL) = Register; - return *(volatile ULONG *)(IOAPIC_BASE + IOAPIC_IOWIN); + ASSERT(Register <= 0x3F); + WRITE_REGISTER_ULONG((PULONG)(IOAPIC_BASE + IOAPIC_IOREGSEL), Register); + return READ_REGISTER_ULONG((PULONG)(IOAPIC_BASE + IOAPIC_IOWIN)); } FORCEINLINE @@ -103,8 +104,9 @@ VOID IOApicWrite(UCHAR Register, ULONG Value) { /* Select the register, then do the write */ - *(volatile UCHAR *)(IOAPIC_BASE + IOAPIC_IOREGSEL) = Register; - *(volatile ULONG *)(IOAPIC_BASE + IOAPIC_IOWIN) = Value; + ASSERT(Register <= 0x3F); + WRITE_REGISTER_ULONG((PULONG)(IOAPIC_BASE + IOAPIC_IOREGSEL), Register); + WRITE_REGISTER_ULONG((PULONG)(IOAPIC_BASE + IOAPIC_IOWIN), Value); } FORCEINLINE @@ -113,6 +115,7 @@ ApicWriteIORedirectionEntry( UCHAR Index, IOAPIC_REDIRECTION_REGISTER ReDirReg) { + ASSERT(Index < APIC_MAX_IRQ); IOApicWrite(IOAPIC_REDTBL + 2 * Index, ReDirReg.Long0); IOApicWrite(IOAPIC_REDTBL + 2 * Index + 1, ReDirReg.Long1); } @@ -124,6 +127,7 @@ ApicReadIORedirectionEntry( { IOAPIC_REDIRECTION_REGISTER ReDirReg; + ASSERT(Index < APIC_MAX_IRQ); ReDirReg.Long0 = IOApicRead(IOAPIC_REDTBL + 2 * Index); ReDirReg.Long1 = IOApicRead(IOAPIC_REDTBL + 2 * Index + 1); @@ -344,7 +348,7 @@ HalpAllocateSystemInterrupt( Vector = IrqlToTpr(Irql) & 0xF0; /* Find an empty vector */ - while (HalpVectorToIndex[Vector] != 0xFF) + while (HalpVectorToIndex[Vector] != APIC_FREE_VECTOR) { Vector++; @@ -372,8 +376,7 @@ HalpAllocateSystemInterrupt( ReDirReg.Destination = 0; /* Initialize entry */ - IOApicWrite(IOAPIC_REDTBL + 2 * Irq, ReDirReg.Long0); - IOApicWrite(IOAPIC_REDTBL + 2 * Irq + 1, ReDirReg.Long1); + ApicWriteIORedirectionEntry(Irq, ReDirReg); return Vector; } @@ -410,17 +413,16 @@ ApicInitializeIOApic(VOID) ReDirReg.Destination = 0; /* Loop all table entries */ - for (Index = 0; Index < 24; Index++) + for (Index = 0; Index < APIC_MAX_IRQ; Index++) { /* Initialize entry */ - IOApicWrite(IOAPIC_REDTBL + 2 * Index, ReDirReg.Long0); - IOApicWrite(IOAPIC_REDTBL + 2 * Index + 1, ReDirReg.Long1); + ApicWriteIORedirectionEntry(Index, ReDirReg); } /* Init the vactor to index table */ for (Vector = 0; Vector <= 255; Vector++) { - HalpVectorToIndex[Vector] = 0xFF; + HalpVectorToIndex[Vector] = APIC_FREE_VECTOR; } // HACK: Allocate all IRQs, should rather do that on demand @@ -437,7 +439,7 @@ ApicInitializeIOApic(VOID) ReDirReg.TriggerMode = APIC_TGM_Edge; ReDirReg.Mask = 0; ReDirReg.Destination = ApicRead(APIC_ID); - IOApicWrite(IOAPIC_REDTBL + 2 * APIC_CLOCK_INDEX, ReDirReg.Long0); + ApicWriteIORedirectionEntry(APIC_CLOCK_INDEX, ReDirReg); } VOID @@ -457,9 +459,10 @@ HalpInitializePICs(IN BOOLEAN EnableInterrupts) ApicInitializeIOApic(); /* Manually reserve some vectors */ + HalpVectorToIndex[APC_VECTOR] = APIC_RESERVED_VECTOR; + HalpVectorToIndex[DISPATCH_VECTOR] = APIC_RESERVED_VECTOR; HalpVectorToIndex[APIC_CLOCK_VECTOR] = 8; - HalpVectorToIndex[APC_VECTOR] = 99; - HalpVectorToIndex[DISPATCH_VECTOR] = 99; + HalpVectorToIndex[APIC_SPURIOUS_VECTOR] = APIC_RESERVED_VECTOR; /* Set interrupt handlers in the IDT */ KeRegisterInterruptHandler(APIC_CLOCK_VECTOR, HalpClockInterrupt); @@ -669,7 +672,7 @@ HalDisableSystemInterrupt( ReDirReg.Mask = 1; /* Write back lower dword */ - IOApicWrite(IOAPIC_REDTBL + 2 * Irql, ReDirReg.Long0); + IOApicWrite(IOAPIC_REDTBL + 2 * Index, ReDirReg.Long0); } #ifndef _M_AMD64 @@ -704,8 +707,8 @@ HalBeginSystemInterrupt( /* Get the irq for this vector */ Index = HalpVectorToIndex[Vector]; - /* Check if its valid */ - if (Index != 0xff) + /* Check if it's valid */ + if (Index < APIC_MAX_IRQ) { /* Read the I/O redirection entry */ RedirReg = ApicReadIORedirectionEntry(Index); @@ -715,6 +718,9 @@ HalBeginSystemInterrupt( } else { + /* This should be a reserved vector! */ + ASSERT(Index == APIC_RESERVED_VECTOR); + /* Re-request the interrupt to be handled later */ ApicRequestInterrupt(Vector, APIC_TGM_Edge); } diff --git a/hal/halx86/apic/apicp.h b/hal/halx86/apic/apicp.h index ee8eec3d665..d3e1ece84aa 100644 --- a/hal/halx86/apic/apicp.h +++ b/hal/halx86/apic/apicp.h @@ -25,6 +25,10 @@ #define TprToIrql(Tpr) (HalVectorToIRQL[Tpr >> 4]) #endif +#define APIC_MAX_IRQ 24 +#define APIC_FREE_VECTOR 0xFF +#define APIC_RESERVED_VECTOR 0xFE + /* The IMCR is supported by two read/writable or write-only I/O ports, 22h and 23h, which receive address and data respectively. To access the IMCR, write a value of 70h to I/O port 22h, which selects the IMCR. @@ -284,14 +288,14 @@ FORCEINLINE ULONG ApicRead(ULONG Offset) { - return *(volatile ULONG *)(APIC_BASE + Offset); + return READ_REGISTER_ULONG((PULONG)(APIC_BASE + Offset)); } FORCEINLINE VOID ApicWrite(ULONG Offset, ULONG Value) { - *(volatile ULONG *)(APIC_BASE + Offset) = Value; + WRITE_REGISTER_ULONG((PULONG)(APIC_BASE + Offset), Value); } VOID diff --git a/hal/halx86/apic/apictimer.c b/hal/halx86/apic/apictimer.c index 15f87a58499..ec1e0f1ecef 100644 --- a/hal/halx86/apic/apictimer.c +++ b/hal/halx86/apic/apictimer.c @@ -61,6 +61,13 @@ ApicInitializeTimer(ULONG Cpu) // KeSetTimeIncrement } +VOID +FASTCALL +HalpProfileInterruptHandler(_In_ PKTRAP_FRAME TrapFrame) +{ + KeProfileInterruptWithSource(TrapFrame, ProfileTime); +} + /* PUBLIC FUNCTIONS ***********************************************************/ diff --git a/hal/halx86/apic/rtctimer.c b/hal/halx86/apic/rtctimer.c index 817e6e2de88..2d30ad55c43 100644 --- a/hal/halx86/apic/rtctimer.c +++ b/hal/halx86/apic/rtctimer.c @@ -167,13 +167,6 @@ HalpClockInterruptHandler(IN PKTRAP_FRAME TrapFrame) KeUpdateSystemTime(TrapFrame, LastIncrement, Irql); } -VOID -FASTCALL -HalpProfileInterruptHandler(IN PKTRAP_FRAME TrapFrame) -{ - __debugbreak(); -} - ULONG NTAPI HalSetTimeIncrement(IN ULONG Increment) diff --git a/hal/halx86/apic/tsc.c b/hal/halx86/apic/tsc.c index ce06b2f6d8e..1ff881206fd 100644 --- a/hal/halx86/apic/tsc.c +++ b/hal/halx86/apic/tsc.c @@ -55,8 +55,7 @@ NTAPI HalpInitializeTsc(VOID) { ULONG_PTR Flags; - KIDTENTRY OldIdtEntry, *IdtPointer; - PKPCR Pcr = KeGetPcr(); + PVOID PreviousHandler; UCHAR RegisterA, RegisterB; /* Check if the CPU supports RDTSC */ @@ -79,8 +78,7 @@ HalpInitializeTsc(VOID) HalpWriteCmos(RTC_REGISTER_A, RegisterA); /* Save old IDT entry */ - IdtPointer = KiGetIdtEntry(Pcr, HalpRtcClockVector); - OldIdtEntry = *IdtPointer; + PreviousHandler = KeQueryInterruptHandler(HalpRtcClockVector); /* Set the calibration ISR */ KeRegisterInterruptHandler(HalpRtcClockVector, TscCalibrationISR); @@ -105,8 +103,8 @@ HalpInitializeTsc(VOID) /* Disable the timer interrupt */ HalDisableSystemInterrupt(HalpRtcClockVector, CLOCK_LEVEL); - /* Restore old IDT entry */ - *IdtPointer = OldIdtEntry; + /* Restore the previous handler */ + KeRegisterInterruptHandler(HalpRtcClockVector, PreviousHandler); /* Calculate an average, using simplified linear regression */ HalpCpuClockFrequency.QuadPart = DoLinearRegression(NUM_SAMPLES - 1,

1 0

[reactos] 02/06: [MINIHAL] Compile less stuff
by Timo Kreuzer 17 Jun '21

17 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=ee514e954ba29dc81e7e1… commit ee514e954ba29dc81e7e1c10482d94d7abca08ac Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Thu Jun 3 11:19:30 2021 +0200 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Jun 17 23:27:44 2021 +0200 [MINIHAL] Compile less stuff --- hal/halx86/legacy/bus/pcibus.c | 5 ++++- hal/halx86/legacy/bussupp.c | 10 ++++++++++ hal/halx86/minihal/CMakeLists.txt | 1 - 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/hal/halx86/legacy/bus/pcibus.c b/hal/halx86/legacy/bus/pcibus.c index b4c0bd070df..f71142626b9 100644 --- a/hal/halx86/legacy/bus/pcibus.c +++ b/hal/halx86/legacy/bus/pcibus.c @@ -519,7 +519,7 @@ HalpSetPCIData(IN PBUS_HANDLER BusHandler, /* Update the total length read */ return Len; } - +#ifndef _MINIHAL_ ULONG NTAPI HalpGetPCIIntOnISABus(IN PBUS_HANDLER BusHandler, @@ -540,6 +540,7 @@ HalpGetPCIIntOnISABus(IN PBUS_HANDLER BusHandler, Irql, Affinity); } +#endif // _MINIHAL_ VOID NTAPI @@ -562,6 +563,7 @@ HalpPCIISALine2Pin(IN PBUS_HANDLER BusHandler, UNIMPLEMENTED_DBGBREAK(); } +#ifndef _MINIHAL_ NTSTATUS NTAPI HalpGetISAFixedPCIIrq(IN PBUS_HANDLER BusHandler, @@ -654,6 +656,7 @@ HalpRegisterPciDebuggingDeviceInfo(VOID) UNIMPLEMENTED_DBGBREAK("You have implemented the KD routines for searching PCI debugger" "devices, but you have forgotten to implement this routine\n"); } +#endif // _MINIHAL_ static ULONG NTAPI PciSize(ULONG Base, ULONG Mask) diff --git a/hal/halx86/legacy/bussupp.c b/hal/halx86/legacy/bussupp.c index 6e291d58479..97b535285d3 100644 --- a/hal/halx86/legacy/bussupp.c +++ b/hal/halx86/legacy/bussupp.c @@ -152,6 +152,7 @@ HalpAllocateBusHandler(IN INTERFACE_TYPE InterfaceType, return Bus; } +#ifndef _MINIHAL_ CODE_SEG("INIT") VOID NTAPI @@ -225,6 +226,7 @@ HalpRegisterInternalBusHandlers(VOID) /* No support for EISA or MCA */ ASSERT(HalpBusType == MACHINE_TYPE_ISA); } +#endif // _MINIHAL_ #ifndef _MINIHAL_ CODE_SEG("INIT") @@ -1230,6 +1232,7 @@ HalpInitializePciBus(VOID) #endif } +#ifndef _MINIHAL_ CODE_SEG("INIT") VOID NTAPI @@ -1262,6 +1265,7 @@ HalpRegisterKdSupportFunctions(VOID) /* Register ACPI stub */ KdCheckPowerButton = HalpCheckPowerButton; } +#endif // _MINIHAL_ NTSTATUS NTAPI @@ -1405,6 +1409,7 @@ HaliTranslateBusAddress(IN INTERFACE_TYPE InterfaceType, /* PUBLIC FUNCTIONS **********************************************************/ +#ifndef _MINIHAL_ /* * @implemented */ @@ -1430,6 +1435,7 @@ HalAdjustResourceList(IN PIO_RESOURCE_REQUIREMENTS_LIST *ResourceList) HalDereferenceBusHandler(Handler); return Status; } +#endif // _MINIHAL_ /* * @implemented @@ -1474,6 +1480,7 @@ HalAssignSlotResources(IN PUNICODE_STRING RegistryPath, } } +#ifndef _MINIHAL_ /* * @implemented */ @@ -1493,6 +1500,7 @@ HalGetBusData(IN BUS_DATA_TYPE BusDataType, 0, Length); } +#endif // _MINIHAL_ /* * @implemented @@ -1526,6 +1534,7 @@ HalGetBusDataByOffset(IN BUS_DATA_TYPE BusDataType, return Status; } +#ifndef _MINIHAL_ /* * @implemented */ @@ -1623,6 +1632,7 @@ HalSetBusDataByOffset(IN BUS_DATA_TYPE BusDataType, HalDereferenceBusHandler(Handler); return Status; } +#endif // _MINIHAL_ /* * @implemented diff --git a/hal/halx86/minihal/CMakeLists.txt b/hal/halx86/minihal/CMakeLists.txt index 94a3e93112f..e8300e7bc56 100644 --- a/hal/halx86/minihal/CMakeLists.txt +++ b/hal/halx86/minihal/CMakeLists.txt @@ -5,7 +5,6 @@ list(APPEND MINI_HAL_SOURCE ../legacy/bus/cmosbus.c ../legacy/bus/isabus.c ../legacy/bus/pcibus.c - ../legacy/bus/sysbus.c ../legacy/bussupp.c ../generic/bios.c ../generic/dma.c

1 0

[reactos] 01/06: [HAL] Rename apic.h to apicp.h and move it back to the apic folder
by Timo Kreuzer 17 Jun '21

17 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=7243503916bc0c0201db3… commit 7243503916bc0c0201db3809e67b02c3dc13f516 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Mon May 24 11:27:12 2021 +0200 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Jun 17 23:27:44 2021 +0200 [HAL] Rename apic.h to apicp.h and move it back to the apic folder --- hal/halx86/apic/apic.c | 2 +- hal/halx86/{include/apic.h => apic/apicp.h} | 0 hal/halx86/apic/apictimer.c | 2 +- hal/halx86/apic/halinit.c | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hal/halx86/apic/apic.c b/hal/halx86/apic/apic.c index 4c98e13bdf9..2811cc45535 100644 --- a/hal/halx86/apic/apic.c +++ b/hal/halx86/apic/apic.c @@ -12,7 +12,7 @@ /* INCLUDES *******************************************************************/ #include <hal.h> -#include <apic.h> +#include "apicp.h" #define NDEBUG #include <debug.h> diff --git a/hal/halx86/include/apic.h b/hal/halx86/apic/apicp.h similarity index 100% rename from hal/halx86/include/apic.h rename to hal/halx86/apic/apicp.h diff --git a/hal/halx86/apic/apictimer.c b/hal/halx86/apic/apictimer.c index 1a152e2d410..15f87a58499 100644 --- a/hal/halx86/apic/apictimer.c +++ b/hal/halx86/apic/apictimer.c @@ -9,7 +9,7 @@ /* INCLUDES ******************************************************************/ #include <hal.h> -#include <apic.h> +#include "apicp.h" #define NDEBUG #include <debug.h> diff --git a/hal/halx86/apic/halinit.c b/hal/halx86/apic/halinit.c index 504f568dbb1..5223e29b758 100644 --- a/hal/halx86/apic/halinit.c +++ b/hal/halx86/apic/halinit.c @@ -8,7 +8,7 @@ /* INCLUDES *****************************************************************/ #include <hal.h> -#include <apic.h> +#include "apicp.h" #define NDEBUG #include <debug.h>

1 0

[reactos] 03/03: [NTOS:IOPNP] Both PNP_PROPERTY_REMOVAL_POLICY_OVERRIDE and PNP_PROPERTY_LOCATION_PATHS are unimplemented, return such status.
by Hermès Bélusca-Maïto 16 Jun '21

16 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=4864c874a28b908c73702… commit 4864c874a28b908c737020d1945cd9812f981ffd Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Fri Jun 11 02:00:23 2021 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Thu Jun 17 00:35:22 2021 +0200 [NTOS:IOPNP] Both PNP_PROPERTY_REMOVAL_POLICY_OVERRIDE and PNP_PROPERTY_LOCATION_PATHS are unimplemented, return such status. Fixes CID 1441387: Uninitialized scalar variable (UNINIT), in the case of PNP_PROPERTY_LOCATION_PATHS. --- ntoskrnl/io/pnpmgr/plugplay.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ntoskrnl/io/pnpmgr/plugplay.c b/ntoskrnl/io/pnpmgr/plugplay.c index cb7e03976ab..b9551207c33 100644 --- a/ntoskrnl/io/pnpmgr/plugplay.c +++ b/ntoskrnl/io/pnpmgr/plugplay.c @@ -418,6 +418,9 @@ IopGetDeviceProperty(PPLUGPLAY_CONTROL_PROPERTY_DATA PropertyData) } else if (Property == PNP_PROPERTY_REMOVAL_POLICY_OVERRIDE) { + UNIMPLEMENTED; + BufferSize = 0; + Status = STATUS_NOT_IMPLEMENTED; } else if (Property == PNP_PROPERTY_REMOVAL_POLICY_HARDWARE_DEFAULT) { @@ -476,6 +479,9 @@ IopGetDeviceProperty(PPLUGPLAY_CONTROL_PROPERTY_DATA PropertyData) #if (WINVER >= _WIN32_WINNT_WS03) case PNP_PROPERTY_LOCATION_PATHS: + UNIMPLEMENTED; + BufferSize = 0; + Status = STATUS_NOT_IMPLEMENTED; break; #endif

1 0

[reactos] 02/03: [NTOS:IO] Comment out (and explain why it is there) an unused LdrEntry assignment in IopInitializeBootDrivers(). CID 1237114
by Hermès Bélusca-Maïto 16 Jun '21

16 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f2645e48b9aabe15642bd… commit f2645e48b9aabe15642bd463a1a706929dc29d73 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Fri Jun 11 01:46:50 2021 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Thu Jun 17 00:35:22 2021 +0200 [NTOS:IO] Comment out (and explain why it is there) an unused LdrEntry assignment in IopInitializeBootDrivers(). CID 1237114 --- ntoskrnl/io/iomgr/driver.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ntoskrnl/io/iomgr/driver.c b/ntoskrnl/io/iomgr/driver.c index edb64baef7b..a5ab9904bb4 100644 --- a/ntoskrnl/io/iomgr/driver.c +++ b/ntoskrnl/io/iomgr/driver.c @@ -1050,8 +1050,14 @@ IopInitializeBootDrivers(VOID) BOOT_DRIVER_LIST_ENTRY, Link); + // FIXME: TODO: This LdrEntry is to be used in a special handling + // for SETUPLDR (a similar procedure is done on Windows), where + // the loader would, under certain conditions, be loaded in the + // SETUPLDR-specific code block below... +#if 0 /* Get the driver loader entry */ LdrEntry = BootEntry->LdrEntry; +#endif /* Allocate our internal accounting structure */ DriverInfo = ExAllocatePoolWithTag(PagedPool,

1 0

[reactos] 01/03: [NTOS:IO] Revert part of commit a82ff90b2: don't do direct ULONG data access: it might be unaligned.
by Hermès Bélusca-Maïto 16 Jun '21

16 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=fe416b67dca395e4ad02a… commit fe416b67dca395e4ad02aa9e6eb83733ea3ee63b Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Thu Jun 17 00:29:58 2021 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Thu Jun 17 00:35:21 2021 +0200 [NTOS:IO] Revert part of commit a82ff90b2: don't do direct ULONG data access: it might be unaligned. Requested by Victor. --- ntoskrnl/io/iomgr/driver.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ntoskrnl/io/iomgr/driver.c b/ntoskrnl/io/iomgr/driver.c index 6a42dd30f7a..edb64baef7b 100644 --- a/ntoskrnl/io/iomgr/driver.c +++ b/ntoskrnl/io/iomgr/driver.c @@ -213,7 +213,10 @@ IopGetDriverNames( status = STATUS_ILL_FORMED_SERVICE_ENTRY; goto Cleanup; } - driverType = *(PULONG)((ULONG_PTR)kvInfo + kvInfo->DataOffset); + + RtlMoveMemory(&driverType, + (PVOID)((ULONG_PTR)kvInfo + kvInfo->DataOffset), + sizeof(ULONG)); ExFreePool(kvInfo); /* Compute the necessary driver name string size */ @@ -913,7 +916,10 @@ IopInitializeBuiltinDriver(IN PLDR_DATA_TABLE_ENTRY BootLdrEntry) ExFreePool(kvInfo); goto Cleanup; } - instanceCount = *(PULONG)((ULONG_PTR)kvInfo + kvInfo->DataOffset); + + RtlMoveMemory(&instanceCount, + (PVOID)((ULONG_PTR)kvInfo + kvInfo->DataOffset), + sizeof(ULONG)); ExFreePool(kvInfo); DPRINT("Processing %u instances for %wZ module\n", instanceCount, ModuleName);

1 0

[reactos] 02/02: [NTOS:IO] Fix driverName.Buffer leak in some failure paths in IopGetDriverNames().
by Hermès Bélusca-Maïto 16 Jun '21

16 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=32a82eb123e75544cf6cd… commit 32a82eb123e75544cf6cd9341589928bed5be89c Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Thu Jun 10 23:11:27 2021 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Wed Jun 16 22:18:02 2021 +0200 [NTOS:IO] Fix driverName.Buffer leak in some failure paths in IopGetDriverNames(). driverName.Buffer leaked when the "(!NT_SUCCESS(status) || ServiceName != NULL)" case is taken because ServiceName != NULL, and some of the functions fail. --- ntoskrnl/io/iomgr/driver.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ntoskrnl/io/iomgr/driver.c b/ntoskrnl/io/iomgr/driver.c index f48a85a9705..6a42dd30f7a 100644 --- a/ntoskrnl/io/iomgr/driver.c +++ b/ntoskrnl/io/iomgr/driver.c @@ -171,21 +171,22 @@ IopGetDriverNames( status = ZwQueryKey(ServiceHandle, KeyBasicInformation, NULL, 0, &infoLength); if (status != STATUS_BUFFER_TOO_SMALL) { - return NT_SUCCESS(status) ? STATUS_UNSUCCESSFUL : status; + status = (NT_SUCCESS(status) ? STATUS_UNSUCCESSFUL : status); + goto Cleanup; } /* Allocate the buffer and retrieve the data */ basicInfo = ExAllocatePoolWithTag(PagedPool, infoLength, TAG_IO); if (!basicInfo) { - return STATUS_INSUFFICIENT_RESOURCES; + status = STATUS_INSUFFICIENT_RESOURCES; + goto Cleanup; } status = ZwQueryKey(ServiceHandle, KeyBasicInformation, basicInfo, infoLength, &infoLength); if (!NT_SUCCESS(status)) { - ExFreePoolWithTag(basicInfo, TAG_IO); - return status; + goto Cleanup; } serviceName.Length = basicInfo->NameLength; @@ -248,7 +249,6 @@ IopGetDriverNames( PWCHAR buf = ExAllocatePoolWithTag(PagedPool, serviceName.Length, TAG_IO); if (!buf) { - ExFreePoolWithTag(driverName.Buffer, TAG_IO); status = STATUS_INSUFFICIENT_RESOURCES; goto Cleanup; } @@ -265,6 +265,9 @@ Cleanup: if (basicInfo) ExFreePoolWithTag(basicInfo, TAG_IO); + if (!NT_SUCCESS(status) && driverName.Buffer) + ExFreePoolWithTag(driverName.Buffer, TAG_IO); + return status; }

1 0

[reactos] 01/02: [NTOS:IO] Fix basicInfo handling in IopGetDriverNames().
by Hermès Bélusca-Maïto 16 Jun '21

16 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=e09d1dec7ae938e27ca30… commit e09d1dec7ae938e27ca3013d2ffefae6bf78d230 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Thu Jun 10 22:31:45 2021 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Wed Jun 16 22:18:02 2021 +0200 [NTOS:IO] Fix basicInfo handling in IopGetDriverNames(). - Fix CID 1477246: Uninitialized pointer read (UNINIT) (happens in the last ExFreePoolWithTag(basicInfo, TAG_IO) call when the "(!NT_SUCCESS(status) || ServiceName != NULL)" case is not taken). - Centralize all the ExFreePoolWithTag(basicInfo, TAG_IO) cleanups at the end of the function. - Both cases "(driverName.Buffer == NULL)" and "(ServiceName != NULL)" can only be taken when basicInfo != NULL, so assert on this fact. --- ntoskrnl/io/iomgr/driver.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/ntoskrnl/io/iomgr/driver.c b/ntoskrnl/io/iomgr/driver.c index 6d82bfc536e..f48a85a9705 100644 --- a/ntoskrnl/io/iomgr/driver.c +++ b/ntoskrnl/io/iomgr/driver.c @@ -163,7 +163,7 @@ IopGetDriverNames( /* Check whether we need to get ServiceName as well, either to construct * the driver name (because we could not use "ObjectName"), or because * it is requested by the caller. */ - PKEY_BASIC_INFORMATION basicInfo; + PKEY_BASIC_INFORMATION basicInfo = NULL; if (!NT_SUCCESS(status) || ServiceName != NULL) { /* Retrieve the necessary buffer size */ @@ -197,19 +197,20 @@ IopGetDriverNames( * it will be either "\Driver\<ServiceName>" or "\FileSystem\<ServiceName>" */ if (driverName.Buffer == NULL) { + ASSERT(basicInfo); // Container for serviceName + /* Retrieve the driver type */ ULONG driverType; status = IopGetRegistryValue(ServiceHandle, L"Type", &kvInfo); if (!NT_SUCCESS(status)) { - ExFreePoolWithTag(basicInfo, TAG_IO); - return status; + goto Cleanup; } if (kvInfo->Type != REG_DWORD || kvInfo->DataLength != sizeof(ULONG)) { ExFreePool(kvInfo); - ExFreePoolWithTag(basicInfo, TAG_IO); // container for serviceName - return STATUS_ILL_FORMED_SERVICE_ENTRY; + status = STATUS_ILL_FORMED_SERVICE_ENTRY; + goto Cleanup; } driverType = *(PULONG)((ULONG_PTR)kvInfo + kvInfo->DataOffset); ExFreePool(kvInfo); @@ -227,8 +228,8 @@ IopGetDriverNames( driverName.Buffer = ExAllocatePoolWithTag(NonPagedPool, driverName.MaximumLength, TAG_IO); if (!driverName.Buffer) { - ExFreePoolWithTag(basicInfo, TAG_IO); // container for serviceName - return STATUS_INSUFFICIENT_RESOURCES; + status = STATUS_INSUFFICIENT_RESOURCES; + goto Cleanup; } if (driverType == SERVICE_RECOGNIZER_DRIVER || driverType == SERVICE_FILE_SYSTEM_DRIVER) @@ -241,24 +242,30 @@ IopGetDriverNames( if (ServiceName != NULL) { + ASSERT(basicInfo); // Container for serviceName + /* Allocate a copy for the caller */ PWCHAR buf = ExAllocatePoolWithTag(PagedPool, serviceName.Length, TAG_IO); if (!buf) { - ExFreePoolWithTag(basicInfo, TAG_IO); // container for serviceName ExFreePoolWithTag(driverName.Buffer, TAG_IO); - return STATUS_INSUFFICIENT_RESOURCES; + status = STATUS_INSUFFICIENT_RESOURCES; + goto Cleanup; } RtlMoveMemory(buf, serviceName.Buffer, serviceName.Length); ServiceName->MaximumLength = serviceName.Length; ServiceName->Length = serviceName.Length; ServiceName->Buffer = buf; } - ExFreePoolWithTag(basicInfo, TAG_IO); // container for ServiceName *DriverName = driverName; + status = STATUS_SUCCESS; - return STATUS_SUCCESS; +Cleanup: + if (basicInfo) + ExFreePoolWithTag(basicInfo, TAG_IO); + + return status; } /*

1 0

[reactos] 02/02: [NTOS:IO] Fix similar bugs in IopGetDriverNames(), IopInitializeBuiltinDriver() and IopLoadDriver().
by Hermès Bélusca-Maïto 16 Jun '21

16 Jun '21

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=0d28f271565c5ec3e04b6… commit 0d28f271565c5ec3e04b6277d83dd6b341e680eb Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Thu Jun 10 21:20:17 2021 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Wed Jun 16 22:14:01 2021 +0200 [NTOS:IO] Fix similar bugs in IopGetDriverNames(), IopInitializeBuiltinDriver() and IopLoadDriver(). - As they are initialized, some registry string values must be NULL-terminated. - When retrieving REG_DWORD registry values, validate their length. --- ntoskrnl/io/iomgr/driver.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ntoskrnl/io/iomgr/driver.c b/ntoskrnl/io/iomgr/driver.c index 4fd092662d4..6d82bfc536e 100644 --- a/ntoskrnl/io/iomgr/driver.c +++ b/ntoskrnl/io/iomgr/driver.c @@ -144,7 +144,7 @@ IopGetDriverNames( return STATUS_ILL_FORMED_SERVICE_ENTRY; } - driverName.Length = kvInfo->DataLength - sizeof(WCHAR); + driverName.Length = kvInfo->DataLength - sizeof(UNICODE_NULL); driverName.MaximumLength = kvInfo->DataLength; driverName.Buffer = ExAllocatePoolWithTag(NonPagedPool, driverName.MaximumLength, TAG_IO); if (!driverName.Buffer) @@ -156,6 +156,7 @@ IopGetDriverNames( RtlMoveMemory(driverName.Buffer, (PVOID)((ULONG_PTR)kvInfo + kvInfo->DataOffset), driverName.Length); + driverName.Buffer[driverName.Length / sizeof(WCHAR)] = UNICODE_NULL; ExFreePool(kvInfo); } @@ -204,7 +205,7 @@ IopGetDriverNames( ExFreePoolWithTag(basicInfo, TAG_IO); return status; } - if (kvInfo->Type != REG_DWORD) + if (kvInfo->Type != REG_DWORD || kvInfo->DataLength != sizeof(ULONG)) { ExFreePool(kvInfo); ExFreePoolWithTag(basicInfo, TAG_IO); // container for serviceName @@ -897,7 +898,7 @@ IopInitializeBuiltinDriver(IN PLDR_DATA_TABLE_ENTRY BootLdrEntry) { goto Cleanup; } - if (kvInfo->Type != REG_DWORD) + if (kvInfo->Type != REG_DWORD || kvInfo->DataLength != sizeof(ULONG)) { ExFreePool(kvInfo); goto Cleanup; @@ -924,7 +925,7 @@ IopInitializeBuiltinDriver(IN PLDR_DATA_TABLE_ENTRY BootLdrEntry) continue; } - instancePath.Length = kvInfo->DataLength - sizeof(WCHAR); + instancePath.Length = kvInfo->DataLength - sizeof(UNICODE_NULL); instancePath.MaximumLength = kvInfo->DataLength; instancePath.Buffer = ExAllocatePoolWithTag(NonPagedPool, instancePath.MaximumLength, @@ -932,8 +933,9 @@ IopInitializeBuiltinDriver(IN PLDR_DATA_TABLE_ENTRY BootLdrEntry) if (instancePath.Buffer) { RtlMoveMemory(instancePath.Buffer, - (PVOID)((ULONG_PTR)kvInfo + kvInfo->DataOffset), - instancePath.Length); + (PVOID)((ULONG_PTR)kvInfo + kvInfo->DataOffset), + instancePath.Length); + instancePath.Buffer[instancePath.Length / sizeof(WCHAR)] = UNICODE_NULL; PDEVICE_OBJECT pdo = IopGetDeviceObjectFromDeviceInstance(&instancePath); PiQueueDeviceAction(pdo, PiActionAddBootDevices, NULL, NULL); @@ -1900,6 +1902,7 @@ IopLoadDriver( RtlMoveMemory(ImagePath.Buffer, (PVOID)((ULONG_PTR)kvInfo + kvInfo->DataOffset), ImagePath.Length); + ImagePath.Buffer[ImagePath.Length / sizeof(WCHAR)] = UNICODE_NULL; ExFreePool(kvInfo); } else

1 0

← Newer
1
...
11
12
13
14
15
16
17
...
23
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-diffs June 2021