Re: Added TokenOwner and TokenPrimaryGroup cases to NtSetInformationToken.

[Vitaliy Margolen]

On 10/30/2010 06:12 AM, Sami Aario wrote:
> Added TokenOwner and TokenPrimaryGroup cases to NtSetInformationToken.
>
> From: Sami Aario <saempy@hima.(none)>

Your e-mail isn't set correctly in git configuration.
> dlls/ntdll/nt.c                |   63 ++++++++++++++++++++++++++++++++++++++++
>  include/wine/server_protocol.h |   18 +++++++++++-
>  server/protocol.def            |    6 ++++
>  server/request.h               |    5 +++
>  server/token.c                 |   36 +++++++++++++++++++++++
>  server/trace.c                 |   10 ++++++

Please don't include changes to automatically generated files 
(server_protocol.h, request.h, trace.c).
> -
>    SERVER_START_REQ( set_token_sid )
> -
>    {
> -
>        SID *sid = ((TOKEN_OWNER *)TokenInformation)->Owner;
> -
>        WORD size;
> -
> -
>        if (sid) size = RtlLengthRequiredSid(sid->SubAuthorityCount);
> -
>        else size = 0;
> -
> -
>        TRACE("size=%d\n", size);
> -
> -
>        req->handle = wine_server_obj_handle( TokenHandle );
> -
>        req->which_sid = TokenInformationClass;
> -
>        wine_server_add_data( req, sid, size );
> -
>        ret = wine_server_call( req );
> -
>    }
> -
>    SERVER_END_REQ;

Please get everything outside of SERVER_START_REQ()/SERVER_END_REQ except 
actual wine_server function calls and handling of returned result(s). Some 
functions can do their own server calls and you will have a deadlock.
> -
>    case TokenOwner:
> -
>        free( token->user );
> -
>        token->user = NULL;
> -
> -
>        if (sid_len)
> -
>            token->user = memdup( sid, sid_len );
> -
>        break;

This is wrong, you can't clear token user - it always have to be set, or you 
risking an assert in get_token_sid. Which means you have to validate the 
data user passes in.

Vitaliy.