[PATCH] Input: uinput - fix circular locking dependency with ff-core

[PATCH] Input: uinput - fix circular locking dependency with ff-core

[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 replacing udev->mutex with the existing
udev->requests_lock spinlock in uinput_request_send().  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 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 -> requests_lock (spinlock, leaf)
  udev->mutex -> requests_lock (spinlock, leaf)
  udev->mutex -> input_mutex -> dev->mutex -> ff->mutex (no back-edge)

Link: https://lore.kernel.org/all/CABXGCsMoxag+kEwHhb7KqhuyxfmGGd0P=tHZyb1uKE0pLr8Hkg@mail.gmail.com/
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
 drivers/input/misc/uinput.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index e589060db280..dcb855234049 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -146,19 +146,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->requests_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 +162,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->requests_lock);
 	return retval;
 }
 
@@ -175,6 +171,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 +292,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 requests_lock so that concurrent
+	 * uinput_request_send() sees the state change before we
+	 * flush pending requests and tear down the device.
+	 */
+	spin_lock(&udev->requests_lock);
 	udev->state = UIST_NEW_DEVICE;
+	spin_unlock(&udev->requests_lock);
 
 	if (dev) {
 		name = dev->name;
@@ -366,7 +376,9 @@ static int uinput_create_device(struct uinput_device *udev)
 	if (error)
 		goto fail2;
 
+	spin_lock(&udev->requests_lock);
 	udev->state = UIST_CREATED;
+	spin_unlock(&udev->requests_lock);
 
 	return 0;
 
-- 
2.53.0

Re: [PATCH] Input: uinput - fix circular locking dependency with ff-core

[mikhail.v.gavrilov]

On Sun, 2026-03-01 at 03:36 +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.
> 
> 
> 
Hi Dmitry,

Friendly ping on this patch. It fixes a reproducible lockdep circular
dependency warning in uinput with force-feedback gamepads under
Wine/Proton, tested on 7.0-rc1.

I realize I should have included:

  Fixes: ff462551235d ("Input: uinput - switch to the new FF
interface")
  Cc: stable@vger.kernel.org

Should I resend as v2 with these tags, or can you add them when
applying?

-- 
Best Regards,
Mike Gavrilov.

Re: [PATCH] Input: uinput - fix circular locking dependency with ff-core

[Dmitry Torokhov]

On Wed, Mar 11, 2026 at 10:47:25PM +0500, mikhail.v.gavrilov@gmail.com wrote:
> On Sun, 2026-03-01 at 03:36 +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.
> > 
> > 
> > 
> Hi Dmitry,
> 
> Friendly ping on this patch. It fixes a reproducible lockdep circular
> dependency warning in uinput with force-feedback gamepads under
> Wine/Proton, tested on 7.0-rc1.
> 
> I realize I should have included:
> 
>   Fixes: ff462551235d ("Input: uinput - switch to the new FF
> interface")
>   Cc: stable@vger.kernel.org
> 
> Should I resend as v2 with these tags, or can you add them when
> applying?

Sorry I need a bit more time to evaluate the patch. I think what you
proposed makes sense, just give me a couple more days.

If there are no changes needed I can add necessary tags on my end.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: uinput - fix circular locking dependency with ff-core

[Dmitry Torokhov]

Hi Mikhail,

On Sun, Mar 01, 2026 at 03:36:28AM +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 replacing udev->mutex with the existing
> udev->requests_lock spinlock in uinput_request_send().  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 spinlock.

Thank you for the patch, it looks solid, however I wonder if creating a
separate "state_lock" spinlock would not be better than reusing
requests_lock?

Thanks.

-- 
Dmitry

Re: [PATCH] Input: uinput - fix circular locking dependency with ff-core

[Mikhail Gavrilov]

On Mon, Mar 23, 2026 at 7:47 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Thank you for the patch, it looks solid, however I wonder if creating a
> separate "state_lock" spinlock would not be better than reusing
> requests_lock?

Hi Dmitry,

Thank you for the review.

A separate spinlock would certainly be cleaner from a naming
perspective. One thing I'd like to note though: the current
approach of reusing requests_lock has the benefit of atomically
checking state and queueing the event in uinput_request_send(),
and atomically changing state and flushing requests in
uinput_destroy_device(). With a separate state_lock these become
two independent locks, so the ordering between them would need to
be defined.

That said, if you prefer the cleaner separation I'm happy to make
the change. Please let me know.

-- 
Best Regards,
Mike Gavrilov.

Re: [PATCH] Input: uinput - fix circular locking dependency with ff-core

[Dmitry Torokhov]

On Mon, Mar 23, 2026 at 10:17:01AM +0500, Mikhail Gavrilov wrote:
> On Mon, Mar 23, 2026 at 7:47 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Thank you for the patch, it looks solid, however I wonder if creating a
> > separate "state_lock" spinlock would not be better than reusing
> > requests_lock?
> 
> Hi Dmitry,
> 
> Thank you for the review.
> 
> A separate spinlock would certainly be cleaner from a naming
> perspective. One thing I'd like to note though: the current
> approach of reusing requests_lock has the benefit of atomically
> checking state and queueing the event in uinput_request_send(),
> and atomically changing state and flushing requests in
> uinput_destroy_device(). With a separate state_lock these become
> two independent locks, so the ordering between them would need to
> be defined.

Hmm, I am not sure I see the issue. We are not going to change state
back to UIST_CREATED until after uinput_destroy_device() returns so we
will not submit more requests...

What am I missing?

Thanks.

-- 
Dmitry

Re: [PATCH] Input: uinput - fix circular locking dependency with ff-core

[Mikhail Gavrilov]

On Mon, Mar 23, 2026 at 10:34 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hmm, I am not sure I see the issue. We are not going to change state
> back to UIST_CREATED until after uinput_destroy_device() returns so we
> will not submit more requests...
>
> What am I missing?

You are right, there is no lock ordering issue since the state
transition is one-way.

The reason I reused requests_lock is that uinput_request_send()
needs to atomically check state and access udev->dev. If we use a
separate state_lock and release it before calling
uinput_dev_event(), uinput_destroy_device() could run in between,
unregister the device, and we'd hit a use-after-free on udev->dev.

A separate lock would need to be held across the same scope,
making it functionally equivalent to reusing requests_lock.

-- 
Best Regards,
Mike Gavrilov.