Linux Input/HID development
 help / color / mirror / Atom feed
From: Doruk Tan Ozturk <doruk@0sec.ai>
To: jikos@kernel.org, bentiss@kernel.org
Cc: spbnick@gmail.com, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Doruk Tan Ozturk <doruk@0sec.ai>
Subject: [PATCH v2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error
Date: Sat, 11 Jul 2026 10:03:50 +0200	[thread overview]
Message-ID: <20260711080350.81108-1-doruk@0sec.ai> (raw)
In-Reply-To: <20260711073003.71012-1-doruk@0sec.ai>

uclogic_probe() arms a per-device timer whose callback
uclogic_inrange_timeout() dereferences drvdata->pen_input, and
uclogic_raw_event_pen() re-arms it with a 100 ms timeout on every
in-range pen report.

uclogic_remove() drained the timer with timer_delete_sync() before
hid_hw_stop().  timer_delete_sync() does not block re-arming: a pen
report delivered before hid_hw_stop() kills the URBs can re-arm the timer
after it was drained.  hid_hw_stop() then frees the hidinput pen_input
(via hidinput_disconnect() -> input_unregister_device()), and the pending
timer fires on freed memory.

Use timer_shutdown_sync() instead, still before hid_hw_stop().  It drains
the callback while pen_input is still valid and permanently blocks
re-arming, so an in-flight raw_event cannot revive the timer; hid_hw_stop()
then frees pen_input with the timer already dead.

The probe error path had the same exposure: if hid_hw_start() started I/O
and then failed, raw_event may have armed the timer, which would fire on
the devm-freed drvdata after probe returns.  Shut the timer down there too.

Unlike letsketch, whose input devices are devm-allocated and outlive
hid_hw_stop(), uclogic's pen_input is freed inside hid_hw_stop(), so the
timer must be shut down before it rather than after.

Found by 0sec (https://0sec.ai) using automated source analysis; not
runtime-reproduced.

Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v2:
 - Shut the timer down *before* hid_hw_stop() rather than after.  v1
   mirrored the letsketch ordering (hid_hw_stop() first), but uclogic's
   pen_input is the hidinput device freed inside hid_hw_stop(), not a
   devm device that outlives it as in letsketch.  A timer armed just
   before the URBs are killed could still fire on the freed pen_input in
   the window before timer_shutdown_sync() drained it.  Running
   timer_shutdown_sync() before hid_hw_stop() drains the callback while
   pen_input is still valid and blocks re-arming, closing that window.
 - Also shut the timer down on the probe error path.

 drivers/hid/hid-uclogic-core.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..d74f98efa879 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -267,6 +267,13 @@ static int uclogic_probe(struct hid_device *hdev,
 	/* Assume "remove" might not be called if "probe" failed */
 	if (params_initialized)
 		uclogic_params_cleanup(&drvdata->params);
+	/*
+	 * If hid_hw_start() started I/O and then failed, raw_event may have
+	 * armed the timer; shut it down so it cannot fire on the devm-freed
+	 * drvdata after probe returns.
+	 */
+	if (drvdata)
+		timer_shutdown_sync(&drvdata->inrange_timer);
 	return rc;
 }
 
@@ -548,7 +555,15 @@ static void uclogic_remove(struct hid_device *hdev)
 {
 	struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
 
-	timer_delete_sync(&drvdata->inrange_timer);
+	/*
+	 * timer_delete_sync() does not prevent re-arming, so a pen report
+	 * delivered before hid_hw_stop() kills the URBs could re-arm the
+	 * timer; hid_hw_stop() then frees the hidinput pen_input and the
+	 * pending timer fires on freed memory.  timer_shutdown_sync() drains
+	 * the callback while pen_input is still valid and permanently blocks
+	 * re-arming, so an in-flight raw_event cannot revive it.
+	 */
+	timer_shutdown_sync(&drvdata->inrange_timer);
 	hid_hw_stop(hdev);
 	kfree(drvdata->desc_ptr);
 	uclogic_params_cleanup(&drvdata->params);
-- 
2.43.0


  parent reply	other threads:[~2026-07-11  8:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-11  7:30 [PATCH] HID: uclogic: fix UAF on inrange_timer at driver unbind Doruk Tan Ozturk
2026-07-11  7:41 ` sashiko-bot
2026-07-11  8:03 ` Doruk Tan Ozturk [this message]
2026-07-11  8:37   ` [PATCH v2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260711080350.81108-1-doruk@0sec.ai \
    --to=doruk@0sec.ai \
    --cc=bentiss@kernel.org \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=spbnick@gmail.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox