[PATCH v2 xserver] dix: Prevent access to freed memory if a client kills itself.

Jamey Sharp jamey at minilop.net
Fri Sep 23 07:46:35 PDT 2011


Looks good to me!

Reviewed-by: Jamey Sharp <jamey at minilop.net>

On 9/23/11, Rami Ylimäki <rami.ylimaki at vincit.fi> wrote:
> The 'Dispatch' function accesses freed client structure if a client
> happens to kill itself in a request. For example, I have a test client
> that is used to check that it handles the XIO error correctly. The XIO
> error is generated by requesting the client to kill itself with
> XKillClient.
>
> We don't have to care about LBX specific functionality anymore because
> LBX support has been removed from the server.
>
> Signed-off-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
> Reviewed-by: Erkki Seppälä <erkki.seppala at vincit.fi>
> ---
> XPE turned out to be OK and there is no need to export functions for
> it.
>
>  dix/dispatch.c |   24 ++++++++++--------------
>  1 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/dix/dispatch.c b/dix/dispatch.c
> index 43cb4d1..b544899 100644
> --- a/dix/dispatch.c
> +++ b/dix/dispatch.c
> @@ -3263,21 +3263,17 @@ ProcKillClient(ClientPtr client)
>      }
>
>      rc = dixLookupClient(&killclient, stuff->id, client, DixDestroyAccess);
> -    if (rc == Success) {
> -	CloseDownClient(killclient);
> -	/* if an LBX proxy gets killed, isItTimeToYield will be set */
> -	if (isItTimeToYield || (client == killclient))
> -	{
> -	    /* force yield and return Success, so that Dispatch()
> -	     * doesn't try to touch client
> -	     */
> -	    isItTimeToYield = TRUE;
> -	    return Success;
> -	}
> -	return Success;
> +    if (rc == Success)
> +    {
> +        if (client == killclient)
> +        {
> +            MarkClientException(client);
> +            isItTimeToYield = TRUE;
> +        }
> +        else
> +            CloseDownClient(killclient);
>      }
> -    else
> -	return rc;
> +    return rc;
>  }
>
>  int
> --
> 1.7.1
>
>


More information about the xorg-devel mailing list