public inbox for linux-input@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Input: uinput - fix circular locking dependency with ff-core
@ 2026-04-07  7:50 Mikhail Gavrilov
  2026-04-08  4:56 ` Dmitry Torokhov
  0 siblings, 1 reply; 2+ messages in thread
From: Mikhail Gavrilov @ 2026-04-07  7:50 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, linux-kernel, stable, Mikhail Gavrilov

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: ff462551235d ("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>
---

Changes since v1:
https://lore.kernel.org/all/20260228223628.472208-1-mikhail.v.gavrilov@gmail.com/

- Use a dedicated state_lock spinlock instead of reusing requests_lock,
  as suggested by Dmitry Torokhov
- Add Fixes and Cc: stable tags

 drivers/input/misc/uinput.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index e589060db280..e24caf6fc8e8 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -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);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] Input: uinput - fix circular locking dependency with ff-core
  2026-04-07  7:50 [PATCH v2] Input: uinput - fix circular locking dependency with ff-core Mikhail Gavrilov
@ 2026-04-08  4:56 ` Dmitry Torokhov
  0 siblings, 0 replies; 2+ messages in thread
From: Dmitry Torokhov @ 2026-04-08  4:56 UTC (permalink / raw)
  To: Mikhail Gavrilov; +Cc: linux-input, linux-kernel, stable

On Tue, Apr 07, 2026 at 12:50:31PM +0500, Mikhail Gavrilov wrote:
> 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: ff462551235d ("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>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-04-08  4:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07  7:50 [PATCH v2] Input: uinput - fix circular locking dependency with ff-core Mikhail Gavrilov
2026-04-08  4:56 ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox