[reactos] 01/01: [NTOSKRNL] Force a probe against ReturnLength on query & Misc ICIF stuff
12 Jun
2022
12 Jun
'22
11:44 a.m.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=d0d86ab5885af2cc9fa40…
commit d0d86ab5885af2cc9fa40104f47040c63db407be
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sat Jun 11 13:19:51 2022 +0200
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Sun Jun 12 11:05:05 2022 +0200
[NTOSKRNL] Force a probe against ReturnLength on query & Misc ICIF stuff
NtQueryInformationToken is by far the only system call in NT where ReturnLength simply
cannot be optional. On Windows this parameter is always probed and an argument to NULL
directly leads to an access violation exception.
This is due to the fact of how tokens work, as its information contents (token user,
owner, primary group, et al) are dynamic and can vary throughout over time in memory.
What happens on current ReactOS master however is that ReturnLength is only probed if
the parameter is not NULL. On a NULL case scenario the probing checks succeed and
NtQueryInformationToken fails later. For this, just get rid of CompleteProbing
parameter and opt in for a bit mask flag based approach, with
ICIF_FORCE_RETURN_LENGTH_PROBE being set on DefaultQueryInfoBufferCheck which
NtQueryInformationToken calls it to do sanity checks.
In addition to that...
- Document the ICIF probe helpers
- Annotate the ICIF prope helpers with SAL
- With the riddance of CompleteProbing and adoption of flags based approach, add
ICIF_PROBE_READ_WRITE and ICIF_PROBE_READ flags alongside with
ICIF_FORCE_RETURN_LENGTH_PROBE
---
ntoskrnl/ex/event.c | 4 +-
ntoskrnl/ex/mutant.c | 4 +-
ntoskrnl/ex/sem.c | 4 +-
ntoskrnl/ex/timer.c | 4 +-
ntoskrnl/include/internal/icif.h | 7 +-
ntoskrnl/include/internal/probe.h | 196 ++++++++++++++++++++++++++++++++++----
ntoskrnl/io/iomgr/iocomp.c | 4 +-
ntoskrnl/ps/query.c | 8 +-
ntoskrnl/se/tokencls.c | 4 +-
9 files changed, 200 insertions(+), 35 deletions(-)
diff --git a/ntoskrnl/ex/event.c b/ntoskrnl/ex/event.c
index e30420f65bb..27222cb3c4a 100644
--- a/ntoskrnl/ex/event.c
+++ b/ntoskrnl/ex/event.c
@@ -329,12 +329,12 @@ NtQueryEvent(IN HANDLE EventHandle,
ExEventInfoClass,
sizeof(ExEventInfoClass) /
sizeof(ExEventInfoClass[0]),
+ ICIF_PROBE_READ_WRITE,
EventInformation,
EventInformationLength,
ReturnLength,
NULL,
- PreviousMode,
- TRUE);
+ PreviousMode);
if(!NT_SUCCESS(Status))
{
/* Invalid buffers */
diff --git a/ntoskrnl/ex/mutant.c b/ntoskrnl/ex/mutant.c
index bab471c6173..c5b0204c3c1 100644
--- a/ntoskrnl/ex/mutant.c
+++ b/ntoskrnl/ex/mutant.c
@@ -239,12 +239,12 @@ NtQueryMutant(IN HANDLE MutantHandle,
ExMutantInfoClass,
sizeof(ExMutantInfoClass) /
sizeof(ExMutantInfoClass[0]),
+ ICIF_PROBE_READ_WRITE,
MutantInformation,
MutantInformationLength,
ResultLength,
NULL,
- PreviousMode,
- TRUE);
+ PreviousMode);
if(!NT_SUCCESS(Status))
{
DPRINT("NtQueryMutant() failed, Status: 0x%x\n", Status);
diff --git a/ntoskrnl/ex/sem.c b/ntoskrnl/ex/sem.c
index 29c622814ff..29b8b75bf5a 100644
--- a/ntoskrnl/ex/sem.c
+++ b/ntoskrnl/ex/sem.c
@@ -235,12 +235,12 @@ NtQuerySemaphore(IN HANDLE SemaphoreHandle,
ExSemaphoreInfoClass,
sizeof(ExSemaphoreInfoClass) /
sizeof(ExSemaphoreInfoClass[0]),
+ ICIF_PROBE_READ_WRITE,
SemaphoreInformation,
SemaphoreInformationLength,
ReturnLength,
NULL,
- PreviousMode,
- TRUE);
+ PreviousMode);
if (!NT_SUCCESS(Status))
{
/* Invalid buffers */
diff --git a/ntoskrnl/ex/timer.c b/ntoskrnl/ex/timer.c
index 0c33675333c..6566648c5d6 100644
--- a/ntoskrnl/ex/timer.c
+++ b/ntoskrnl/ex/timer.c
@@ -532,12 +532,12 @@ NtQueryTimer(IN HANDLE TimerHandle,
ExTimerInfoClass,
sizeof(ExTimerInfoClass) /
sizeof(ExTimerInfoClass[0]),
+ ICIF_PROBE_READ_WRITE,
TimerInformation,
TimerInformationLength,
ReturnLength,
NULL,
- PreviousMode,
- TRUE);
+ PreviousMode);
if (!NT_SUCCESS(Status)) return Status;
/* Get the Timer Object */
diff --git a/ntoskrnl/include/internal/icif.h b/ntoskrnl/include/internal/icif.h
index e73ad9e1b2c..39066d935e9 100644
--- a/ntoskrnl/include/internal/icif.h
+++ b/ntoskrnl/include/internal/icif.h
@@ -2,8 +2,7 @@
* PROJECT: ReactOS Kernel
* LICENSE: GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
* PURPOSE: Internal header for information classes info interface
- * COPYRIGHT: Copyright ???
- * Copyright 2020-2021 George Bișoc <george.bisoc(a)reactos.org>
+ * COPYRIGHT: Copyright 2020-2022 George Bișoc <george.bisoc(a)reactos.org>
*/
#pragma once
@@ -22,6 +21,10 @@
#define ICIF_SET_SIZE_VARIABLE 0x8
#define ICIF_SIZE_VARIABLE (ICIF_QUERY_SIZE_VARIABLE | ICIF_SET_SIZE_VARIABLE)
+#define ICIF_PROBE_READ_WRITE 0x0
+#define ICIF_PROBE_READ 0x1
+#define ICIF_FORCE_RETURN_LENGTH_PROBE 0x2
+
typedef struct _INFORMATION_CLASS_INFO
{
USHORT RequiredSizeQUERY;
diff --git a/ntoskrnl/include/internal/probe.h b/ntoskrnl/include/internal/probe.h
index b819718f5ae..ff1d3c5275b 100644
--- a/ntoskrnl/include/internal/probe.h
+++ b/ntoskrnl/include/internal/probe.h
@@ -1,16 +1,79 @@
+/*
+ * PROJECT: ReactOS Kernel
+ * LICENSE: GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
+ * PURPOSE: Internal header containing information class probing helpers
+ * COPYRIGHT: Copyright 2022 George Bișoc <george.bisoc(a)reactos.org>
+ */
+
#pragma once
#include <reactos/probe.h>
+/**
+ * @brief
+ * Probe helper that validates the provided parameters whenever
+ * a NtSet*** system call is invoked from user or kernel mode.
+ *
+ * @param[in] Class
+ * The specific class information that the caller explicitly
+ * requested information to be set into an object.
+ *
+ * @param[in] ClassList
+ * An internal INFORMATION_CLASS_INFO consisting of a list array
+ * of information classes checked against the requested information
+ * classes given in Class parameter.
+ *
+ * @param[in] ClassListEntries
+ * Specifies the number of class entries in an array, provided by
+ * the ClassList parameter.
+ *
+ * @param[in] Buffer
+ * A pointer to an arbitrary data content in memory to be validated.
+ * Such pointer points to the actual arbitrary information class buffer
+ * to be set into the object. This buffer is validated only if the
+ * calling processor mode is UM (aka user mode).
+ *
+ * @param[in] BufferLength
+ * The length of the buffer pointed by the Buffer parameter, whose size
+ * is in bytes.
+ *
+ * @param[in] PreviousMode
+ * The processor calling level mode. This level mode determines the procedure
+ * of probing validation in action. If the level calling mode is KM (aka kernel mode)
+ * this function will only validate the properties of the information class itself
+ * such as the required information length size. If the calling mode is UM, the
+ * pointer buffer provided by Buffer parameter is also validated.
+ *
+ * @return
+ * The outcome of the probe validation is based upon the returned NTSTATUS code.
+ * STATUS_SUCCESS is returned if the validation succeeded. Otherwise, one of the
+ * following failure status codes is returned:
+ *
+ * STATUS_INVALID_INFO_CLASS -- Indicates the given information class is not a valid
+ * valid SET class (ICIF_SET flag is not set to the corresponding information class)
+ * or the actual class is not present in the class list array.
+ *
+ * STATUS_INFO_LENGTH_MISMATCH -- Indicates the information length doesn't match
with
+ * the one that the information class itself expects. This is the case with classes
+ * ICIF_SET_SIZE_VARIABLE is not set, which means that the class requires a fixed
+ * length size.
+ *
+ * STATUS_ACCESS_VIOLATION -- Indicates the buffer is not within the user mode probe
+ * address range. The function will raise an exception.
+ *
+ * STATUS_DATATYPE_MISALIGNMENT -- Indicates the address of the buffer in memory is
+ * not aligned to the required alignment set.
+ */
static
__inline
NTSTATUS
-DefaultSetInfoBufferCheck(ULONG Class,
- const INFORMATION_CLASS_INFO *ClassList,
- ULONG ClassListEntries,
- PVOID Buffer,
- ULONG BufferLength,
- KPROCESSOR_MODE PreviousMode)
+DefaultSetInfoBufferCheck(
+ _In_ ULONG Class,
+ _In_ const INFORMATION_CLASS_INFO *ClassList,
+ _In_ ULONG ClassListEntries,
+ _In_ PVOID Buffer,
+ _In_ ULONG BufferLength,
+ _In_ KPROCESSOR_MODE PreviousMode)
{
NTSTATUS Status = STATUS_SUCCESS;
@@ -53,18 +116,116 @@ DefaultSetInfoBufferCheck(ULONG Class,
return Status;
}
+/**
+ * @brief
+ * Probe helper that validates the provided parameters whenever
+ * a NtQuery*** system call is invoked from user or kernel mode.
+ *
+ * @param[in] Class
+ * The specific class information that the caller explicitly
+ * requested information to be queried from an object.
+ *
+ * @param[in] ClassList
+ * An internal INFORMATION_CLASS_INFO consisting of a list array
+ * of information classes checked against the requested information
+ * classes given in Class parameter.
+ *
+ * @param[in] ClassListEntries
+ * Specifies the number of class entries in an array, provided by
+ * the ClassList parameter.
+ *
+ * @param[in] Flags
+ * Specifies a bit mask flag that influences how the query probe
+ * validation must be performed against Buffer and ReturnLength
+ * parameters. For further information in regard of this parameter,
+ * see remarks.
+ *
+ * @param[in] Buffer
+ * A pointer to an arbitrary data content in memory to be validated.
+ * Such parameter must be an initialized variable where the queried
+ * information is going to be received into this pointer. If the calling
+ * processor mode is UM (aka user mode) this parameter is validated.
+ * This parameter can be NULL (see remarks for more details).
+ *
+ * @param[in] BufferLength
+ * The length of the buffer pointed by the Buffer parameter, whose size
+ * is in bytes. If the Buffer parameter is NULL, this parameter can be 0.
+ *
+ * @param[in] ReturnLength
+ * The returned length of the buffer whose size is in bytes. If Buffer is
+ * NULL as well as BufferLength is 0, this parameter receives the actual
+ * return length needed to store the buffer in memory space. If the
+ * processor level calling mode is UM, this parameter is validated.
+ * If ICIF_FORCE_RETURN_LENGTH_PROBE is specified in Flags parameter,
+ * ReturnLength mustn't be NULL (see remarks). Otherwise it can be NULL.
+ *
+ * @param[in] ReturnLengthPtr
+ * This parameter is of the same nature as the ReturnLength one, with the
+ * difference being that the type parameter can be a ULONG on x86 systems
+ * or a ULONGLONG on AMD64 systems. If the processor level calling mode is
+ * UM, this parameter is validated. This parameter is currently unused.
+ *
+ * @param[in] PreviousMode
+ * The processor calling level mode. This level mode determines the procedure
+ * of probing validation in action. If the level calling mode is KM (aka kernel mode)
+ * this function will only validate the properties of the information class itself
+ * such as the required information length size. If the calling mode is UM, the
+ * pointer buffer provided by Buffer parameter is also validated as well as
+ * the return length parameter.
+ *
+ * @return
+ * The outcome of the probe validation is based upon the returned NTSTATUS code.
+ * STATUS_SUCCESS is returned if the validation succeeded. Otherwise, one of the
+ * following failure status codes is returned:
+ *
+ * STATUS_INVALID_INFO_CLASS -- Indicates the given information class is not a valid
+ * QUERY class (ICIF_QUERY flag is not set to the corresponding information class)
+ * or the actual class is not present in the class list array.
+ *
+ * STATUS_INFO_LENGTH_MISMATCH -- Indicates the information length doesn't match with
the
+ * one that the information class itself expects. This is the case with classes where
+ * ICIF_QUERY_SIZE_VARIABLE is not set, which means that the class requires a fixed
length size.
+ *
+ * STATUS_ACCESS_VIOLATION -- Indicates the buffer is not within the user mode probe
address range
+ * or the buffer variable is not writable (see remarks). The function will raise an
exception.
+ *
+ * STATUS_DATATYPE_MISALIGNMENT -- Indicates the address of the buffer in memory is not
+ * aligned to the required alignment set.
+ *
+ * @remarks
+ * The probing of Buffer and ReturnLength are influenced based on the probe flags
+ * pointed by Flags parameter. The following flags are:
+ *
+ * ICIF_PROBE_READ_WRITE -- This flag explicitly tells the function to do a read and
+ * write probe against Buffer parameter. ProbeForWrite is invoked in this case.
+ * This is the default mechanism.
+ *
+ * ICIF_PROBE_READ -- This flag explicitly tells the function to do a read probe against
+ * Buffer parameter only, that is, the function does not probe if the parameter is
actually
+ * writable. ProbeForRead is invoked in this case.
+ *
+ * ICIF_FORCE_RETURN_LENGTH_PROBE -- If this flag is set, the function will force probe
+ * the ReturnLength parameter. In this scenario if ReturnLength is NULL a
STATUS_ACCESS_VIOLATION
+ * exception is raised. NtQueryInformationToken is the only NT system call where
ReturnLength
+ * has to be properly initialized and not NULL.
+ *
+ * Buffer parameter can be NULL if the caller does not want to actually query a certain
information
+ * from an object. This is typically with query NT syscalls where a caller has to query
the actual
+ * buffer length needed to store the queried information before doing a "real"
query in the first place.
+ */
static
__inline
NTSTATUS
-DefaultQueryInfoBufferCheck(ULONG Class,
- const INFORMATION_CLASS_INFO *ClassList,
- ULONG ClassListEntries,
- PVOID Buffer,
- ULONG BufferLength,
- PULONG ReturnLength,
- PULONG_PTR ReturnLengthPtr,
- KPROCESSOR_MODE PreviousMode,
- BOOLEAN CompleteProbing)
+DefaultQueryInfoBufferCheck(
+ _In_ ULONG Class,
+ _In_ const INFORMATION_CLASS_INFO *ClassList,
+ _In_ ULONG ClassListEntries,
+ _In_ ULONG Flags,
+ _In_opt_ PVOID Buffer,
+ _In_ ULONG BufferLength,
+ _In_opt_ PULONG ReturnLength,
+ _In_opt_ PULONG_PTR ReturnLengthPtr,
+ _In_ KPROCESSOR_MODE PreviousMode)
{
NTSTATUS Status = STATUS_SUCCESS;
@@ -91,7 +252,7 @@ DefaultQueryInfoBufferCheck(ULONG Class,
{
if (Buffer != NULL)
{
- if (!CompleteProbing)
+ if (Flags & ICIF_PROBE_READ)
{
ProbeForRead(Buffer,
BufferLength,
@@ -105,10 +266,11 @@ DefaultQueryInfoBufferCheck(ULONG Class,
}
}
- if (ReturnLength != NULL)
+ if ((Flags & ICIF_FORCE_RETURN_LENGTH_PROBE) || (ReturnLength !=
NULL))
{
ProbeForWriteUlong(ReturnLength);
}
+
if (ReturnLengthPtr != NULL)
{
ProbeForWrite(ReturnLengthPtr, sizeof(ULONG_PTR),
sizeof(ULONG_PTR));
diff --git a/ntoskrnl/io/iomgr/iocomp.c b/ntoskrnl/io/iomgr/iocomp.c
index 41457a66574..fbc655aedfc 100644
--- a/ntoskrnl/io/iomgr/iocomp.c
+++ b/ntoskrnl/io/iomgr/iocomp.c
@@ -395,12 +395,12 @@ NtQueryIoCompletion(IN HANDLE IoCompletionHandle,
IoCompletionInfoClass,
sizeof(IoCompletionInfoClass) /
sizeof(IoCompletionInfoClass[0]),
+ ICIF_PROBE_READ_WRITE,
IoCompletionInformation,
IoCompletionInformationLength,
ResultLength,
NULL,
- PreviousMode,
- TRUE);
+ PreviousMode);
if (!NT_SUCCESS(Status)) return Status;
/* Get the Object */
diff --git a/ntoskrnl/ps/query.c b/ntoskrnl/ps/query.c
index 7b6f8aecf0e..cc7bae50ba4 100644
--- a/ntoskrnl/ps/query.c
+++ b/ntoskrnl/ps/query.c
@@ -90,12 +90,12 @@ NtQueryInformationProcess(
Status = DefaultQueryInfoBufferCheck(ProcessInformationClass,
PsProcessInfoClass,
RTL_NUMBER_OF(PsProcessInfoClass),
+ ICIF_PROBE_READ,
ProcessInformation,
ProcessInformationLength,
ReturnLength,
NULL,
- PreviousMode,
- FALSE);
+ PreviousMode);
if (!NT_SUCCESS(Status))
{
DPRINT1("NtQueryInformationProcess(): Information verification class failed!
(Status -> 0x%lx, ProcessInformationClass -> %lx)\n", Status,
ProcessInformationClass);
@@ -2643,12 +2643,12 @@ NtQueryInformationThread(IN HANDLE ThreadHandle,
Status = DefaultQueryInfoBufferCheck(ThreadInformationClass,
PsThreadInfoClass,
RTL_NUMBER_OF(PsThreadInfoClass),
+ ICIF_PROBE_READ,
ThreadInformation,
ThreadInformationLength,
ReturnLength,
NULL,
- PreviousMode,
- FALSE);
+ PreviousMode);
if (!NT_SUCCESS(Status))
{
DPRINT1("NtQueryInformationThread(): Information verification class failed!
(Status -> 0x%lx , ThreadInformationClass -> %lx)\n", Status,
ThreadInformationClass);
diff --git a/ntoskrnl/se/tokencls.c b/ntoskrnl/se/tokencls.c
index 70b1ecafa79..b9d1615d23e 100644
--- a/ntoskrnl/se/tokencls.c
+++ b/ntoskrnl/se/tokencls.c
@@ -492,12 +492,12 @@ NtQueryInformationToken(
Status = DefaultQueryInfoBufferCheck(TokenInformationClass,
SeTokenInformationClass,
RTL_NUMBER_OF(SeTokenInformationClass),
+ ICIF_PROBE_READ_WRITE |
ICIF_FORCE_RETURN_LENGTH_PROBE,
TokenInformation,
TokenInformationLength,
ReturnLength,
NULL,
- PreviousMode,
- TRUE);
+ PreviousMode);
if (!NT_SUCCESS(Status))
{
DPRINT("NtQueryInformationToken() failed, Status: 0x%x\n", Status);
830
days inactive
830
days old
0 comments
1 participants
participants (1)
-
George Bișoc