mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-05-16 12:31:52 -04:00
Input: uinput - fix circular locking dependency with ff-core
A lockdep circular locking dependency warning can be triggered
reproducibly when using a force-feedback gamepad with uinput (for
example, playing ELDEN RING under Wine with a Flydigi Vader 5
controller):
ff->mutex -> udev->mutex -> input_mutex -> dev->mutex -> ff->mutex
The cycle is caused by four lock acquisition paths:
1. ff upload: input_ff_upload() holds ff->mutex and calls
uinput_dev_upload_effect() -> uinput_request_submit() ->
uinput_request_send(), which acquires udev->mutex.
2. device create: uinput_ioctl_handler() holds udev->mutex and calls
uinput_create_device() -> input_register_device(), which acquires
input_mutex.
3. device register: input_register_device() holds input_mutex and
calls kbd_connect() -> input_register_handle(), which acquires
dev->mutex.
4. evdev release: evdev_release() calls input_flush_device() under
dev->mutex, which calls input_ff_flush() acquiring ff->mutex.
Fix this by introducing a new state_lock spinlock to protect
udev->state and udev->dev access in uinput_request_send() instead of
acquiring udev->mutex. The function only needs to atomically check
device state and queue an input event into the ring buffer via
uinput_dev_event() -- both operations are safe under a spinlock
(ktime_get_ts64() and wake_up_interruptible() do not sleep). This
breaks the ff->mutex -> udev->mutex link since a spinlock is a leaf in
the lock ordering and cannot form cycles with mutexes.
To keep state transitions visible to uinput_request_send(), protect
writes to udev->state in uinput_create_device() and
uinput_destroy_device() with the same state_lock spinlock.
Additionally, move init_completion(&request->done) from
uinput_request_send() to uinput_request_submit() before
uinput_request_reserve_slot(). Once the slot is allocated,
uinput_flush_requests() may call complete() on it at any time from
the destroy path, so the completion must be initialised before the
request becomes visible.
Lock ordering after the fix:
ff->mutex -> state_lock (spinlock, leaf)
udev->mutex -> state_lock (spinlock, leaf)
udev->mutex -> input_mutex -> dev->mutex -> ff->mutex (no back-edge)
Fixes: ff46255123 ("Input: uinput - switch to the new FF interface")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/CABXGCsMoxag+kEwHhb7KqhuyxfmGGd0P=tHZyb1uKE0pLr8Hkg@mail.gmail.com/
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Link: https://patch.msgid.link/20260407075031.38351-1-mikhail.v.gavrilov@gmail.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
This commit is contained in:
committed by
Dmitry Torokhov
parent
0d9363a764
commit
4cda78d6f8
@@ -57,6 +57,7 @@ struct uinput_device {
|
||||
struct input_dev *dev;
|
||||
struct mutex mutex;
|
||||
enum uinput_state state;
|
||||
spinlock_t state_lock;
|
||||
wait_queue_head_t waitq;
|
||||
unsigned char ready;
|
||||
unsigned char head;
|
||||
@@ -146,19 +147,15 @@ static void uinput_request_release_slot(struct uinput_device *udev,
|
||||
static int uinput_request_send(struct uinput_device *udev,
|
||||
struct uinput_request *request)
|
||||
{
|
||||
int retval;
|
||||
int retval = 0;
|
||||
|
||||
retval = mutex_lock_interruptible(&udev->mutex);
|
||||
if (retval)
|
||||
return retval;
|
||||
spin_lock(&udev->state_lock);
|
||||
|
||||
if (udev->state != UIST_CREATED) {
|
||||
retval = -ENODEV;
|
||||
goto out;
|
||||
}
|
||||
|
||||
init_completion(&request->done);
|
||||
|
||||
/*
|
||||
* Tell our userspace application about this new request
|
||||
* by queueing an input event.
|
||||
@@ -166,7 +163,7 @@ static int uinput_request_send(struct uinput_device *udev,
|
||||
uinput_dev_event(udev->dev, EV_UINPUT, request->code, request->id);
|
||||
|
||||
out:
|
||||
mutex_unlock(&udev->mutex);
|
||||
spin_unlock(&udev->state_lock);
|
||||
return retval;
|
||||
}
|
||||
|
||||
@@ -175,6 +172,13 @@ static int uinput_request_submit(struct uinput_device *udev,
|
||||
{
|
||||
int retval;
|
||||
|
||||
/*
|
||||
* Initialize completion before allocating the request slot.
|
||||
* Once the slot is allocated, uinput_flush_requests() may
|
||||
* complete it at any time, so it must be initialized first.
|
||||
*/
|
||||
init_completion(&request->done);
|
||||
|
||||
retval = uinput_request_reserve_slot(udev, request);
|
||||
if (retval)
|
||||
return retval;
|
||||
@@ -289,7 +293,14 @@ static void uinput_destroy_device(struct uinput_device *udev)
|
||||
struct input_dev *dev = udev->dev;
|
||||
enum uinput_state old_state = udev->state;
|
||||
|
||||
/*
|
||||
* Update state under state_lock so that concurrent
|
||||
* uinput_request_send() sees the state change before we
|
||||
* flush pending requests and tear down the device.
|
||||
*/
|
||||
spin_lock(&udev->state_lock);
|
||||
udev->state = UIST_NEW_DEVICE;
|
||||
spin_unlock(&udev->state_lock);
|
||||
|
||||
if (dev) {
|
||||
name = dev->name;
|
||||
@@ -366,7 +377,9 @@ static int uinput_create_device(struct uinput_device *udev)
|
||||
if (error)
|
||||
goto fail2;
|
||||
|
||||
spin_lock(&udev->state_lock);
|
||||
udev->state = UIST_CREATED;
|
||||
spin_unlock(&udev->state_lock);
|
||||
|
||||
return 0;
|
||||
|
||||
@@ -384,6 +397,7 @@ static int uinput_open(struct inode *inode, struct file *file)
|
||||
return -ENOMEM;
|
||||
|
||||
mutex_init(&newdev->mutex);
|
||||
spin_lock_init(&newdev->state_lock);
|
||||
spin_lock_init(&newdev->requests_lock);
|
||||
init_waitqueue_head(&newdev->requests_waitq);
|
||||
init_waitqueue_head(&newdev->waitq);
|
||||
|
||||
Reference in New Issue
Block a user