https://git.reactos.org/?p=reactos.git;a=commitdiff;h=03294dd09763d8d2b4979a...
commit 03294dd09763d8d2b4979a413c018ccd2a16fdde Author: Pierre Schweitzer pierre@reactos.org AuthorDate: Sat Oct 27 21:54:55 2018 +0200 Commit: Pierre Schweitzer pierre@reactos.org CommitDate: Sat Oct 27 22:16:37 2018 +0200
[NTOSKRNL] Rewrite IoCheckEaBufferValidity() so that it's less magic
And make its coding style consistent with our rules --- ntoskrnl/io/iomgr/util.c | 127 +++++++++++++++++++++++------------------------ 1 file changed, 62 insertions(+), 65 deletions(-)
diff --git a/ntoskrnl/io/iomgr/util.c b/ntoskrnl/io/iomgr/util.c index 7329dbf8f1..fb56f8d170 100644 --- a/ntoskrnl/io/iomgr/util.c +++ b/ntoskrnl/io/iomgr/util.c @@ -192,82 +192,79 @@ IoCheckEaBufferValidity(IN PFILE_FULL_EA_INFORMATION EaBuffer, IN ULONG EaLength, OUT PULONG ErrorOffset) { - PFILE_FULL_EA_INFORMATION EaBufferEnd; - ULONG NextEaBufferOffset; - LONG IntEaLength; + ULONG NextEntryOffset; + UCHAR EaNameLength; + ULONG ComputedLength; + PFILE_FULL_EA_INFORMATION Current;
PAGED_CODE();
- /* Length of the rest */ - IntEaLength = EaLength; - EaBufferEnd = EaBuffer; - - /* The rest length of the buffer */ - while (IntEaLength >= FIELD_OFFSET(FILE_FULL_EA_INFORMATION, EaName[0])) + /* We will browse all the entries */ + for (Current = EaBuffer; ; Current = (PFILE_FULL_EA_INFORMATION)((ULONG_PTR)Current + NextEntryOffset)) { - /* - * The rest of buffer must greater than - * sizeof(FILE_FULL_EA_INFORMATION) + buffer - */ - NextEaBufferOffset = - EaBufferEnd->EaNameLength + EaBufferEnd->EaValueLength + - FIELD_OFFSET(FILE_FULL_EA_INFORMATION, EaName[0]) + 1; - - if ((ULONG)IntEaLength >= NextEaBufferOffset) + /* Check that we have enough bits left for the current entry */ + if (EaLength < FIELD_OFFSET(FILE_FULL_EA_INFORMATION, EaName)) + { + goto FailPath; + } + + EaNameLength = Current->EaNameLength; + ComputedLength = Current->EaValueLength + EaNameLength + FIELD_OFFSET(FILE_FULL_EA_INFORMATION, EaName) + 1; + /* Check that we have enough bits left for storing the name and its value */ + if (EaLength < ComputedLength) + { + goto FailPath; + } + + /* Make sure the name is null terminated */ + if (Current->EaName[EaNameLength] != ANSI_NULL) + { + goto FailPath; + } + + /* Get the next entry offset */ + NextEntryOffset = Current->NextEntryOffset; + /* If it's 0, it's a termination case */ + if (NextEntryOffset == 0) { - /* is the EaBufferName terminated with zero? */ - if (EaBufferEnd->EaName[EaBufferEnd->EaNameLength]==0) + /* If we don't overflow! */ + if (EaLength - ComputedLength < 0) { - /* more EaBuffers ahead */ - if (EaBufferEnd->NextEntryOffset == 0) - { - /* test the rest buffersize */ - IntEaLength = IntEaLength - NextEaBufferOffset; - if (IntEaLength >= 0) - { - return STATUS_SUCCESS; - } - } - else - { - /* - * From MSDN: - * http://msdn2.microsoft.com/en-us/library/ms795740.aspx - * For all entries except the last one, the value of - * NextEntryOffset must be greater than zero and - * must fall on a ULONG boundary. - */ - NextEaBufferOffset = ((NextEaBufferOffset + 3) & ~3); - if ((EaBufferEnd->NextEntryOffset == NextEaBufferOffset) && - ((LONG)EaBufferEnd->NextEntryOffset > 0)) - { - /* - * The rest of buffer must be greater - * than the following offset. - */ - IntEaLength = - IntEaLength - EaBufferEnd->NextEntryOffset; - - if (IntEaLength >= 0) - { - EaBufferEnd = (PFILE_FULL_EA_INFORMATION) - ((ULONG_PTR)EaBufferEnd + - EaBufferEnd->NextEntryOffset); - continue; - } - } - } + goto FailPath; } + + break; } - break; - }
- if (ErrorOffset != NULL) - { - /* Calculate the error offset */ - *ErrorOffset = (ULONG)((ULONG_PTR)EaBufferEnd - (ULONG_PTR)EaBuffer); + /* Compare the next offset we computed with the provided one, they must match */ + if (ALIGN_UP_BY(ComputedLength, sizeof(ULONG)) != NextEntryOffset) + { + goto FailPath; + } + + /* Check next entry offset value is positive */ + if (NextEntryOffset < 0) + { + goto FailPath; + } + + /* Compute the remaining bits */ + EaLength -= NextEntryOffset; + /* We must have bits left */ + if (EaLength < 0) + { + goto FailPath; + } + + /* Move to the next entry */ }
+ /* If we end here, everything went OK */ + return STATUS_SUCCESS; + +FailPath: + /* If we end here, we failed, set failed offset */ + *ErrorOffset = (ULONG_PTR)Current - (ULONG_PTR)EaBuffer; return STATUS_EA_LIST_INCONSISTENT; }
-
Pierre Schweitzer