Hi Hans,<br><br>On Wednesday, 5 October 2016, Hans de Goede <<a href="mailto:hdegoede@redhat.com">hdegoede@redhat.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">When the xserver uses threaded input, it keeps a pointer to the InputInfo<br>
passed into xf86AddEnabledDevice and calls pointer->read_input on events.<br>
<br>
But when the first enabled device goes away the pInfo we've passed into<br>
xf86AddEnabledDevice gets freed and eventually pInfo->read_input gets<br>
overwritten (or pInfo points to unmapped memory) leading to a segfault.<br>
<br>
This commit fixes this by replacing the pInfo passed into<br>
xf86AddEnabledDevice with a pointer to a global InputInfo stored inside<br>
the driver_context struct.<br>
<br>
Signed-off-by: Hans de Goede <<a href="javascript:;" onclick="_e(event, 'cvml', 'hdegoede@redhat.com')">hdegoede@redhat.com</a>><br>
---<br>
 src/xf86libinput.c | 22 ++++++++++++++++------<br>
 1 file changed, 16 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/src/xf86libinput.c b/src/xf86libinput.c<br>
index 21f87f5..485e212 100644<br>
--- a/src/xf86libinput.c<br>
+++ b/src/xf86libinput.c<br>
@@ -86,6 +86,7 @@<br>
<br>
 struct xf86libinput_driver {<br>
        struct libinput *libinput;<br>
+       struct _InputInfoRec InputInfo;<br>
        int device_enabled_count;<br>
 };<br>
<br>
@@ -582,7 +583,17 @@ xf86libinput_on(DeviceIntPtr dev)<br>
<br>
        if (driver_context.device_<wbr>enabled_count == 0) {<br>
 #if HAVE_THREADED_INPUT<br>
-               xf86AddEnabledDevice(pInfo);<br>
+               /*<br>
+                * The xserver keeps a pointer to the InputInfo passed into<br>
+                * xf86AddEnabledDevice and calls pointer->read_input on<br>
+                * events. Thus we cannot simply pass in our current pInfo<br>
+                * as that will be deleted when the current input device gets<br>
+                * unplugged. Instead pass in a pointer to a global<br>
+                * InputInfo inside the driver_context.<br>
+                */<br>
+               driver_context.InputInfo.fd = pInfo->fd;</blockquote><div>Reading the above comment makes me wonder about the lifetime of the fd. I take it that we're not leaking it atm ?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+               driver_context.InputInfo.read_<wbr>input = pInfo->read_input;<br>
+               xf86AddEnabledDevice(&driver_<wbr>context.InputInfo);<br>
 #else<br>
                /* Can't use xf86AddEnabledDevice on an epollfd */<br>
                AddEnabledDevice(pInfo->fd);</blockquote><div>Can we use the same storage for the !HAVE_THREADED_INPUT code paths ?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
@@ -606,7 +617,7 @@ xf86libinput_off(DeviceIntPtr dev)<br>
<br>
        if (--driver_context.device_<wbr>enabled_count == 0) {<br>
 #if HAVE_THREADED_INPUT<br>
-               xf86RemoveEnabledDevice(pInfo)<wbr>;<br>
+               xf86RemoveEnabledDevice(&<wbr>driver_context.InputInfo);<br>
 #else<br>
                RemoveEnabledDevice(pInfo->fd)<wbr>;<br>
 #endif<br>
@@ -1923,7 +1934,7 @@ out:<br>
 }<br>
<br>
 static void<br>
-xf86libinput_read_input(<wbr>InputInfoPtr pInfo)<br>
+xf86libinput_read_input(<wbr>InputInfoPtr do_not_use)</blockquote><div>Worth just dropping the argument and fixing the caller(s)?</div><div><br></div><div>Emil</div><div><br></div>