[ros-dev] [ros-diffs] [reactos] 01/01: [NTOS:MM] Fix NtFreeVirtualMemory() FreeType parameter checks. CORE-13126

[Alex Ionescu]

I'm unsure how reducing the two explicit checks into a single one is a
better decision. This is much less scalable and much less clear of what the
caller did wrong. I'd like to see the old checks (correctly) restored.

Best regards,
Alex Ionescu

On Fri, Dec 15, 2017 at 2:58 AM, Serge Gautherie <
reactos-git_serge_171003 at gautherie.fr> wrote:

> https://git.reactos.org/?p=reactos.git;a=commitdiff;h=
> 614ace03fa4ec1957f32727ab0975a98c858894d
>
> commit 614ace03fa4ec1957f32727ab0975a98c858894d
> Author: Serge Gautherie <reactos-git_serge_171003 at gautherie.fr>
> AuthorDate: Sat Dec 9 12:16:44 2017 +0100
>
>     [NTOS:MM] Fix NtFreeVirtualMemory() FreeType parameter checks.
> CORE-13126
> ---
>  ntoskrnl/mm/ARM3/virtual.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/ntoskrnl/mm/ARM3/virtual.c b/ntoskrnl/mm/ARM3/virtual.c
> index 0e5fe77a23..fc4ea2e6d2 100644
> --- a/ntoskrnl/mm/ARM3/virtual.c
> +++ b/ntoskrnl/mm/ARM3/virtual.c
> @@ -5108,21 +5108,11 @@ NtFreeVirtualMemory(IN HANDLE ProcessHandle,
>      PAGED_CODE();
>
>      //
> -    // Only two flags are supported
> +    // Only two flags are supported, exclusively.
>      //
> -    if (!(FreeType & (MEM_RELEASE | MEM_DECOMMIT)))
> +    if (FreeType != MEM_RELEASE && FreeType != MEM_DECOMMIT)
>      {
> -        DPRINT1("Invalid FreeType\n");
> -        return STATUS_INVALID_PARAMETER_4;
> -    }
> -
> -    //
> -    // Check if no flag was used, or if both flags were used
> -    //
> -    if (!((FreeType & (MEM_DECOMMIT | MEM_RELEASE))) ||
> -         ((FreeType & (MEM_DECOMMIT | MEM_RELEASE)) == (MEM_DECOMMIT |
> MEM_RELEASE)))
> -    {
> -        DPRINT1("Invalid FreeType combination\n");
> +        DPRINT1("Invalid FreeType (0x%08lx)\n", FreeType);
>          return STATUS_INVALID_PARAMETER_4;
>      }
>
> @@ -5200,8 +5190,8 @@ NtFreeVirtualMemory(IN HANDLE ProcessHandle,
>          }
>      }
>
> -    DPRINT("NtFreeVirtualMemory: Process 0x%p, Adress 0x%p, size 0x%x,
> FreeType %x.\n",
> -        Process, PBaseAddress, PRegionSize, FreeType);
> +    DPRINT("NtFreeVirtualMemory: Process 0x%p, Address 0x%p, Size 0x%Ix,
> FreeType 0x%08lx\n",
> +           Process, PBaseAddress, PRegionSize, FreeType);
>
>      //
>      // Lock the address space
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20171216/0b7b065e/attachment.html>