On 08/11/14 17:12, David Herrmann wrote: > Hi > > On Mon, Aug 11, 2014 at 5:05 PM, Olivier Brunel <[email protected]> wrote: >> On 08/11/14 16:54, Lennart Poettering wrote: >>> On Mon, 11.08.14 16:39, Olivier Brunel ([email protected]) wrote: >>> >>>> >>>> On 08/11/14 16:25, Lennart Poettering wrote: >>>>> On Fri, 08.08.14 20:45, Olivier Brunel ([email protected]) 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 > _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
