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