[systemd-devel] [PATCH 1/3] logind: session: don't set /dev/ttyX owner to root on restore_vt

Olivier Brunel jjk at jjacky.com
Mon Aug 11 08:37:26 PDT 2014


On 08/11/14 17:12, David Herrmann wrote:
> Hi
> 
> On Mon, Aug 11, 2014 at 5:05 PM, Olivier Brunel <jjk at jjacky.com> wrote:
>> On 08/11/14 16:54, Lennart Poettering wrote:
>>> On Mon, 11.08.14 16:39, Olivier Brunel (jjk at jjacky.com) wrote:
>>>
>>>>
>>>> On 08/11/14 16:25, Lennart Poettering wrote:
>>>>> On Fri, 08.08.14 20:45, Olivier Brunel (jjk at jjacky.com) wrote:
>>>>>
>>>>>> In session_prepare_vt() we set owner of /dev/ttyX to the user, as that is
>>>>>> needed for things to work. However, we shouldn't "reset" it to root on
>>>>>> session_restore_vt() since it could have in fact already been set to
>>>>>> the user.
>>>>>
>>>>> I don't follow here, can't parse this. Could you please elaborate?
>>>>
>>>> I meant, before the call to session_prepare_vt() the owner of /dev/ttyX
>>>> might not be root, but already set to the user. In which case setting it
>>>> "back" to root might not be expected/best.
>>>
>>> But that sounds more as if session_restore_vt() should not be used as-is
>>> as cleanup path for session_prepare_vt(), no?
>>>
>>>> E.g. if a log in on tty3 right now, /dev/tty3 will be owned by my user,
>>>> then if a process try a TakeControl() and it failed (or after it's done)
>>>> the ownership would be set to root, even though it wasn't actually root
>>>> to begin with.
>>>
>>> Isn't this very theoretic? I mean, when does TakeControl() actually
>>> really fail for you IRL?
>>
>> Right, this was noticed when trying to start a second rootless X, since
>> with systemd-215 things currently fail, and the ownership of /dev/ttyX
>> would then be switched/changed to root.
> 
> Wait, what? Can you please elaborate. Currently, only one process can

Sorry, I meant e.g. having one rootless X on tt1 and starting another
one on tty2. Currently this fails (see other patch/mail), and is how
this was observed.

> be controller at a time, and session_prepare_vt() is called *after*
> the controller is set. Therefore, it is not called when you try
> starting a second controller.
> 
> The only race I see is this:
>  * start legacy Xserver (which doesn't use TakeControl()) which sets
> user-id on the TTY
>  * start new Xserver which uses TakeControl() (and thus calls
> session_prepare_vt())
>  * stop new Xserver (thus drop Control and reset the TTY)
> 
> In this case, the new xserver will try to start up, but fail as the
> old one is still running. Therefore, it *might* call ReleaseControl()
> and thus reset the TTY. However, you're not supposed to mix both and
> this is not a legitimate use-case. I mean, the old server is root and
> modifies the TTY by itself (without using systemd). Obviously, this is
> racy.
> 
> From a systemd-logind perspective, this is the same as if you run
> "sudo chown xyz /dev/tty" manually.
> 
>> session_prepare_vt() could also check the owner first, and note what to
>> reset to on session_restore_vt(), to effectively restore things as they
>> were.
> 
> Even though I don't see any problem here, I'd be fine with such a
> patch. Care to send one? Note that you probably need to store that
> information in the session-file (session_save() / session_load()) too.

If you think the current behavior (setting to root on
session_restore_vt()) is fine, I'm fine leaving it as is. Else, I could
work on a patch as described.

> 
> Thanks
> David
> 



More information about the systemd-devel mailing list