[PATCH] Input: uinput - fix use-after-free of FF requests in ioctl handlers

[PATCH] Input: uinput - fix use-after-free of FF requests in ioctl handlers

[Cen Zhang (Microsoft)]

uinput_request_find() reads udev->requests[id] without holding
requests_lock, and the UI_BEGIN_FF_* / UI_END_FF_* ioctl handlers then
keep using the pointer it returned: the BEGIN handlers copy data out of
the request (including *req->u.upload.effect, which points into the
evdev caller's stack), the END handlers set req->retval and call
complete(&req->done).

The problem is that the request is a local variable of the task
sleeping in uinput_request_submit(). Once that task wakes up it clears
the slot and returns, and the request is gone. It can wake up at any
time as far as the ioctl handler is concerned: another UI_END_FF_* may
have completed it already, uinput_flush_requests() may have completed
it because the device is being destroyed, or the 30 second timeout may
have expired. If that happens after the lookup, the handler ends up
reading from and writing to a stack frame that no longer exists, and
complete() then tries to take a spinlock that contains whatever the
stack was reused for.

An easy-to-hit case: two processes sharing a uinput fd and both issuing
UI_END_FF_UPLOAD for the same request id crash a KASAN kernel within
seconds.

  Oops: general protection fault, probably for non-canonical address
    0xdffffc0000000001: 0000 [#1] SMP KASAN NOPTI
  KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
  RIP: 0010:complete+0x95/0x260
  Call Trace:
   uinput_ioctl_handler.isra.0+0x4b7/0x1a40
   __x64_sys_ioctl+0x184/0x1d0

Fix by holding requests_lock from the lookup until we are done with
the request, similar to what uinput_flush_requests() already does.
uinput_request_release_slot() takes that lock too, so the submitter
can not release the slot (and return) while a handler is still using
the request. Once the slot is released, a lookup can no longer return
the expired request.

To keep copy_to_user()/copy_from_user() out of the locked region, the
part of each handler that touches the request is moved into a small
helper that runs under the lock.

Fixes: 6b4877c7bdc6 ("Input: uinput - avoid crash when sending FF request to device going away")
Fixes: 8e009118a45a ("Input: uinput - allow FF requests to time out")
Cc: stable@vger.kernel.org
Cc: Francis Perron <francis@akrites.dev>
Reported-by: Hillf Danton <hdanton@sina.com>
Closes: https://lore.kernel.org/all/20240521113619.2092-1-hdanton@sina.com/
Reported-by: AutonomousCodeSecurity@microsoft.com
Assisted-by: LLM
Signed-off-by: Cen Zhang (Microsoft) <cenzhang@linux.microsoft.com>
---
 drivers/input/misc/uinput.c | 110 ++++++++++++++++++++++++------------
 1 file changed, 73 insertions(+), 37 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index d32fa4b508fc..1a294f4e941b 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -119,16 +119,80 @@ static bool uinput_request_alloc_id(struct uinput_device *udev,
 	return reserved;
 }
 
+/*
+ * Find an input request, by ID. Returns NULL if the ID isn't valid.
+ * Requests live on their submitter's stack, so the caller must hold
+ * requests_lock while using the result.
+ */
 static struct uinput_request *uinput_request_find(struct uinput_device *udev,
 						  unsigned int id)
 {
-	/* Find an input request, by ID. Returns NULL if the ID isn't valid. */
+	lockdep_assert_held(&udev->requests_lock);
+
 	if (id >= UINPUT_NUM_REQUESTS)
 		return NULL;
 
 	return udev->requests[id];
 }
 
+static int uinput_ff_upload_begin(struct uinput_device *udev,
+				  struct uinput_ff_upload *ff_up)
+{
+	struct uinput_request *req;
+
+	guard(spinlock)(&udev->requests_lock);
+
+	req = uinput_request_find(udev, ff_up->request_id);
+	if (!req || req->code != UI_FF_UPLOAD || !req->u.upload.effect)
+		return -EINVAL;
+
+	ff_up->retval = 0;
+	ff_up->effect = *req->u.upload.effect;
+	if (req->u.upload.old)
+		ff_up->old = *req->u.upload.old;
+	else
+		memset(&ff_up->old, 0, sizeof(ff_up->old));
+
+	return 0;
+}
+
+static int uinput_ff_erase_begin(struct uinput_device *udev,
+				 struct uinput_ff_erase *ff_erase)
+{
+	struct uinput_request *req;
+
+	guard(spinlock)(&udev->requests_lock);
+
+	req = uinput_request_find(udev, ff_erase->request_id);
+	if (!req || req->code != UI_FF_ERASE)
+		return -EINVAL;
+
+	ff_erase->retval = 0;
+	ff_erase->effect_id = req->u.effect_id;
+
+	return 0;
+}
+
+static int uinput_ff_request_end(struct uinput_device *udev,
+				 unsigned int id, unsigned int code, int retval)
+{
+	struct uinput_request *req;
+
+	guard(spinlock)(&udev->requests_lock);
+
+	req = uinput_request_find(udev, id);
+	if (!req || req->code != code)
+		return -EINVAL;
+
+	if (code == UI_FF_UPLOAD && !req->u.upload.effect)
+		return -EINVAL;
+
+	req->retval = retval;
+	complete(&req->done);
+
+	return 0;
+}
+
 static int uinput_request_reserve_slot(struct uinput_device *udev,
 				       struct uinput_request *request)
 {
@@ -913,7 +977,6 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 	struct uinput_device	*udev = file->private_data;
 	struct uinput_ff_upload ff_up;
 	struct uinput_ff_erase  ff_erase;
-	struct uinput_request   *req;
 	char			*phys;
 	const char		*name;
 	unsigned int		size;
@@ -1011,19 +1074,9 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 		if (retval)
 			goto out;
 
-		req = uinput_request_find(udev, ff_up.request_id);
-		if (!req || req->code != UI_FF_UPLOAD ||
-		    !req->u.upload.effect) {
-			retval = -EINVAL;
+		retval = uinput_ff_upload_begin(udev, &ff_up);
+		if (retval)
 			goto out;
-		}
-
-		ff_up.retval = 0;
-		ff_up.effect = *req->u.upload.effect;
-		if (req->u.upload.old)
-			ff_up.old = *req->u.upload.old;
-		else
-			memset(&ff_up.old, 0, sizeof(struct ff_effect));
 
 		retval = uinput_ff_upload_to_user(p, &ff_up);
 		goto out;
@@ -1034,14 +1087,10 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 			goto out;
 		}
 
-		req = uinput_request_find(udev, ff_erase.request_id);
-		if (!req || req->code != UI_FF_ERASE) {
-			retval = -EINVAL;
+		retval = uinput_ff_erase_begin(udev, &ff_erase);
+		if (retval)
 			goto out;
-		}
 
-		ff_erase.retval = 0;
-		ff_erase.effect_id = req->u.effect_id;
 		if (copy_to_user(p, &ff_erase, sizeof(ff_erase))) {
 			retval = -EFAULT;
 			goto out;
@@ -1054,15 +1103,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 		if (retval)
 			goto out;
 
-		req = uinput_request_find(udev, ff_up.request_id);
-		if (!req || req->code != UI_FF_UPLOAD ||
-		    !req->u.upload.effect) {
-			retval = -EINVAL;
-			goto out;
-		}
-
-		req->retval = ff_up.retval;
-		complete(&req->done);
+		retval = uinput_ff_request_end(udev, ff_up.request_id,
+					       UI_FF_UPLOAD, ff_up.retval);
 		goto out;
 
 	case UI_END_FF_ERASE:
@@ -1071,14 +1113,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 			goto out;
 		}
 
-		req = uinput_request_find(udev, ff_erase.request_id);
-		if (!req || req->code != UI_FF_ERASE) {
-			retval = -EINVAL;
-			goto out;
-		}
-
-		req->retval = ff_erase.retval;
-		complete(&req->done);
+		retval = uinput_ff_request_end(udev, ff_erase.request_id,
+					       UI_FF_ERASE, ff_erase.retval);
 		goto out;
 	}
 
-- 
2.55.0

Re: [PATCH] Input: uinput - fix use-after-free of FF requests in ioctl handlers

[Chad Aldreda]

On Tue, 13 Oct 2026, Cen Zhang (Microsoft) wrote:
> An easy-to-hit case: two processes sharing a uinput fd and both issuing
> UI_END_FF_UPLOAD for the same request id crash a KASAN kernel within
> seconds.

Tested on mainline d48723711bae (v7.4-rc1-672), where this still
applies, in QEMU with KASAN, lockdep and kmemleak. A test program
creates a uinput device with FF_RUMBLE, uploads and erases effects
through its evdev node, and answers the requests through /dev/uinput.

Without the patch, a variant of the case above reproduces at once: a
handler process answering each request while two other processes sharing
the uinput fd issue UI_END_FF_UPLOAD and UI_END_FF_ERASE for every id
gives "BUG: spinlock wrong CPU" from UI_END_FF_UPLOAD, then a NULL
pointer dereference in ttwu_queue_wakelist() under complete() from
UI_END_FF_ERASE, and the guest locks up. The BEGIN side fails too: two
processes issuing UI_BEGIN_FF_UPLOAD while the handler answers give a
general protection fault in uinput_ioctl_handler() within seconds.

With the patch there are no KASAN, lockdep, oops or kmemleak reports.
Each race ran for 20 s, and both together for another 20 s. Over a
million requests were answered. Duplicate BEGIN and END calls either
succeeded, while the request was still in its slot, or got -EINVAL once
it had been released. KASAN does not see stack use-after-return, so
without the patch it caught only what followed the corruption; "clean"
here means no crash or corruption.

Normal use still works: an upload and an erase complete, a negative
retval from the handler (-ENOSPC) comes back from EVIOCSFF, BEGIN on an
idle slot and END on an out-of-range id return -EINVAL, and an
unanswered upload times out with -ETIMEDOUT after 30 s, after which
answering it returns -EINVAL. The timeout case behaves the same without
the patch.

I also ran 300 rounds of UI_DEV_DESTROY while one process uploaded
effects in a loop and another ended every slot. That was clean with the
patch, but also clean without it, so it does not show the flush window
either way.

Not tested: KCSAN. The double-END runs did reach the window the
automated review describes for request->retval (about 540k ENDs
succeeded on a request already answered but not yet released), but every
answer used retval 0, so a wrong value would not have shown. Also not
tested: the compat ioctls, UI_BEGIN_FF_ERASE racing, an END landing
between a timeout and the slot release, real force-feedback hardware or
applications, and module unload (uinput was built in). Only x86_64 with
4 vCPUs.

The harness and this report were produced with an AI assistant (Claude
Opus 5.5), which also did the earlier runs, including the unpatched
comparison. I ran the final version on the patched kernel myself and got
the same results. The harness is available on request.

Tested-by: Chad Aldreda <ca@modelmiser.com>