[PATCH xf86-input-libinput] Swap the registered input device on DEVICE_OFF when needed

[Hans de Goede]

Hi,

On 18-10-16 03:47, Peter Hutterer wrote:
> If we don't swap out the pInfo previously passed to xf86AddEnabledDevice(),
> the thread eventually calls read_input on a struct that has been deleted.
> Avoid this by swapping out the to-be-destroyed pInfo with the first one we
> find.
>
> Reproducer: sudo udevadm trigger --type=devices --action=add
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

Patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans


> ---
> Changes to the RFC:
> - checking for the driver now
>
>  src/xf86libinput.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>
> diff --git a/src/xf86libinput.c b/src/xf86libinput.c
> index 69f7ae3..061e495 100644
> --- a/src/xf86libinput.c
> +++ b/src/xf86libinput.c
> @@ -87,6 +87,7 @@
>  struct xf86libinput_driver {
>  	struct libinput *libinput;
>  	int device_enabled_count;
> +	void *registered_InputInfoPtr;
>  };
>
>  static struct xf86libinput_driver driver_context;
> @@ -583,6 +584,7 @@ xf86libinput_on(DeviceIntPtr dev)
>  	if (driver_context.device_enabled_count == 0) {
>  #if HAVE_THREADED_INPUT
>  		xf86AddEnabledDevice(pInfo);
> +		driver_context.registered_InputInfoPtr = pInfo;
>  #else
>  		/* Can't use xf86AddEnabledDevice on an epollfd */
>  		AddEnabledDevice(pInfo->fd);
> @@ -1131,6 +1133,39 @@ xf86libinput_init(DeviceIntPtr dev)
>  	return 0;
>  }
>
> +static bool
> +is_libinput_device(InputInfoPtr pInfo)
> +{
> +	char *driver;
> +	BOOL rc;
> +
> +	driver = xf86CheckStrOption(pInfo->options, "driver", "");
> +	rc = strcmp(driver, "libinput") == 0;
> +	free(driver);
> +
> +	return rc;
> +}
> +
> +static void
> +swap_registered_device(InputInfoPtr pInfo)
> +{
> +	InputInfoPtr next;
> +
> +	if (pInfo != driver_context.registered_InputInfoPtr)
> +		return;
> +
> +	next = xf86FirstLocalDevice();
> +	while (next == pInfo || !is_libinput_device(next))
> +		next = next->next;
> +
> +	input_lock();
> +	xf86RemoveEnabledDevice(pInfo);
> +	if (next) /* shouldn't ever be NULL anyway */
> +		xf86AddEnabledDevice(next);
> +	driver_context.registered_InputInfoPtr = next;
> +	input_unlock();
> +}
> +
>  static void
>  xf86libinput_destroy(DeviceIntPtr dev)
>  {
> @@ -1138,6 +1173,17 @@ xf86libinput_destroy(DeviceIntPtr dev)
>  	struct xf86libinput *driver_data = pInfo->private;
>  	struct xf86libinput_device *shared_device = driver_data->shared_device;
>
> +	/* If the device being destroyed is the one we used for
> +	 * xf86AddEnabledDevice(), we need to swap it out for one that is
> +	 * still live. xf86AddEnabledDevice() buffers some data and once the
> +	 * deletes pInfo (when DEVICE_OFF completes) the thread will keep
> +	 * calling that struct's read_input because we never removed it.
> +	 * Avoid this by removing ours and substituting one that's still
> +	 * valid, the fd is the same anyway (libinput's epollfd).
> +	 */
> +	if (driver_context.device_enabled_count > 0)
> +		swap_registered_device(pInfo);
> +
>  	xorg_list_del(&driver_data->shared_device_link);
>
>  	if (driver_data->tablet_tool)
>