[PATCH] systemd-logind: let the logind code decided whether to close an fd

[Peter Hutterer]

We can only request one fd per device from systemd-logind. If a fd is re-used
by the same device, releasing the fd from one device doesn't mean we can close
it. The systemd code knows when it's really released, so let it close the fd.

Test case: xorg.conf section for an input device with hotplugging enabled.
evdev detects the duplicate and closes the hotplugged device, which closes the
fd. The other instance of evdev thinks the fd is still valid so now you're
playing a double lottery. First, which client(s) will get the evdev fd?
Second, which requests will be picked up by evdev and which ones will be
picked up by the client? You'll never now, but the fun is in finding out.

Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
The first hunk could be reduced by an if statement but I found it more
obvious this way.

 config/config.c                              | 6 ++----
 hw/xfree86/common/xf86Xinput.c               | 6 ++----
 hw/xfree86/common/xf86platformBus.c          | 3 +--
 hw/xfree86/os-support/linux/lnx_platform.c   | 2 +-
 hw/xfree86/os-support/linux/systemd-logind.c | 7 +++++--
 include/systemd-logind.h                     | 4 ++--
 6 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/config/config.c b/config/config.c
index def7f16..5514516 100644
--- a/config/config.c
+++ b/config/config.c
@@ -250,8 +250,6 @@ config_odev_free_attributes(struct OdevAttributes *attribs)
         free(iter);
     }
 
-    if (fd != -1) {
-        systemd_logind_release_fd(major, minor);
-        close(fd);
-    }
+    if (fd != -1)
+        systemd_logind_release_fd(major, minor, fd);
 }
diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
index bc6b73f..220790d 100644
--- a/hw/xfree86/common/xf86Xinput.c
+++ b/hw/xfree86/common/xf86Xinput.c
@@ -771,8 +771,7 @@ xf86DeleteInput(InputInfoPtr pInp, int flags)
     FreeInputAttributes(pInp->attrs);
 
     if (pInp->flags & XI86_SERVER_FD) {
-        systemd_logind_release_fd(pInp->major, pInp->minor);
-        close(pInp->fd);
+        systemd_logind_release_fd(pInp->major, pInp->minor, pInp->fd);
     }
 
     /* Remove the entry from the list. */
@@ -873,8 +872,7 @@ xf86NewInputDevice(InputInfoPtr pInfo, DeviceIntPtr *pdev, BOOL enable)
                             sizeof(pInfo) * (new_input_devices_count + 1));
                 new_input_devices[new_input_devices_count] = pInfo;
                 new_input_devices_count++;
-                systemd_logind_release_fd(pInfo->major, pInfo->minor);
-                close(fd);
+                systemd_logind_release_fd(pInfo->major, pInfo->minor, fd);
                 return BadMatch;
             }
             pInfo->fd = fd;
diff --git a/hw/xfree86/common/xf86platformBus.c b/hw/xfree86/common/xf86platformBus.c
index 4e80f9e..dd118a2 100644
--- a/hw/xfree86/common/xf86platformBus.c
+++ b/hw/xfree86/common/xf86platformBus.c
@@ -340,8 +340,7 @@ static Bool doPlatformProbe(struct xf86_platform_device *dev, DriverPtr drvp,
             fd = xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_FD, -1);
             major = xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_MAJOR, 0);
             minor = xf86_get_platform_device_int_attrib(dev, ODEV_ATTRIB_MINOR, 0);
-            systemd_logind_release_fd(major, minor);
-            close(fd);
+            systemd_logind_release_fd(major, minor, fd);
             config_odev_add_int_attribute(dev->attribs, ODEV_ATTRIB_FD, -1);
             dev->flags &= ~XF86_PDEV_SERVER_FD;
         }
diff --git a/hw/xfree86/os-support/linux/lnx_platform.c b/hw/xfree86/os-support/linux/lnx_platform.c
index dbd7aa0..308275a 100644
--- a/hw/xfree86/os-support/linux/lnx_platform.c
+++ b/hw/xfree86/os-support/linux/lnx_platform.c
@@ -37,7 +37,7 @@ get_drm_info(struct OdevAttributes *attribs, char *path, int delayed_index)
         if (paused) {
             LogMessage(X_ERROR,
                     "Error systemd-logind returned paused fd for drm node\n");
-            systemd_logind_release_fd(major, minor);
+            systemd_logind_release_fd(major, minor, -1);
             return FALSE;
         }
         config_odev_add_int_attribute(attribs, ODEV_ATTRIB_FD, fd);
diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c
index ed670a8..73a8d55 100644
--- a/hw/xfree86/os-support/linux/systemd-logind.c
+++ b/hw/xfree86/os-support/linux/systemd-logind.c
@@ -162,7 +162,7 @@ cleanup:
 }
 
 void
-systemd_logind_release_fd(int _major, int _minor)
+systemd_logind_release_fd(int _major, int _minor, int fd)
 {
     struct systemd_logind_info *info = &logind_info;
     InputInfoPtr pInfo;
@@ -174,7 +174,7 @@ systemd_logind_release_fd(int _major, int _minor)
     int matches = 0;
 
     if (!info->session || major == 0)
-        return;
+        goto close;
 
     /* Only release the fd if there is only 1 InputInfo left for this major
      * and minor, otherwise other InputInfo's are still referencing the fd. */
@@ -218,6 +218,9 @@ cleanup:
     if (reply)
         dbus_message_unref(reply);
     dbus_error_free(&error);
+close:
+    if (fd != -1)
+        close(fd);
 }
 
 int
diff --git a/include/systemd-logind.h b/include/systemd-logind.h
index 06dd031..a4067d0 100644
--- a/include/systemd-logind.h
+++ b/include/systemd-logind.h
@@ -30,14 +30,14 @@
 int systemd_logind_init(void);
 void systemd_logind_fini(void);
 int systemd_logind_take_fd(int major, int minor, const char *path, Bool *paus);
-void systemd_logind_release_fd(int major, int minor);
+void systemd_logind_release_fd(int major, int minor, int fd);
 int systemd_logind_controls_session(void);
 void systemd_logind_vtenter(void);
 #else
 #define systemd_logind_init()
 #define systemd_logind_fini()
 #define systemd_logind_take_fd(major, minor, path, paus) -1
-#define systemd_logind_release_fd(major, minor)
+#define systemd_logind_release_fd(major, minor, fd) close(fd)
 #define systemd_logind_controls_session() 0
 #define systemd_logind_vtenter()
 #endif
-- 
1.9.0