[systemd-devel] [PATCH 3/3] logind: session: Fix not allowing more than one controller

[David Herrmann]

Hi

On Mon, Aug 11, 2014 at 4:52 PM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Mon, 11.08.14 16:46, Olivier Brunel (jjk at jjacky.com) wrote:
>
>>
>> On 08/11/14 16:34, Lennart Poettering wrote:
>> > On Fri, 08.08.14 20:45, Olivier Brunel (jjk at jjacky.com) wrote:
>> >
>> >> While a session can only ever have one controller, there can be more than
>> >> one session with a controller at a time. However, because of the handling
>> >> of SIGUSR1 for handling VT switch, trying to set a controller on a session
>> >> while another session had a controller would fail.
>> >
>> > I really don't feel comfortable with using SIGUSR1 for this, anyway I
>> > must say... SIGUSR1 and SIGUSR2 I think should be left for admins to
>> > communicate with the daemon for some simple operations, but using this
>> > internally sounds wrong. Now, the VT is so stupid to require a signa
>> > handler here, but I think using SIGRTMIN+1 or so might be the better
>> > choice here?
>> >
>> > Now, what makes we wonder here, shouldn't we just install a single
>> > signal event handler for this when logind initializes, and leaving it on
>> > until the very end?
>>
>> Just to note: the problem is that when the signal is called, there's no
>> way of telling which VT it is about. I think it was only intended for
>> one process to handle one VT, so there was no question. But if logind
>> wants to handle more than one VT, to know on which VT to operate then
>> I'm not sure it would actually be doable unless using a different signal
>> per VT...
>
> The VT is such a crap. Gah...

It is!

> Maybe we can iterate through the VTs we have fds for and simply
> acknowledge all of them? Wouldn't that work?
>
> With rtsigs we actually have the option to allocate one signal per vt if
> we want to.
>
> But anyway, David really shouild comment on this. David?

No-one ran multiple "new" sessions so far, so we didn't notice this.
But yeah, it's a real bug. What we should do is:

1) Register the signal handler globally (eg., logind.c).
2) Use some RT signal instead of SIGUSR1
3) When the signal is received, we should call VT_RELDISP on
seat0->active->vtfd in case it is >=0.

I will prepare a patch.

Thanks
David