Signed-off-by: Matteo Bruni mbruni@codeweavers.com --- I don't know that we care a lot about the leak fixed in the first hunk. If we go with it, the LocalFree() ideally should be removed when we'll get rid of the todo_wine.
For bug 43310.
dlls/advapi32/tests/security.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index 51dcf90851..769328be81 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -6570,6 +6570,7 @@ static void test_system_security_access(void) /* privilege is checked on access */ err = GetSecurityInfo( hkey, SE_REGISTRY_KEY, SACL_SECURITY_INFORMATION, NULL, NULL, NULL, &sacl, &sd ); todo_wine ok( err == ERROR_PRIVILEGE_NOT_HELD, "got %u\n", err ); + LocalFree( sd );
priv.PrivilegeCount = 1; priv.Privileges[0].Luid = luid; @@ -7082,6 +7083,7 @@ static void test_token_security_descriptor(void) CloseHandle(info.hThread);
LocalFree(acl_child); + HeapFree(GetProcessHeap(), 0, sd2); LocalFree(psid);
CloseHandle(token3);
Signed-off-by: Matteo Bruni mbruni@codeweavers.com --- I agree with Sebastian (bug 38671 comment 1) in that it seems harmless. Let's make Valgrind happy anyway...
For bugs 38671, 43308, 43309.
dlls/advapi32/tests/security.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index 769328be81..d5e898ca8a 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -3559,7 +3559,7 @@ static void test_CreateDirectoryA(void) sa.bInheritHandle = TRUE; InitializeSecurityDescriptor(pSD, SECURITY_DESCRIPTOR_REVISION); pCreateWellKnownSid(WinBuiltinAdministratorsSid, NULL, admin_sid, &sid_size); - pDacl = HeapAlloc(GetProcessHeap(), 0, 100); + pDacl = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, 100); bret = InitializeAcl(pDacl, 100, ACL_REVISION); ok(bret, "Failed to initialize ACL.\n"); bret = pAddAccessAllowedAceEx(pDacl, ACL_REVISION, OBJECT_INHERIT_ACE|CONTAINER_INHERIT_ACE, @@ -6306,6 +6306,7 @@ static void test_AddMandatoryAce(void) HeapFree(GetProcessHeap(), 0, sd2); CloseHandle(handle);
+ memset(buffer_acl, 0, sizeof(buffer_acl)); ret = InitializeAcl(acl, 256, ACL_REVISION); ok(ret, "InitializeAcl failed with %u\n", GetLastError());
@@ -6793,6 +6794,7 @@ static void test_maximum_allowed(void)
ret = InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION); ok(ret, "InitializeSecurityDescriptor failed with %u\n", GetLastError()); + memset(buffer_acl, 0, sizeof(buffer_acl)); ret = InitializeAcl(acl, 256, ACL_REVISION); ok(ret, "InitializeAcl failed with %u\n", GetLastError()); ret = SetSecurityDescriptorDacl(sd, TRUE, acl, FALSE); @@ -6922,6 +6924,7 @@ static void test_token_security_descriptor(void) ret = InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION); ok(ret, "InitializeSecurityDescriptor failed with error %u\n", GetLastError());
+ memset(buffer_acl, 0, sizeof(buffer_acl)); ret = InitializeAcl(acl, 256, ACL_REVISION); ok(ret, "InitializeAcl failed with error %u\n", GetLastError());
On 2018-01-10 22:59, Matteo Bruni wrote:
@@ -6570,6 +6570,7 @@ static void test_system_security_access(void) /* privilege is checked on access */ err = GetSecurityInfo( hkey, SE_REGISTRY_KEY, SACL_SECURITY_INFORMATION, NULL, NULL, NULL, &sacl, &sd ); todo_wine ok( err == ERROR_PRIVILEGE_NOT_HELD, "got %u\n", err );
- LocalFree( sd );
It may be better to only do this if (!err). Otherwise you might introduce heap corruption on Windows, since sd is uninitialized.
Best, Thomas
2018-01-10 23:22 GMT+01:00 Thomas Faber thomas.faber@reactos.org:
On 2018-01-10 22:59, Matteo Bruni wrote:
@@ -6570,6 +6570,7 @@ static void test_system_security_access(void) /* privilege is checked on access */ err = GetSecurityInfo( hkey, SE_REGISTRY_KEY, SACL_SECURITY_INFORMATION, NULL, NULL, NULL, &sacl, &sd ); todo_wine ok( err == ERROR_PRIVILEGE_NOT_HELD, "got %u\n", err );
- LocalFree( sd );
It may be better to only do this if (!err). Otherwise you might introduce heap corruption on Windows, since sd is uninitialized.
Eh, good point. I'll resend with that change.
Signed-off-by: Matteo Bruni mbruni@codeweavers.com --- v2: Don't call LocalFree() with an uninitialized argument.
Alternatively we could just ignore that Wine-only leak.
dlls/advapi32/tests/security.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index 51dcf90851..791aef1879 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -6570,6 +6570,8 @@ static void test_system_security_access(void) /* privilege is checked on access */ err = GetSecurityInfo( hkey, SE_REGISTRY_KEY, SACL_SECURITY_INFORMATION, NULL, NULL, NULL, &sacl, &sd ); todo_wine ok( err == ERROR_PRIVILEGE_NOT_HELD, "got %u\n", err ); + if (err == ERROR_SUCCESS) + LocalFree( sd );
priv.PrivilegeCount = 1; priv.Privileges[0].Luid = luid; @@ -7082,6 +7084,7 @@ static void test_token_security_descriptor(void) CloseHandle(info.hThread);
LocalFree(acl_child); + HeapFree(GetProcessHeap(), 0, sd2); LocalFree(psid);
CloseHandle(token3);
-
Matteo Bruni -
Matteo Bruni -
Thomas Faber