[PATCH xserver] os: Fix strtok/free crash in ComputeLocalClient

[Michel Dänzer]

On 2017-12-07 11:19 AM, walter harms wrote:
> Am 06.12.2017 12:16, schrieb Tomasz Śniatowski:
>> Don't reuse cmd for strtok output to ensure the proper pointer is
>> freed afterwards.
>>
>> The code incorrectly assumed the pointer returned by strtok(cmd, ":")
>> would always point to cmd. However, strtok(str, sep) != str if str
>> begins with sep. This caused an invalid-free crash when running
>> a program under X with a name beginning with a colon.
>>
>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=104123
>>
> 
> Yes, strtok returns a pointer to its arguments.
> btw: strtok is not thread-safe, could that be an issue with that function ?

Hmm. My initial answer was no, since ComputeLocalClient should only be
called on the main thread. But if there can be a race with strtok
getting called from other places, there might be an issue. Anyway,
that's not something introduced by this patch but could be addressed in
another patch. This patch is

Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>


>> Signed-off-by: Tomasz Śniatowski <kailoran at gmail.com>
>> ---
>>  os/access.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/os/access.c b/os/access.c
>> index 8828e0834..97246160c 100644
>> --- a/os/access.c
>> +++ b/os/access.c
>> @@ -1137,12 +1137,12 @@ ComputeLocalClient(ClientPtr client)
>>          /* Cut off any colon and whatever comes after it, see
>>           * https://lists.freedesktop.org/archives/xorg-devel/2015-December/048164.html
>>           */
>> -        cmd = strtok(cmd, ":");
>> +        char *tok = strtok(cmd, ":");
>>  
>>  #if !defined(WIN32) || defined(__CYGWIN__)
>> -        ret = strcmp(basename(cmd), "ssh") != 0;
>> +        ret = strcmp(basename(tok), "ssh") != 0;
>>  #else
>> -        ret = strcmp(cmd, "ssh") != 0;
>> +        ret = strcmp(tok, "ssh") != 0;
>>  #endif
>>  
>>          free(cmd);
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
> 


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer