Re: (resend) [PATCH 1/2] ntdll: add stub implementation for ProcessQuotaLimits

[Vijay Kiran Kamuju]

On Fri, Mar 20, 2020 at 2:08 PM Zhiyi Zhang  wrote:
>      break;
> - case ProcessQuotaLimits:
> -
>    {
> -
>        QUOTA_LIMITS qlimits;
> -
> -
>        if (ProcessInformationLength >= sizeof(QUOTA_LIMITS))
> -
>        {
> -
>            if (!ProcessInformation)
> -
>                ret = STATUS_ACCESS_VIOLATION;
> -
>            else if (!ProcessHandle)
> -
>                ret = STATUS_INVALID_HANDLE;
> -
>            else
> -
>            {
> -
>
> Please remove this empty line.

Will incorporate this fix
> -
>                qlimits.PagedPoolLimit = (SIZE_T)-1;
> -
>                qlimits.NonPagedPoolLimit = (SIZE_T)-1;
> -
>                /* Default minimum working set size is 204800 bytes (50 Pages) */
> -
>                qlimits.MinimumWorkingSetSize = 204800;
> -
>                /* Default maximum working set size is 1413120 bytes (345 Pages) */
> -
>                qlimits.MaximumWorkingSetSize = 1413120;
> -
>                qlimits.PagefileLimit = (SIZE_T)-1;
> -
>                qlimits.TimeLimit.QuadPart = -1;
>
> Please test against these hard-coded values. And add a FIXME here because they should be queried from process and SetProcessWorkingSetSize() can change them.

I will put a FIXME, saying that its a semistub. And also put a comment
that these values that SetProcessWorkingSetSize can change these
values.
PagedPoolLimit, NonPagedPoolLimit always changes from windows version.
MinimumWorkingSetSize,MaximumWorkingSetSize are using the default
values, it may differ from system to system based on available
Physical RAM.
PagefileLimit and TimeLimit can be tested, if you want I can add
checks for them as they are consistent across versions.
This is a semi-stub implementation, reporting default values.
> -
>                memcpy(ProcessInformation, &qlimits, sizeof(QUOTA_LIMITS));
>
> Please use sizeof(qlimits) instead.

Will incorporate this fix.
> -
> -
>                len = sizeof(QUOTA_LIMITS);
> -
>            }
> -
> -
>            if (ProcessInformationLength > sizeof(QUOTA_LIMITS))
> -
>                ret = STATUS_INFO_LENGTH_MISMATCH;
>
> You should check ProcessInformationLength == sizeof(QUOTA_LIMITS) and remove this. You didn't
> correctly test the required size. You should test size of sizeof(QUOTA_LIMITS)-/+1

Will add more tests for size = sizeof(QUOTA_LIMITS)+1 and
sizeof(QUOTA_LIMITS)-1.
Currently have a test for sizeof(QUOTA_LIMITS)*2, I will change it.
> -
>        }
> -
>        else
> -
>        {
> -
>            len = sizeof(QUOTA_LIMITS);
> -
>            ret = STATUS_INFO_LENGTH_MISMATCH;
> -
>        }
> -
>    }
> -
>    break;
> case ProcessIoCounters:
>        {
>            IO_COUNTERS pii;

Thanks for the review