Daemon end of session/greeter pipes not closed

[rumen]

[Impact]
LightDM doesn't close the server side end of the pipes used to communicate with session processes and greeters. This means each session/greeter that is created leaks two file descriptors eventually leading to the system stopping it from creating new pipes.

[Test Case]
1. Start LightDM
2. Check how many pipes are open
# lsof -p {lightdm_pid} | grep FIFO | wc -l
3. Create sessions by either cycling between users in Unity Greeter or logging in and out
4. Check how many pipes exist using step 2.
Expected result:
No more pipes should be open
Observed result:
Many pipes remain open

[Regression Potential]
Low. Fix is to close pipes when finished with them. Tested with regression tests.

[Robert Ancell (robert-ancell)]

There is a bug here:

+++ b/src/greeter.c
@@ -937,6 +937,8 @@ greeter_finalize (GObject *object)
     if (self->priv->from_greeter_channel)
         g_io_channel_unref (self->priv->from_greeter_channel);

+ close(g_io_channel_unix_get_fd(self->priv->to_greeter_channel));
+ close(g_io_channel_unix_get_fd(self->priv->from_greeter_channel));

You access self->priv->from_greeter_channel after unreffing it...

[rumen (rumen)]

You are right! Thank you!
Here is (hopefully) fixed version of the patch.
I believe it is legal to close FD before unrefing the channel.
I do not know what g_io_channel_unref() exacty does, but
one thing is for sure it does not close the associated FD.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "FD_leak_fix.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.13.0-0ubuntu1

