[PATCH] systemd-logind: do not rely on directed signals

[Hans de Goede]

Hi,

On 22-06-15 21:13, David Herrmann wrote:
> Right now, Xorg does not install DBus matches for "PauseDevice" /
> "ResumeDevice". Therefore, it should usually not receive those DBus
> signals from logind. It is just a coincidence that systemd-logind sends
> those signals in a directed manner right now. Therefore, dbus-daemon
> bypasses the broadcast matches.
>
> However, this is not ABI and Xorg should not rely on this. systemd-logind
> is free to send those signals as broadcasts, in which case Xorg will
> freeze the VT. Fix this by always installing those matches.
>
> Cc: Hans de Goede <hdegoede at redhat.com>
> Cc: Keith Packard <keithp at keithp.com>
> Reported-by: Jan Alexander Steffens <jan.steffens at gmail.com>
> Signed-off-by: David Herrmann <dh.herrmann at gmail.com>
> ---
> Hi
>
> I'd really appreciate if we could back-port this to 'stable'. The implicit
> forwarding of directed signals in dbus-daemon is quite nasty and probably needs
> to be fixed. With this patch in place, we make sure Xorg does not rely on that
> feature and is safe regarding future changes of the default dbus policy.
>
> There's an ongoing discussion regarding some possible security issues with this,
> so it's not very unlikely that _something_ will change in dbus upstream.
>
> I also assume this was just an oversight by Hans, and it was not done this way
> intentionally. Hans?

This was not intentionally, I just assumed that these signals would be directed
since that seems logical, and things worked that way. I'm fine with adding the
matches. I'll test this tomorrow, and if it works add my Reviewed-by and
Tested-by and send a pull-req with this patch + some other patches I've pending.

Thanks for working on this!

Regards,

Hans


>
> Thanks
> David
>
>   hw/xfree86/os-support/linux/systemd-logind.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c
> index 4ad41a3..a0434a1 100644
> --- a/hw/xfree86/os-support/linux/systemd-logind.c
> +++ b/hw/xfree86/os-support/linux/systemd-logind.c
> @@ -506,6 +506,24 @@ connect_hook(DBusConnection *connection, void *data)
>           goto cleanup;
>       }
>
> +    dbus_bus_add_match(connection,
> +        "type='signal',sender='org.freedesktop.login1',interface='org.freedesktop.login1.Session',member='PauseDevice'",
> +        &error);
> +    if (dbus_error_is_set(&error)) {
> +        LogMessage(X_ERROR, "systemd-logind: could not add match: %s\n",
> +                   error.message);
> +        goto cleanup;
> +    }
> +
> +    dbus_bus_add_match(connection,
> +        "type='signal',sender='org.freedesktop.login1',interface='org.freedesktop.login1.Session',member='ResumeDevice'",
> +        &error);
> +    if (dbus_error_is_set(&error)) {
> +        LogMessage(X_ERROR, "systemd-logind: could not add match: %s\n",
> +                   error.message);
> +        goto cleanup;
> +    }
> +
>       /*
>        * HdG: This is not useful with systemd <= 208 since the signal only
>        * contains invalidated property names there, rather than property, val
>