From: Vijay Kiran Kamuju Subject: Re: [PATCH V2] advapi32/tests: Add additional tests for NtAccessCheck Message-Id: Date: Wed, 24 Apr 2019 18:24:13 +0200 In-Reply-To: <99622a8e-ff5f-acd2-c66d-e8755f7c9044@codeweavers.com> References: <20190424150632.1012-1-infyquest@gmail.com> <99622a8e-ff5f-acd2-c66d-e8755f7c9044@codeweavers.com> On Wed, Apr 24, 2019 at 5:27 PM Zhiyi Zhang wrote: > > > > On 4/24/19 11:06 PM, Vijay Kiran Kamuju wrote: > > Add todo_wine for failing tests > > > > From: Vijay Kiran Kamuju > > Signed-off-by: Vijay Kiran Kamuju > > --- > > 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"); >