---------------
lightdm (1.13.0-0ubuntu1) vivid; urgency=medium

  * New upstream release:
    - Fix crash when having configuration keys defined in multiple places
      (LP: #1377373)
    - Fix pipe file descriptor leak for each session login / authentication
      (LP: #1190344)
    - Use correct syntax for DesktopNames key in session files (LP: #1383321)
    - Match seat configuration with globbing (LP: #1364911)
    - Allow user switching in multi-seat until bug stopping greeter showing on
      logout is fixed
    - Disable log message when AccountsService users change (LP: #1376357)
    - Update AppArmor scripts, requires AppArmor 2.9
    - Update tests to run better on servers
  * debian/config-error-dialog.sh:
    - Show warning dialog instead of interrupted login if syntax error in
      ~/.profile etc (LP: #678421)
 -- Robert Ancell <email address hidden> Thu, 13 Nov 2014 11:08:17 +1300

[Brian Murray (brian-murray)]

Hello rumen, or anyone else affected,

Accepted lightdm into utopic-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/lightdm/1.12.2-0ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Brian Murray (brian-murray)]

Hello rumen, or anyone else affected,

Accepted lightdm into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lightdm/1.10.4-0ubuntu2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Mathew Hodson (mhodson)]

Using 1.10.4-0ubuntu2 from trusty-proposed, I don't see any pipes open.

$ lsof -p 1055
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
lightdm 1055 root cwd unknown /proc/1055/cwd (readlink: Permission denied)
lightdm 1055 root rtd unknown /proc/1055/root (readlink: Permission denied)
lightdm 1055 root txt unknown /proc/1055/exe (readlink: Permission denied)
lightdm 1055 root NOFD /proc/1055/fd (opendir: Permission denied)
$ lsof -p 1055 | grep FIFO | wc -l
0

[Mathew Hodson (mhodson)]

I needed to run it with sudo.

Every time I switch to Guest Session, logout, and then switch back to the first user, I notice two more pipes open.

$ sudo lsof -p 1055 | grep FIFO
lsof: WARNING: can't stat() fuse.gvfsd-fuse file system /run/user/1000/gvfs
      Output information may be incomplete.
lightdm 1055 root 4r FIFO 0,8 0t0 12349 pipe
lightdm 1055 root 5w FIFO 0,8 0t0 12349 pipe
lightdm 1055 root 13w FIFO 0,8 0t0 11794 pipe
lightdm 1055 root 14r FIFO 0,8 0t0 11795 pipe
lightdm 1055 root 15w FIFO 0,8 0t0 12546 pipe
lightdm 1055 root 16r FIFO 0,8 0t0 12547 pipe
lightdm 1055 root 18w FIFO 0,8 0t0 1244475 pipe
lightdm 1055 root 19r FIFO 0,8 0t0 1244476 pipe
lightdm 1055 root 20w FIFO 0,8 0t0 1257093 pipe
lightdm 1055 root 21r FIFO 0,8 0t0 1257094 pipe
lightdm 1055 root 22w FIFO 0,8 0t0 1316710 pipe
lightdm 1055 root 23r FIFO 0,8 0t0 1316711 pipe
lightdm 1055 root 24w FIFO 0,8 0t0 1332415 pipe
lightdm 1055 root 25r FIFO 0,8 0t0 1332416 pipe
lightdm 1055 root 26w FIFO 0,8 0t0 1351976 pipe
lightdm 1055 root 27r FIFO 0,8 0t0 1351977 pipe
lightdm 1055 root 28w FIFO 0,8 0t0 1369466 pipe
lightdm 1055 root 29r FIFO 0,8 0t0 1369467 pipe

[Open Sense Solutions (opensense)]

I just want to confirm this bug affects precise and precise-updates which is using lightdm 1.2.3

[Dimitri John Ledkov (xnox)]

Even thought this does not address the bug completely, can the lightdm update be released none-the-less? as it does not degrade beyond what's in current release/updates pockets?

[Adam Conrad (adconrad)]

The verification of the Stable Release Update for lightdm has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.10.4-0ubuntu2

---------------
lightdm (1.10.4-0ubuntu2) trusty; urgency=medium

  * debian/config-error-dialog.sh:
  * debian/lightdm-session:
    - Use bash for the session to improve error handling (LP: #678421)
  * debian/control:
    - Depend on bash

lightdm (1.10.4-0ubuntu1) trusty; urgency=medium

  * New upstream release:
    - Fix pipe file descriptor leak for each session login / authentication
      (LP: #1190344)
    - Support DesktopNames key in session files (LP: #1383321)
  * debian/config-error-dialog.sh:
    - Show warning dialog instead of interrupted login if syntax error in
      ~/.profile etc (LP: #678421)
 -- Robert Ancell <email address hidden> Tue, 25 Nov 2014 11:25:23 +1300

[Adam Conrad (adconrad)]

Reopening this bug, as it wasn't actually fixed by the SRU that claimed to fix it.

[Robert Ancell (robert-ancell)]

Hmm, turns out I only committed the session leak from the attached patch and not the greeter leak. I've committed that fix now.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.13.2-0ubuntu1

---------------
lightdm (1.13.2-0ubuntu1) vivid; urgency=medium

  * New upstream release:
    - Fix pipe file descriptor leak for each greeter session. (LP: #1190344)
    - Support active session changing via logind. (LP: #1371378)
    - Don't allow liblightdm-gobject to be disabled. It is required for
      liblightdm-qt and the tests so it's not worth supporting builds without
      it.
    - Add bash autocompletion support
  * debian/lightdm.install:
    - Install autocompletion configuration
 -- Robert Ancell <email address hidden> Tue, 10 Mar 2015 14:54:29 +1300

[Timo Aaltonen (tjaalton)]

Hello rumen, or anyone else affected,

Accepted lightdm into utopic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lightdm/1.12.3-0ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Timo Aaltonen (tjaalton)]

Hello rumen, or anyone else affected,

Accepted lightdm into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lightdm/1.10.5-0ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Robert Ancell (robert-ancell)]

Confirmed fixed in precise-proposed, trusty-proposed and utopic-proposed.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.10.5-0ubuntu1

---------------
lightdm (1.10.5-0ubuntu1) trusty; urgency=medium

  * New upstream release:
    - Fix pipe file descriptor leak for each greeter session. (LP: #1190344)
    - Don't attempt generate D-Bus seat/session removal signals on shutdown.
  * debian/guest-account.sh:
    - Rename variables to make script compatible with Bash (LP: #1411100)
  * debian/control:
    - Set required version of bash
 -- Robert Ancell <email address hidden> Tue, 10 Mar 2015 16:49:11 +1300

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.12.3-0ubuntu1

---------------
lightdm (1.12.3-0ubuntu1) utopic; urgency=medium

  * New upstream release:
    - Fix pipe file descriptor leak for each greeter session. (LP: #1190344)
    - Don't attempt generate D-Bus seat/session removal signals on shutdown.
  * debian/guest-account.sh:
    - Rename variables to make script compatible with Bash (LP: #1411100)
  * debian/lightdm-session:
    - Use bash for the session to improve error handling (LP: #678421)
  * debian/control:
    - Depend on bash

lightdm (1.12.2-0ubuntu1) utopic; urgency=medium

  * New upstream release:
    - Fix pipe file descriptor leak for each session login / authentication
      (LP: #1190344)
    - Use correct syntax for DesktopNames key in session files (LP: #1383321)
    - Mock /run in tests
  * debian/config-error-dialog.sh:
    - Show warning dialog instead of interrupted login if syntax error in
      ~/.profile etc (LP: #678421)
 -- Robert Ancell <email address hidden> Tue, 10 Mar 2015 16:02:44 +1300

[Brian Murray (brian-murray)]

Hello rumen, or anyone else affected,

Accepted lightdm into precise-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/lightdm/1.2.3-0ubuntu2.7 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Robert Ancell (robert-ancell)]

Still fixed in 1.2.3-0ubuntu2.7

[Launchpad Janitor (janitor)]

This bug was fixed in the package lightdm - 1.2.3-0ubuntu2.7

---------------
lightdm (1.2.3-0ubuntu2.7) precise; urgency=medium

  * debian/patches/15_gsources.patch:
    - Correctly remove GSources on finalize (LP: #1431654)

lightdm (1.2.3-0ubuntu2.6) precise; urgency=medium

  * debian/patches/09_close_pipes.patch:
    - Close pipes correctly (LP: #1190344)
  * debian/patches/10_conf_section_name.patch:
    - Correct section name in default users.conf (LP: #1069218)
  * debian/patches/11_quit_timeout.patch:
    - Destroy quit timeout when a process object is destroyed - fixes a crash
      where a deleted Process object might be accessed after a timeout
      (LP: #1207935)
  * debian/patches/12_layout_no_x11.patch:
    - Handle not getting an X connection when attempting to get X layouts
      (LP: #1235915)
  * debian/patches/13_introspection_makefile.patch:
    - Fix introspection build with newer versions of g-ir-scanner
  * debian/patches/14_compile_warnings.patch:
    - Fix compile warnings
 -- Robert Ancell <email address hidden> Tue, 17 Mar 2015 14:21:43 +1300

[jacob clark (jcsuntech)]

Still seeing this issue with package and tracking FIFO on log in log out

lightdm.x86_64 1.25.0-1.el7
lightdm-gobject.x86_64 1.25.0-1.el7
lightdm-gtk.x86_64 1.8.5-19.el7
lightdm-gtk-common.noarch 1.8.5-19.el7
[root@caeusadm tmp]# lightdm -v
lightdm 1.25.0
[root@caeusadm tmp]# lsof -p 13696 | grep FIFO | wc -l
188
[root@caeusadm tmp]# lsof -p 13696 | grep FIFO | wc -l
190

Increments by 2 for every log in log out and never frees them up.

When it passes open file limit (currently 1024) lightdm crashes and all users lose session.