[PATCH V2] advapi32/tests: Add additional tests for NtAccessCheck

[Vijay Kiran Kamuju]

On Wed, Apr 24, 2019 at 5:27 PM Zhiyi Zhang <zzhang at codeweavers.com> wrote:
>
>
>
> On 4/24/19 11:06 PM, Vijay Kiran Kamuju wrote:
> > Add todo_wine for failing tests
> >
> > From: Vijay Kiran Kamuju <infyquest at gmail.com>
> > Signed-off-by: Vijay Kiran Kamuju <infyquest at gmail.com>
> > ---
> >  dlls/advapi32/tests/security.c | 35 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
> > index d9cae64da8b..7534e007915 100644
> > --- a/dlls/advapi32/tests/security.c
> > +++ b/dlls/advapi32/tests/security.c
> > @@ -1343,11 +1343,13 @@ static void test_AccessCheck(void)
> >
> >      if(pNtAccessCheck)
> >      {
> > +       DWORD ntPrivSetLen = sizeof(PRIVILEGE_SET);
> > +
> >         /* Generic access mask - no privilegeset buffer */
> >         SetLastError(0xdeadbeef);
> >         Access = ntAccessStatus = 0x1abe11ed;
> >         ntret = pNtAccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping,
> > -                              NULL, &PrivSetLen, &Access, &ntAccessStatus);
> > +                              NULL, &ntPrivSetLen, &Access, &ntAccessStatus);
> >         err = GetLastError();
> >         ok(ntret == STATUS_ACCESS_VIOLATION,
> >            "NtAccessCheck should have failed with STATUS_ACCESS_VIOLATION, got %x\n", ntret);
> > @@ -1355,6 +1357,7 @@ static void test_AccessCheck(void)
> >            "NtAccessCheck shouldn't set last error, got %d\n", err);
> >         ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
> >            "Access and/or AccessStatus were changed!\n");
> > +       ok(ntPrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", ntPrivSetLen);
> >
> >        /* Generic access mask - no returnlength */
> >        SetLastError(0xdeadbeef);
> > @@ -1381,6 +1384,36 @@ static void test_AccessCheck(void)
> >           "NtAccessCheck shouldn't set last error, got %d\n", err);
> >        ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
> >           "Access and/or AccessStatus were changed!\n");
> > +
> > +      /* Generic access mask - zero returnlength */
> > +      SetLastError(0xdeadbeef);
> > +      Access = ntAccessStatus = 0x1abe11ed;
> > +       ntPrivSetLen = 0;
> Formating.
Will change the formating.
>
> > +      ntret = pNtAccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping,
> > +                             PrivSet, &ntPrivSetLen, &Access, &ntAccessStatus);
> > +      err = GetLastError();
> > +      ok(ntret == STATUS_GENERIC_NOT_MAPPED,
> > +         "NtAccessCheck should have failed with STATUS_GENERIC_NOT_MAPPED, got %x\n", ntret);
> > +      ok(err == 0xdeadbeef,
> > +         "NtAccessCheck shouldn't set last error, got %d\n", err);
> > +      ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
> > +         "Access and/or AccessStatus were changed!\n");
> > +      todo_wine ok(ntPrivSetLen == 0, "PrivSetLen returns %d\n", ntPrivSetLen);
> > +
> > +      /* Generic access mask - insufficient returnlength */
> > +      SetLastError(0xdeadbeef);
> > +      Access = ntAccessStatus = 0x1abe11ed;
> > +       ntPrivSetLen = sizeof(PRIVILEGE_SET)-1;
> > +      ntret = pNtAccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping,
> > +                             PrivSet, &ntPrivSetLen, &Access, &ntAccessStatus);
> > +      err = GetLastError();
> > +      ok(ntret == STATUS_GENERIC_NOT_MAPPED,
> This doesn't look right. You are testing the return length. Make sure other parameters are valid.
> This way we know the error is indeed from the invalid length.
It seems the test is getting STATUS_GENERIC_NOT_MAPPED because its
GENERIC_READ, I will add another test for KEY_READ.
That should help us understand the behavior further.
>
> And I think we should move NtAccessCheck test into ntdll tests if it is not too much trouble.
I will move the tests once I try my tests on testbot.
> > +         "NtAccessCheck should have failed with STATUS_GENERIC_NOT_MAPPED, got %x\n", ntret);
> > +      ok(err == 0xdeadbeef,
> > +         "NtAccessCheck shouldn't set last error, got %d\n", err);
> > +      ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
> > +         "Access and/or AccessStatus were changed!\n");
> > +      todo_wine ok(ntPrivSetLen == sizeof(PRIVILEGE_SET)-1, "PrivSetLen returns %d\n", ntPrivSetLen);
> >      }
> >      else
> >         win_skip("NtAccessCheck unavailable. Skipping.\n");
>