2009/11/9 Dan Kegel <d...@kegel.com>:
> I've never used the security apis, so I'm pretty unfamiliar with them.
> Valgrinding chromium's sandbox_unittests.exe shows the leak
>
> 16 bytes in 1 blocks are definitely lost in loss record 123 of 728
> at RtlAllocateHeap (heap.c:1423)
> by RtlAllocateAndInitializeSid (sec.c:156)
> by NtQueryInformationToken (nt.c:379)
> by GetTokenInformation (security.c:676)
> by ATL::CAccessToken::GetInfoConvert<ATL::CSid,_TOKEN_PRIMARY_GROUP>
> (atlsecurity.h:754)
> by ATL::CAccessToken::GetPrimaryGroup (atlsecurity.inl:3623)
> by sandbox::RestrictedTokenTest_CustomInit_Test::TestBody
> (restricted_token_unittest.cc:92)
>
> The test in question is on line 92 of
> http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/src/restricted_token_unittest.cc?revision=11651
>
> Can someone familiar with this stuff have a look?
> I suspect it's a problem with the test, but
> can't figure out what's going on. I would
> have thought the destructors for ATL::CAccessToken
> and ATL::CSid would have freed everything.
> (I think the code leaks token_handle, but fixing
> that doesn't get rid of the reported leak.)
Hi Dan,
The bug is in the Wine code. Try the attached patch, which should fix
it as a side-effect.
Thanks,
--
Rob Shearman
From 766547300875c677ea9cfa6b3a10a3fb717c68ee Mon Sep 17 00:00:00 2001
From: Robert Shearman <robertshear...@gmail.com>
Date: Mon, 27 Apr 2009 18:53:15 +0100
Subject: [PATCH] server: Extend get_token_user server call to also retrieve SIDs for the token's owner or primary group.
To: wine-patches <wine-patc...@winehq.org>
---
dlls/ntdll/nt.c | 51 ++++++++++++++++++++++++++++-----------------------
server/protocol.def | 9 +++++----
server/token.c | 45 +++++++++++++++++++++++++++++++++++++--------
3 files changed, 70 insertions(+), 35 deletions(-)
diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c
index b29a8ae..bc01a4f 100644
--- a/dlls/ntdll/nt.c
+++ b/dlls/ntdll/nt.c
@@ -257,12 +257,6 @@ NTSTATUS WINAPI NtQueryInformationToken(
switch (tokeninfoclass)
{
- case TokenOwner:
- len = sizeof(TOKEN_OWNER) + sizeof(SID);
- break;
- case TokenPrimaryGroup:
- len = sizeof(TOKEN_PRIMARY_GROUP);
- break;
case TokenSource:
len = sizeof(TOKEN_SOURCE);
break;
@@ -287,16 +281,17 @@ NTSTATUS WINAPI NtQueryInformationToken(
switch (tokeninfoclass)
{
case TokenUser:
- SERVER_START_REQ( get_token_user )
+ SERVER_START_REQ( get_token_sid )
{
TOKEN_USER * tuser = tokeninfo;
PSID sid = tuser + 1;
DWORD sid_len = tokeninfolength < sizeof(TOKEN_USER) ? 0 : tokeninfolength - sizeof(TOKEN_USER);
req->handle = wine_server_obj_handle( token );
+ req->which_sid = tokeninfoclass;
wine_server_set_reply( req, sid, sid_len );
status = wine_server_call( req );
- if (retlen) *retlen = reply->user_len + sizeof(TOKEN_USER);
+ if (retlen) *retlen = reply->sid_len + sizeof(TOKEN_USER);
if (status == STATUS_SUCCESS)
{
tuser->User.Sid = sid;
@@ -372,17 +367,21 @@ NTSTATUS WINAPI NtQueryInformationToken(
break;
}
case TokenPrimaryGroup:
- if (tokeninfo)
+ SERVER_START_REQ( get_token_sid )
{
TOKEN_PRIMARY_GROUP *tgroup = tokeninfo;
- SID_IDENTIFIER_AUTHORITY sid = {SECURITY_NT_AUTHORITY};
- RtlAllocateAndInitializeSid( &sid,
- 2,
- SECURITY_BUILTIN_DOMAIN_RID,
- DOMAIN_ALIAS_RID_ADMINS,
- 0, 0, 0, 0, 0, 0,
- &(tgroup->PrimaryGroup));
+ PSID sid = tgroup + 1;
+ DWORD sid_len = tokeninfolength < sizeof(TOKEN_PRIMARY_GROUP) ? 0 : tokeninfolength - sizeof(TOKEN_PRIMARY_GROUP);
+
+ req->handle = wine_server_obj_handle( token );
+ req->which_sid = tokeninfoclass;
+ wine_server_set_reply( req, sid, sid_len );
+ status = wine_server_call( req );
+ if (retlen) *retlen = reply->sid_len + sizeof(TOKEN_PRIMARY_GROUP);
+ if (status == STATUS_SUCCESS)
+ tgroup->PrimaryGroup = sid;
}
+ SERVER_END_REQ;
break;
case TokenPrivileges:
SERVER_START_REQ( get_token_privileges )
@@ -398,15 +397,21 @@ NTSTATUS WINAPI NtQueryInformationToken(
SERVER_END_REQ;
break;
case TokenOwner:
- if (tokeninfo)
+ SERVER_START_REQ( get_token_sid )
{
- TOKEN_OWNER *owner = tokeninfo;
- PSID sid = owner + 1;
- SID_IDENTIFIER_AUTHORITY localSidAuthority = {SECURITY_NT_AUTHORITY};
- RtlInitializeSid(sid, &localSidAuthority, 1);
- *(RtlSubAuthoritySid(sid, 0)) = SECURITY_INTERACTIVE_RID;
- owner->Owner = sid;
+ TOKEN_OWNER *towner = tokeninfo;
+ PSID sid = towner + 1;
+ DWORD sid_len = tokeninfolength < sizeof(TOKEN_OWNER) ? 0 : tokeninfolength - sizeof(TOKEN_OWNER);
+
+ req->handle = wine_server_obj_handle( token );
+ req->which_sid = tokeninfoclass;
+ wine_server_set_reply( req, sid, sid_len );
+ status = wine_server_call( req );
+ if (retlen) *retlen = reply->sid_len + sizeof(TOKEN_OWNER);
+ if (status == STATUS_SUCCESS)
+ towner->Owner = sid;
}
+ SERVER_END_REQ;
break;
case TokenImpersonationLevel:
SERVER_START_REQ( get_token_impersonation_level )
diff --git a/server/protocol.def b/server/protocol.def
index a0e1702..b99b911 100644
--- a/server/protocol.def
+++ b/server/protocol.def
@@ -2915,18 +2915,19 @@ enum message_type
VARARG(privileges,LUID_AND_ATTRIBUTES); /* privileges used during access check */
@END
-...@req(get_token_user)
+...@req(get_token_sid)
obj_handle_t handle; /* handle to the token */
+ unsigned int which_sid; /* which SID to retrieve from the token */
@REPLY
- data_size_t user_len; /* length needed to store user */
- VARARG(user,SID); /* sid of the user the token represents */
+ data_size_t sid_len; /* length needed to store sid */
+ VARARG(sid,SID); /* the sid specified by which_sid from the token */
@END
@REQ(get_token_groups)
obj_handle_t handle; /* handle to the token */
@REPLY
data_size_t user_len; /* length needed to store user */
- VARARG(user,token_groups); /* groups the token's user belongs to */
+ VARARG(user,token_groups); /* groups the token's user belongs to */
@END
@REQ(get_token_default_dacl)
diff --git a/server/token.c b/server/token.c
index ce896ac..cf0750c 100644
--- a/server/token.c
+++ b/server/token.c
@@ -1225,27 +1225,56 @@ DECL_HANDLER(access_check)
}
/* retrieves the SID of the user that the token represents */
-DECL_HANDLER(get_token_user)
+DECL_HANDLER(get_token_sid)
{
struct token *token;
- reply->user_len = 0;
+ reply->sid_len = 0;
if ((token = (struct token *)get_handle_obj( current->process, req->handle,
TOKEN_QUERY,
&token_ops )))
{
- const SID *user = token->user;
+ const SID *sid = NULL;
+
+ switch (req->which_sid)
+ {
+ case TokenUser:
+ assert(token->user);
+ sid = token->user;
+ break;
+ case TokenPrimaryGroup:
+ sid = token->primary_group;
+ break;
+ case TokenOwner:
+ {
+ struct group *group;
+ LIST_FOR_EACH_ENTRY( group, &token->groups, struct group, entry )
+ {
+ if (group->owner)
+ {
+ sid = &group->sid;
+ break;
+ }
+ }
+ break;
+ }
+ default:
+ set_error( STATUS_INVALID_PARAMETER );
+ goto cleanup;
+ }
- reply->user_len = FIELD_OFFSET(SID, SubAuthority[user->SubAuthorityCount]);
- if (reply->user_len <= get_reply_max_size())
+ if (sid)
+ reply->sid_len = FIELD_OFFSET(SID, SubAuthority[sid->SubAuthorityCount]);
+ if (reply->sid_len <= get_reply_max_size())
{
- SID *user_reply = set_reply_data_size( reply->user_len );
- if (user_reply)
- memcpy( user_reply, user, reply->user_len );
+ SID *sid_reply = set_reply_data_size( reply->sid_len );
+ if (sid_reply)
+ memcpy( sid_reply, sid, reply->sid_len );
}
else set_error( STATUS_BUFFER_TOO_SMALL );
+cleanup:
release_object( token );
}
}
--
1.5.4.3
