[PATCH xserver] os: Treat ssh as a non-local client (v3)

[Martin Peres]

On 13/01/16 03:59, Michel Dänzer wrote:
> On 13.01.2016 03:43, Adam Jackson wrote:
>> On Thu, 2015-12-17 at 16:41 +0900, Michel Dänzer wrote:
>>> From: Adam Jackson <ajax at redhat.com>
>>>
>>> By the time we get to ComputeLocalClient, we've already done
>>> NextAvailableClient → ReserveClientIds → DetermineClientCmd (assuming
>>> we're built with #define CLIENTIDS), so we can look up the name of the
>>> client process and refuse to treat ssh's X forwarding as if it were
>>> local.
>>>
>>> v2: (Michel Dänzer)
>>>      * Only match "ssh" itself, not other executable names starting with
>>>        that prefix.
>>>      * Ignore executable path for the match.
>>> v3: (Michel Dänzer)
>>>      * Use GetClientCmdName (Mark Kettenis)
>>>      * Perform check on Windows as well, but only ignore path on Cygwin
>>>        (Martin Peres, Emil Velikov, Jon Turney)
>>
>> But this doesn't work reliably, which is why I used strncmp in the
>> first place. See my original test report:
>>
>> http://lists.freedesktop.org/archives/xorg-devel/2015-December/048164.html
>
> I somehow missed that before, sorry.
>
> Looks like this could be addressed by cutting of the colon and anything
> after it using something like strtok(_r)?
>
>
>> Note that cmdname is not always "ssh".
>
> Indeed, that's one of the points which prompted my changes. E.g. it can
> contain the executable path, which is stripped off with basename. That
> might not work too well either with your example above without cutting
> off at the colon.
>
>
>> Are we honestly concerned that some client's name is prefixed with
>> "ssh" and _isn't_ ssh or something acting like it?
>
> That seems quite plausible. Debian is already shipping many more
> executables starting with "ssh", that number will probably only increase
> with time. Maybe none of them need DRI2/3 yet, but we shouldn't lightly
> rely on that being the case for any future ones.

How about we also add a message in the logs to say that we treat this 
client as non-local? That seems to be a good idea, especially when we 
could have false positives.