[LightDM] Race condition in lightdm greeter setup

[Robert Ancell]

Hi Veli-Matti,

Thanks for finding this problem.

There's more information in the bug report (
https://bugs.launchpad.net/lightdm/+bug/1172752) but to summarise for
others reading the list:
- It is correct that two sessions are spawned - one is the greeter and one
is the user session being authenticated
- There is a bug here though, the signal handler and pipe is open before
the child processes are execed, which could cause the daemon to handle a
SIGTERM from the child process and exit

--Robert


On 4 May 2013 01:56, Veli-Matti Lintu <veli-matti.lintu at opinsys.fi> wrote:

>
> I came across a race condition in lightdm greeter setup phase before the
> login screen is displayed (at boot time or after logout).
>
> I reported this also on Launchpad with more details (
> https://bugs.launchpad.net/lightdm/+bug/1172752), but to work on a proper
> fix, ideas on how to fix this would be welcome.
>
> During greeter setup "lightdm --session-child" is spawned twice. The first
> call to session_start() that does fork+execlp() is done from
> greeter_start() and right after that handle_login() calls session_stop() +
> session_start(). session_stop() sends SIGTERM to the child process, but if
> the child has not managed to call execlp() yet, it still has signal handler
> set for SIGTERM, because fork copies the parent's signal handlers to the
> child. Now when session_stop() sends SIGTERM to the child, it uses the
> signal handler set by the parent which causes the signal go to signal_cb()
> that then signals the main lightdm process to die.
>
> I managed to get rid of the problem by switching fork() -> vfork() in
> session_start() which blocks the parent before execlp() is run and signal
> handlers are cleared. This ensures that the signal sent from session_stop()
> always ends up only to the child. Using vfork() is probably not the best
> solution, though, so I wonder if others have better ideas on how to fix
> this?
>
>
> --- src/session.c.orig  2012-08-29 21:25:16.000000000 +0000
> +++ src/session.c       2013-04-25 15:15:13.353450704 +0000
> @@ -360,7 +360,7 @@
>      session->priv->username = g_strdup (username);
>
>      /* Run the child */
> -    session->priv->pid = fork ();
> +    session->priv->pid = vfork ();
>      if (session->priv->pid < 0)
>      {
>          g_debug ("Failed to fork session child process: %s", strerror
> (errno));
>
>
> The signal handling in lightdm looks quite tricky, so I'm not really sure
> if this is the best way to handle this. Maybe it would be possible to get
> rid of the first "lightdm --session-child" spawning to make all this
> unnecessary? I could try to work on fixing this if someone who knows the
> code better has an opinion on this.
>
> Happy hacking!
>
> Veli-Matti
> _______________________________________________
> LightDM mailing list
> LightDM at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/lightdm
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/lightdm/attachments/20130516/12bcedff/attachment.html>