[ros-diffs] [reactos] 03/11: [NTOS:FSRTL] Assign the buffer length to ThisBufferLength field

[George Bișoc]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=abe89d7cdecd173628f6f…

commit abe89d7cdecd173628f6fa67819547887a085664
Author:     George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sat Jan 1 20:23:28 2022 +0100
Commit:     George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Tue Jan 11 10:11:08 2022 +0100

    [NTOS:FSRTL] Assign the buffer length to ThisBufferLength field
    
    This fixes an issue where ReactOS would assert on QuotaUsage == 0 as the process was
still taking up quotas during a quota block de-reference with root cause of
ThisBufferLength member field being 0 which made process quota charging/returning flow
unbalanced.
    In addition to that, on FsRtlCancelNotify routine API all we must ensure that if
PsChargePoolQuota or ExAllocatePoolWithTag function fails we have to handle the raised
exceptions accordingly and return the charged quota back (if we actually charged quotas
that is). With said, wrap that part of code with SEH.
    
    === DOCUMENTATION REMARKS ===
    The cause of the assert is due to the fact ThisBufferLength was being handled wrongly
ever since, until this commit. When FsRtl of the Executive has to filter reported changes
(with logic algorithm implemented in FsRtlNotifyFilterReportChange function), the said
function will charge the quota of a given process
    with an amount that is represented as the buffer length whose size is expressed in
bytes. This length buffer is preserved in a local variable called NumberOfBytes, which is
initialized from BufferLength member field of notification structure or from the length
from stack parameters pointed from an IRP.
    
    As it currently stands, the code is implemented in such a way that
FsRtlNotifyFilterReportChange will charge quotas to a process but it doesn't assign
the buffer length to ThisBufferLength. On the first glimpse ThisBufferLength and
BufferLength are very similar members that serve exact same purpose but in reality
there's a subtle distinction between the two.
    
    BufferLength is a member whose length size is given by FSDs (filesystem drivers)
during a notification dispatching. Whenever FsRtl receives the notification structure
packed with data from the filesystem, the length pointed by BufferLength gets passed to
ThisBufferLength and from now on the kernel has to use this member for the whole time of
its task to accomplish
    whatever request it's been given by the filesystem. In other words, BufferLength
is strictly used only to pass length size data to the kernel by initializing
ThisBufferLength based on that length and unequivocally the kernel uses this member field.
What we're doing is that ThisBufferLength never receives the length from BufferLength
therefore whenever FsRtl component
    has to return quotas back it'll return an amount of 0 (which means no amount to
return) and that's a bug in the kernel.
---
 ntoskrnl/fsrtl/notify.c | 49 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/ntoskrnl/fsrtl/notify.c b/ntoskrnl/fsrtl/notify.c
index e7b73771232..f69475c9984 100644
--- a/ntoskrnl/fsrtl/notify.c
+++ b/ntoskrnl/fsrtl/notify.c
@@ -85,6 +85,7 @@ FsRtlCancelNotify(IN PDEVICE_OBJECT DeviceObject,
     PIO_STACK_LOCATION Stack;
     PNOTIFY_CHANGE NotifyChange;
     PREAL_NOTIFY_SYNC RealNotifySync;
+    BOOLEAN PoolQuotaCharged;
     PSECURITY_SUBJECT_CONTEXT _SEH2_VOLATILE SubjectContext = NULL;
 
     /* Get the NOTIFY_CHANGE struct and reset it */
@@ -165,15 +166,38 @@ FsRtlCancelNotify(IN PDEVICE_OBJECT DeviceObject,
                /* If we have a buffer length, but no buffer then allocate one */
                if (Buffer == NULL)
                {
-                   PsChargePoolQuota(NotifyChange->OwningProcess, PagedPool,
BufferLength);
-                   Buffer = ExAllocatePoolWithTag(PagedPool |
POOL_RAISE_IF_ALLOCATION_FAILURE, BufferLength, TAG_FS_NOTIFICATIONS);
-                   NotifyChange->AllocatedBuffer = Buffer;
-               }
+                   /* Assume we haven't charged quotas */
+                   PoolQuotaCharged = FALSE;
 
-               /* Copy data in that buffer */
-               RtlCopyMemory(Buffer, NotifyChange->Buffer,
NotifyChange->DataLength);
-               NotifyChange->ThisBufferLength = BufferLength;
-               NotifyChange->Buffer = Buffer;
+                   _SEH2_TRY
+                   {
+                       /* Charge quotas */
+                       PsChargePoolQuota(NotifyChange->OwningProcess, PagedPool,
BufferLength);
+                       PoolQuotaCharged = TRUE;
+
+                       /* Allocate buffer */
+                       Buffer = ExAllocatePoolWithTag(PagedPool |
POOL_RAISE_IF_ALLOCATION_FAILURE, BufferLength, TAG_FS_NOTIFICATIONS);
+                       NotifyChange->AllocatedBuffer = Buffer;
+
+                       /* Copy data in that buffer */
+                       RtlCopyMemory(Buffer, NotifyChange->Buffer,
NotifyChange->DataLength);
+                       NotifyChange->ThisBufferLength = BufferLength;
+                       NotifyChange->Buffer = Buffer;
+                   }
+                   _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
+                   {
+                       /* Something went wrong, have we charged quotas? */
+                       if (PoolQuotaCharged)
+                       {
+                           /* We did, return quotas */
+                           PsReturnProcessPagedPoolQuota(NotifyChange->OwningProcess,
BufferLength);
+                       }
+
+                       /* And notify immediately */
+                       NotifyChange->Flags |= NOTIFY_IMMEDIATELY;
+                   }
+                   _SEH2_END;
+               }
            }
 
            /* If we have to notify immediately, ensure that any buffer is 0-ed out */
@@ -1322,10 +1346,15 @@ FsRtlNotifyFilterReportChange(IN PNOTIFY_SYNC NotifySync,
                         /* If we couldn't find one, then allocate one */
                         if (NotifyChange->Buffer == NULL)
                         {
+                            /* Assign the length buffer */
+                            NotifyChange->ThisBufferLength = NumberOfBytes;
+
+                            /* Assume we have not charged quotas */
                             PoolQuotaCharged = FALSE;
                             _SEH2_TRY
                             {
-                                PsChargePoolQuota(NotifyChange->OwningProcess,
PagedPool, NumberOfBytes);
+                                /* And charge quotas */
+                                PsChargePoolQuota(NotifyChange->OwningProcess,
PagedPool, NotifyChange->ThisBufferLength);
                                 PoolQuotaCharged = TRUE;
                                 OutputBuffer = ExAllocatePoolWithTag(PagedPool |
POOL_RAISE_IF_ALLOCATION_FAILURE,
                                                                      NumberOfBytes,
TAG_FS_NOTIFICATIONS);
@@ -1337,7 +1366,7 @@ FsRtlNotifyFilterReportChange(IN PNOTIFY_SYNC NotifySync,
                             {
                                 if (PoolQuotaCharged)
                                 {
-                                   
PsReturnProcessPagedPoolQuota(NotifyChange->OwningProcess, NumberOfBytes);
+                                   
PsReturnProcessPagedPoolQuota(NotifyChange->OwningProcess,
NotifyChange->ThisBufferLength);
                                 }
                                 NotifyChange->Flags |= NOTIFY_IMMEDIATELY;
                             }