[PATCH libinput 5/9] evdev: remove a race condition opening the wrong device

[Hans de Goede]

Hi,

On 11/24/2014 01:46 AM, Peter Hutterer wrote:
> Potential race condition:
> - udev notifies us that a udev_device became available
> - we go for a coffee and chat to the neighbours on the way
> - the device is unplugged
> - a new device is plugged in, gets the same devnode
> - we finish our coffee and come back
> - open(udev_device_get_devnode())
> - new device is now opened as the old device
>
> To avoid the above race, we compare the syspath of the device at the open fd
> with the syspath of the device we originally wanted. If they differ, we fail.
>
> evdev_compare_syspath was simply moved up.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

Looks good:

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

Regards,

Hans

> ---
>   src/evdev.c | 49 ++++++++++++++++++++++++++-----------------------
>   1 file changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index 8d457f2..8897cd4 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -1449,6 +1449,29 @@ evdev_notify_added_device(struct evdev_device *device)
>   	notify_added_device(&device->base);
>   }
>
> +static int
> +evdev_device_compare_syspath(struct udev_device *udev_device, int fd)
> +{
> +	struct udev *udev = udev_device_get_udev(udev_device);
> +	struct udev_device *udev_device_new;
> +	struct stat st;
> +	int rc = 1;
> +
> +	if (fstat(fd, &st) < 0)
> +		goto out;
> +
> +	udev_device_new = udev_device_new_from_devnum(udev, 'c', st.st_rdev);
> +	if (!udev_device_new)
> +		goto out;
> +
> +	rc = strcmp(udev_device_get_syspath(udev_device_new),
> +		    udev_device_get_syspath(udev_device));
> +out:
> +	if (udev_device_new)
> +		udev_device_unref(udev_device_new);
> +	return rc;
> +}
> +
>   struct evdev_device *
>   evdev_device_create(struct libinput_seat *seat,
>   		    struct udev_device *udev_device)
> @@ -1471,6 +1494,9 @@ evdev_device_create(struct libinput_seat *seat,
>   		return NULL;
>   	}
>
> +	if (evdev_device_compare_syspath(udev_device, fd) != 0)
> +		goto err;
> +
>   	device = zalloc(sizeof *device);
>   	if (device == NULL)
>   		goto err;
> @@ -1909,29 +1935,6 @@ evdev_device_suspend(struct evdev_device *device)
>   	return 0;
>   }
>
> -static int
> -evdev_device_compare_syspath(struct udev_device *udev_device, int fd)
> -{
> -	struct udev *udev = udev_device_get_udev(udev_device);
> -	struct udev_device *udev_device_new;
> -	struct stat st;
> -	int rc = 1;
> -
> -	if (fstat(fd, &st) < 0)
> -		goto out;
> -
> -	udev_device_new = udev_device_new_from_devnum(udev, 'c', st.st_rdev);
> -	if (!udev_device_new)
> -		goto out;
> -
> -	rc = strcmp(udev_device_get_syspath(udev_device_new),
> -		    udev_device_get_syspath(udev_device));
> -out:
> -	if (udev_device_new)
> -		udev_device_unref(udev_device_new);
> -	return rc;
> -}
> -
>   int
>   evdev_device_resume(struct evdev_device *device)
>   {
>