Uinput: Deadlock upon UI_DEV_DESTROY ioctl while a non-zero number of ff_effects is still active (uploaded)

Uinput: Deadlock upon UI_DEV_DESTROY ioctl while a non-zero number of ff_effects is still active (uploaded)

[Elias Vanderstuyft]

Hi everyone,

In the process of extending the libsuinput library
(https://github.com/tuomasjjrasanen/python-uinput/tree/master/libsuinput/src)
to support force feedback,
I encountered some bugs in the kernel.
For some bugs, I'm not sure for the right fix,
so it's safer to first present the problem and ask for feedback.
For clarity, I divided the bugs between different mail subjects.

This subject handles about a deadlock encountered
when userspace issues a UI_DEV_DESTROY ioctl on a uinput device,
while a non-zero number of ff_effects is still active
(i.e. the application that issued the ioctl
has not yet erased all uploaded effects).

The mutex causing the deadlock is "(struct uinput_device).mutex".

The chain of the deadlock is as follows:
- uinput.c::uinput_ioctl_handler() LOCKS using mutex_lock_interruptible()
- uinput.c::uinput_ioctl_handler() calls uinput.c::uinput_destroy_device()
- uinput.c::uinput_destroy_device() calls input.c::input_unregister_device()
- input.c::input_unregister_device() calls input.c::__input_unregister_device()
- input.c::__input_unregister_device() calls handle->handler->disconnect()
- evdev.c::evdev_disconnect calls evdev.c::evdev_cleanup()
- evdev.c::evdev_cleanup() calls input.c::input_flush_device()
- input.c::input_flush_device() calls dev->flush()
- ff-core.c::flush_effects() calls ff-core.c::erase_effect()
- ff-core.c::erase_effect() calls ff->erase()
- uinput.c::uinput_dev_erase_effect() calls uinput.c::uinput_request_submit()
- uinput.c::uinput_request_submit() calls uinput.c::uinput_request_send()
- uinput.c::uinput_request_send() LOCKS using mutex_lock_interruptible()

Hints for solving this problem are much appreciated.
Maybe by setting a flag when UI_DEV_DESTROY ioctl is issued,
and avoiding to acquire the mutex in uinput_request_send()
when this flag is found to be set?

Thanks,
Elias

Re: Uinput: Deadlock upon UI_DEV_DESTROY ioctl while a non-zero number of ff_effects is still active (uploaded)

[Dmitry Torokhov]

On Thu, Jul 16, 2015 at 11:21:46PM +0200, Elias Vanderstuyft wrote:
> Hi everyone,
> 
> In the process of extending the libsuinput library
> (https://github.com/tuomasjjrasanen/python-uinput/tree/master/libsuinput/src)
> to support force feedback,
> I encountered some bugs in the kernel.
> For some bugs, I'm not sure for the right fix,
> so it's safer to first present the problem and ask for feedback.
> For clarity, I divided the bugs between different mail subjects.
> 
> This subject handles about a deadlock encountered
> when userspace issues a UI_DEV_DESTROY ioctl on a uinput device,
> while a non-zero number of ff_effects is still active
> (i.e. the application that issued the ioctl
> has not yet erased all uploaded effects).
> 
> The mutex causing the deadlock is "(struct uinput_device).mutex".
> 
> The chain of the deadlock is as follows:
> - uinput.c::uinput_ioctl_handler() LOCKS using mutex_lock_interruptible()
> - uinput.c::uinput_ioctl_handler() calls uinput.c::uinput_destroy_device()
> - uinput.c::uinput_destroy_device() calls input.c::input_unregister_device()
> - input.c::input_unregister_device() calls input.c::__input_unregister_device()
> - input.c::__input_unregister_device() calls handle->handler->disconnect()
> - evdev.c::evdev_disconnect calls evdev.c::evdev_cleanup()
> - evdev.c::evdev_cleanup() calls input.c::input_flush_device()
> - input.c::input_flush_device() calls dev->flush()
> - ff-core.c::flush_effects() calls ff-core.c::erase_effect()
> - ff-core.c::erase_effect() calls ff->erase()
> - uinput.c::uinput_dev_erase_effect() calls uinput.c::uinput_request_submit()
> - uinput.c::uinput_request_submit() calls uinput.c::uinput_request_send()
> - uinput.c::uinput_request_send() LOCKS using mutex_lock_interruptible()

Ugh... :(

> 
> Hints for solving this problem are much appreciated.
> Maybe by setting a flag when UI_DEV_DESTROY ioctl is issued,
> and avoiding to acquire the mutex in uinput_request_send()
> when this flag is found to be set?

No, it woudl still be racy. Unfortunately uinput allows to continue
using the device after "destroying" it...

Maybe instead of using mutex in uinput_request_send() we could use RCU
to fetch the input device assigned to uinput device and validate that
it is the same device for which we are trying to submit requests. And
have uinput_destroy_device() smash NULL into udev->dev early, before
calling uinput_flush_requests(udev) and input_unregister_device(). 

Thanks.

-- 
Dmitry