Don't return before file object's resource is acquired in FsRtlAcquireFileExclusiveCommon, except some special return cases, when return is required. Based on hpoussin_filter_extra.patch by Herve Poussineau (@hpoussin) with improved comment, which matches the actual behaviour now.
This is required by fltmgr.sys driver from Windows XP/Server 2003 to work correctly, so this change fixes asserts/exceptions when releasing the file via FsRtlReleaseFile after acquiring, when using 3rd party filter drivers from several antivirus programs (e. g., Avast Free Antivirus all versions, AVG Antivirus Free 18.8, Avira AntiVir Personal 8.2, Dr. Web Security Space 8.0, Kaspersky Antivirus 2012 etc. etc.).
CORE-14157, CORE-14635, CORE-19318
According to our declaration/definition, IoChangeFileObjectFilerContext returns NTSTATUS, not BOOLEAN. Zero return (which was actually checked before) for BOOLEAN means failure, but for NTSTATUS it's success. So it should (and now actually does) free and fail appropriately only in failure case, but not in success, when it shouldn't.
This fixes most of problems with fltmgr.sys driver from Windows XP/Server 2003 and a lot of 3rd party filter drivers which use it from many apps (Avast Free Antivirus all versions, Avira AntiVir Personal 8.2, Dr. Web Security Space 8.0, Kaspersky Antivirus 2012 etc. etc.).
CORE-14157, CORE-14635, CORE-19318
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.
We have a special file, tag.h, which serves as a place to store whatever kernel pool allocation tag yet we still have some tags sparse over the kernel code... So just re-group them in one unique place.
- Fix behaviour when adding or removing entries in the middle of an existing run
- Do not touch output parameters when failing, caller might rely on this.
- Change INIT_FUNCTION and INIT_SECTION to CODE_SEG("INIT") and DATA_SEG("INIT") respectively
- Remove INIT_FUNCTION from function prototypes
- Remove alloc_text pragma calls as they are not needed anymore
It better captures the intent now in FsRtlIsNameInExpressionPrivate and
fixes a slight overallotion by 4 bytes in FsRtlIsDbcsInExpression.
While at it, use the ANSI_DOS_DOT macro in the Dbcs version.
CORE-15902
* Add an NDK header to define INIT_FUNCTION/INIT_SECTION globally
* Use _declspec(allocate(x)) and _declspec(code_seg(x)) on MSVC versions that support it
* Use INIT_FUNCTION on functions only and INIT_SECTION on data only (required by MSVC)
* Place INIT_FUNCTION before the return type (required by MSVC)
* Make sure declarations and implementations share the same modifiers (required by MSVC)
* Add a global linker option to suppress warnings about defined but unused INIT section
* Merge INIT section into .text in freeldr
Oneliner of the day... This typo just prevented the
whole feature to work properly. Because any allocated
work item would miserably fail to be freed.
This will obviously help real world FSD relying on
StackOverflow worker from FsRtl to work better!
CORE-14611
