[PATCH xf86-input-libinput] If the parent libinput_device is unavailable, create a new one

[Hans de Goede]

Hi,

On 15-11-16 05:37, Peter Hutterer wrote:
> The parent device ref's the libinput device during pre_init and unref's it
> during DEVICE_INIT, so the copy is lost. During DEVICE_ON, the libinput device
> is re-added and ref'd, this one stays around now. But the takeaway is: unless
> the device is enabled, no libinput device reference is available.
>
> If a device is a mixed pointer + keyboard device, a subdevice is created
> during a WorkProc. The subdevice relied on the parent's libinput_device being
> available and didn't even check for it. This WorkProc usually runs after
> the parent's DEVICE_ON, so in most cases all is well.
>
> But when running without logind and the server is vt-switched away, the parent
> device only runs PreInit and DEVICE_INIT but never DEVICE_ON, causing the
> subdevice to burn, crash, and generally fail horribly when it dereferences the
> parent's libinput device.
>
> Fix this because we have global warming already and don't need to burn more
> things and also because it's considered bad user experience to have the
> server crash. The simple fix is to check the parent device first and if it is
> unavailable, create a new one because it will end up disabled as well anyway,
> so the ref goes away as well. The use-case where the parent somehow gets
> disabled but the subdevice doesn't is a bit too niche to worry about.
>
> This doesn't happen with logind because in that case we don't get a usable fd
> while VT-switched away, so we can't even run PreInit and never get this far
> (see the paused fd handling in the xfree86 code for that). It can be
> reproduced by setting AutoEnableDevices off, but why would you do that,
> seriously.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=97117
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

Hmm, so what happens if the parent device later does get DEVICE_ON, after
the subdevice has created its own libinputdevice ?

Regards,

Hans



> ---
>  src/xf86libinput.c | 42 +++++++++++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/src/xf86libinput.c b/src/xf86libinput.c
> index 6792d1c..747e84b 100644
> --- a/src/xf86libinput.c
> +++ b/src/xf86libinput.c
> @@ -2850,7 +2850,7 @@ xf86libinput_pre_init(InputDriverPtr drv,
>  	struct xf86libinput *driver_data = NULL;
>  	struct xf86libinput_device *shared_device = NULL;
>  	struct libinput *libinput = NULL;
> -	struct libinput_device *device;
> +	struct libinput_device *device = NULL;
>  	char *path = NULL;
>  	bool is_subdevice;
>
> @@ -2885,7 +2885,28 @@ xf86libinput_pre_init(InputDriverPtr drv,
>  	}
>
>  	is_subdevice = xf86libinput_is_subdevice(pInfo);
> -	if (!is_subdevice) {
> +	if (is_subdevice) {
> +		InputInfoPtr parent;
> +		struct xf86libinput *parent_driver_data;
> +
> +		parent = xf86libinput_get_parent(pInfo);
> +		if (!parent) {
> +			xf86IDrvMsg(pInfo, X_ERROR, "Failed to find parent device\n");
> +			goto fail;
> +		}
> +
> +		parent_driver_data = parent->private;
> +		if (!parent_driver_data) /* parent already removed again */
> +			goto fail;
> +
> +		xf86IDrvMsg(pInfo, X_INFO, "is a virtual subdevice\n");
> +		shared_device = xf86libinput_shared_ref(parent_driver_data->shared_device);
> +		device = shared_device->device;
> +		if (!device)
> +			xf86IDrvMsg(pInfo, X_ERROR, "Parent device not available\n");
> +	}
> +
> +	if (!device) {
>  		device = libinput_path_add_device(libinput, path);
>  		if (!device) {
>  			xf86IDrvMsg(pInfo, X_ERROR, "Failed to create a device for %s\n", path);
> @@ -2903,23 +2924,6 @@ xf86libinput_pre_init(InputDriverPtr drv,
>  			libinput_device_unref(device);
>  			goto fail;
>  		}
> -	} else {
> -		InputInfoPtr parent;
> -		struct xf86libinput *parent_driver_data;
> -
> -		parent = xf86libinput_get_parent(pInfo);
> -		if (!parent) {
> -			xf86IDrvMsg(pInfo, X_ERROR, "Failed to find parent device\n");
> -			goto fail;
> -		}
> -
> -		parent_driver_data = parent->private;
> -		if (!parent_driver_data) /* parent already removed again */
> -			goto fail;
> -
> -		xf86IDrvMsg(pInfo, X_INFO, "is a virtual subdevice\n");
> -		shared_device = xf86libinput_shared_ref(parent_driver_data->shared_device);
> -		device = shared_device->device;
>  	}
>
>  	pInfo->private = driver_data;
>